You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2016/06/27 08:00:30 UTC
svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h
proxy_util.c
Author: ylavic
Date: Mon Jun 27 08:00:30 2016
New Revision: 1750301
URL: http://svn.apache.org/viewvc?rev=1750301&view=rev
Log:
mod_proxy: don't reuse backend connections with data available before the
request is sent. PR 57832.
Modified:
httpd/httpd/trunk/modules/proxy/mod_proxy.h
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1750301&r1=1750300&r2=1750301&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Mon Jun 27 08:00:30 2016
@@ -271,6 +271,7 @@ typedef struct {
unsigned int inreslist:1; /* connection in apr_reslist? */
const char *uds_path; /* Unix domain socket path */
const char *ssl_hostname;/* Hostname (SNI) in use by SSL connection */
+ apr_bucket_brigade *tmp_bb;
} proxy_conn_rec;
typedef struct {
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1750301&r1=1750300&r2=1750301&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Mon Jun 27 08:00:30 2016
@@ -2487,7 +2487,7 @@ ap_proxy_determine_connection(apr_pool_t
#endif
#if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
-PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
+static int get_socket_connected(apr_socket_t *socket)
{
apr_pollfd_t pfds[1];
apr_status_t status;
@@ -2514,7 +2514,7 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
status = apr_socket_recvfrom(&unused, socket, APR_MSG_PEEK,
&buf[0], &len);
if (status == APR_SUCCESS && len)
- return 1;
+ return 2;
else
return 0;
}
@@ -2525,7 +2525,7 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
}
#else
-PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
+static int is_socket_connnected(apr_socket_t *socket)
{
apr_size_t buffer_len = 1;
@@ -2544,12 +2544,19 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
|| APR_STATUS_IS_ECONNRESET(socket_status)) {
return 0;
}
+ else if (status == APR_SUCCESS && buffer_len) {
+ return 2;
+ }
else {
return 1;
}
}
#endif /* USE_ALTERNATE_IS_CONNECTED */
+PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
+{
+ return get_socket_connected(socket) != 0;
+}
/*
* Send a HTTP CONNECT request to a forward proxy.
@@ -2716,7 +2723,35 @@ PROXY_DECLARE(int) ap_proxy_connect_back
(proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
if (conn->sock) {
- if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
+ conn_rec *c = conn->connection;
+ if (!c) {
+ connected = get_socket_connected(conn->sock);
+ }
+ else {
+ if (conn->tmp_bb == NULL) {
+ conn->tmp_bb = apr_brigade_create(c->pool, c->bucket_alloc);
+ }
+ rv = ap_get_brigade(c->input_filters, conn->tmp_bb,
+ AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
+ if (rv == APR_SUCCESS) {
+ apr_off_t len = 0;
+ apr_brigade_length(conn->tmp_bb, 0, &len);
+ if (len) {
+ connected = 2;
+ }
+ else {
+ connected = 1;
+ }
+ }
+ else if (APR_STATUS_IS_EAGAIN(rv)) {
+ connected = 1;
+ }
+ else {
+ connected = 0;
+ }
+ apr_brigade_cleanup(conn->tmp_bb);
+ }
+ if (connected != 1) {
/* This clears conn->scpool (and associated data), so backup and
* restore any ssl_hostname for this connection set earlier by
* ap_proxy_determine_connection().
@@ -2728,9 +2763,17 @@ PROXY_DECLARE(int) ap_proxy_connect_back
}
socket_cleanup(conn);
- ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
- "%s: backend socket is disconnected.",
- proxy_function);
+ if (!connected) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
+ "%s: backend socket is disconnected.",
+ proxy_function);
+ }
+ else {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO()
+ "%s: reusable backend connection is not empty: "
+ "forcibly closed", proxy_function);
+ connected = 0;
+ }
if (ssl_hostname[0]) {
conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
RE: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Original Message-----
> From: Stefan Eissing [mailto:stefan@eissing.org]
> Sent: Montag, 27. Juni 2016 10:24
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
> mod_proxy.h proxy_util.c
>
> This looks nice for HTTP/1.1, but what about other protocols? Do I read it
> correctly that any pending data downstream will reopen the connection?
Agreed. This is a severe change in behaviour and does not let other protocols handle this in their own way.
For them pending data might be fine.
So especially I wouldn't backport it to 2.4.x in this state.
Regards
Rüdiger
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
Commited in r1750505.
> Am 28.06.2016 um 15:13 schrieb Yann Ylavic <yl...@gmail.com>:
>
> Jim didn't tag yet AFAICT, did he?
> If not, since it's mod_proxy_http2 scope only, +1 for me.
>
> On Tue, Jun 28, 2016 at 3:02 PM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> Thanks. I had to change from r-> to ctx->rbase, but otherwise works fine.
>>
>> Shall I commit this and potentially break Jim's tagging again?
>>
>> -Stefan
>>
>>
>>
>>> Am 28.06.2016 um 13:49 schrieb Yann Ylavic <yl...@gmail.com>:
>>>
>>> Patch as a file attached.
>>>
>>> On Tue, Jun 28, 2016 at 1:48 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>>> Maybe if you can test current 2.4.x with this patch and it works as
>>>> expected it could be backported...
>>>>
>>>> On Tue, Jun 28, 2016 at 1:46 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>> Dunno, the issue is that reused TLS connections where data are
>>>>> immediately available from the backend may be missing some bytes...
>>>>>
>>>>> On Tue, Jun 28, 2016 at 1:43 PM, Stefan Eissing
>>>>> <st...@greenbytes.de> wrote:
>>>>>> Ah, understood. Do you want to squeeze it into 2.4.23 or can it wait?
>>>>>>
>>>>>>> Am 28.06.2016 um 13:42 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>
>>>>>>> I don't think trunk needs it because ap_proxy_connect_backend() is
>>>>>>> already doing this work (via ap_proxy_check_backend).
>>>>>>>
>>>>>>> That's why I proposed a 2.4.x only patch, but I can commit it to trunk
>>>>>>> temporarily if that helps (and until) backport...
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
>>>>>>> <st...@greenbytes.de> wrote:
>>>>>>>> We are talking about adding this to trunk first, right? ^^
>>>>>>>>
>>>>>>>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>>>>>>>>>
>>>>>>>>> I believe so. Highly experimental and all such...
>>>>>>>>>
>>>>>>>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>>>>
>>>>>>>>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>>>>>>>>> mod_h2 ?
>>>>>>>>>>
>>>>>>>>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>>>>>>>>> <st...@greenbytes.de> wrote:
>>>>>>>>>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>>>>>>>>>
>>>>>>>>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>>>>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>>>>>>>>>> reusing a backend connection.
>>>>>>>>>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>>>>>>>>>> with (pre-)available data...
>>>>>>>>>>>>>
>>>>>>>>>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>>>>>>>>>
>>>>>>>>>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>>>>>>>>>
>>>>>>>>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>>>>>>>>> ===================================================================
>>>>>>>>>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>>>>>>>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>>>>>>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>>>>>>>>>> - if (ctx->is_ssl) {
>>>>>>>>>>>> - /* If there is still some data on an existing ssl connection, now
>>>>>>>>>>>> - * would be a good timne to get rid of it. */
>>>>>>>>>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>>>>>>>>>> - }
>>>>>>>>>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>>>>>>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>>>>>>>>>> + * let mod_ssl detect them, and create a new connection below.
>>>>>>>>>>>> + */
>>>>>>>>>>>> + apr_bucket_brigade *tmp_bb;
>>>>>>>>>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>>>>>>>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>>>>>>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>>>>>>>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>>>>>>>>>> + ctx->p_conn->close = 1;
>>>>>>>>>>>> + }
>>>>>>>>>>>> + apr_brigade_cleanup(tmp_bb);
>>>>>>>>>>>> + }
>>>>>>>>>>>>
>>>>>>>>>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>>>>>>>>>> * initialize the backend accordingly and determine the server
>>>>>>>>>>>> _
>>>>>>>>>>>>
>>>>>>>>>>>> Stefan?
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>> <httpd-2.4.x-mod_proxy_http2-ssl_reuse_cleanup.patch>
>>
>>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
Jim didn't tag yet AFAICT, did he?
If not, since it's mod_proxy_http2 scope only, +1 for me.
On Tue, Jun 28, 2016 at 3:02 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
> Thanks. I had to change from r-> to ctx->rbase, but otherwise works fine.
>
> Shall I commit this and potentially break Jim's tagging again?
>
> -Stefan
>
>
>
>> Am 28.06.2016 um 13:49 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> Patch as a file attached.
>>
>> On Tue, Jun 28, 2016 at 1:48 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>> Maybe if you can test current 2.4.x with this patch and it works as
>>> expected it could be backported...
>>>
>>> On Tue, Jun 28, 2016 at 1:46 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>>> Dunno, the issue is that reused TLS connections where data are
>>>> immediately available from the backend may be missing some bytes...
>>>>
>>>> On Tue, Jun 28, 2016 at 1:43 PM, Stefan Eissing
>>>> <st...@greenbytes.de> wrote:
>>>>> Ah, understood. Do you want to squeeze it into 2.4.23 or can it wait?
>>>>>
>>>>>> Am 28.06.2016 um 13:42 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>
>>>>>> I don't think trunk needs it because ap_proxy_connect_backend() is
>>>>>> already doing this work (via ap_proxy_check_backend).
>>>>>>
>>>>>> That's why I proposed a 2.4.x only patch, but I can commit it to trunk
>>>>>> temporarily if that helps (and until) backport...
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
>>>>>> <st...@greenbytes.de> wrote:
>>>>>>> We are talking about adding this to trunk first, right? ^^
>>>>>>>
>>>>>>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>>>>>>>>
>>>>>>>> I believe so. Highly experimental and all such...
>>>>>>>>
>>>>>>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>>>
>>>>>>>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>>>>>>>> mod_h2 ?
>>>>>>>>>
>>>>>>>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>>>>>>>> <st...@greenbytes.de> wrote:
>>>>>>>>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>>>>>>>>
>>>>>>>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>>>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>>>>>>>>> reusing a backend connection.
>>>>>>>>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>>>>>>>>> with (pre-)available data...
>>>>>>>>>>>>
>>>>>>>>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>>>>>>>>
>>>>>>>>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>>>>>>>>
>>>>>>>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>>>>>>>> ===================================================================
>>>>>>>>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>>>>>>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>>>>>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>>>>>>>>> - if (ctx->is_ssl) {
>>>>>>>>>>> - /* If there is still some data on an existing ssl connection, now
>>>>>>>>>>> - * would be a good timne to get rid of it. */
>>>>>>>>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>>>>>>>>> - }
>>>>>>>>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>>>>>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>>>>>>>>> + * let mod_ssl detect them, and create a new connection below.
>>>>>>>>>>> + */
>>>>>>>>>>> + apr_bucket_brigade *tmp_bb;
>>>>>>>>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>>>>>>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>>>>>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>>>>>>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>>>>>>>>> + ctx->p_conn->close = 1;
>>>>>>>>>>> + }
>>>>>>>>>>> + apr_brigade_cleanup(tmp_bb);
>>>>>>>>>>> + }
>>>>>>>>>>>
>>>>>>>>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>>>>>>>>> * initialize the backend accordingly and determine the server
>>>>>>>>>>> _
>>>>>>>>>>>
>>>>>>>>>>> Stefan?
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>> <httpd-2.4.x-mod_proxy_http2-ssl_reuse_cleanup.patch>
>
>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
Thanks. I had to change from r-> to ctx->rbase, but otherwise works fine.
Shall I commit this and potentially break Jim's tagging again?
-Stefan
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
Patch as a file attached.
On Tue, Jun 28, 2016 at 1:48 PM, Yann Ylavic <yl...@gmail.com> wrote:
> Maybe if you can test current 2.4.x with this patch and it works as
> expected it could be backported...
>
> On Tue, Jun 28, 2016 at 1:46 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> Dunno, the issue is that reused TLS connections where data are
>> immediately available from the backend may be missing some bytes...
>>
>> On Tue, Jun 28, 2016 at 1:43 PM, Stefan Eissing
>> <st...@greenbytes.de> wrote:
>>> Ah, understood. Do you want to squeeze it into 2.4.23 or can it wait?
>>>
>>>> Am 28.06.2016 um 13:42 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>
>>>> I don't think trunk needs it because ap_proxy_connect_backend() is
>>>> already doing this work (via ap_proxy_check_backend).
>>>>
>>>> That's why I proposed a 2.4.x only patch, but I can commit it to trunk
>>>> temporarily if that helps (and until) backport...
>>>>
>>>>
>>>> On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
>>>> <st...@greenbytes.de> wrote:
>>>>> We are talking about adding this to trunk first, right? ^^
>>>>>
>>>>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>>>>>>
>>>>>> I believe so. Highly experimental and all such...
>>>>>>
>>>>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>
>>>>>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>>>>>> mod_h2 ?
>>>>>>>
>>>>>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>>>>>> <st...@greenbytes.de> wrote:
>>>>>>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>>>>>>
>>>>>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>>>
>>>>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>>>>>>> reusing a backend connection.
>>>>>>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>>>>>>> with (pre-)available data...
>>>>>>>>>>
>>>>>>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>>>>>>
>>>>>>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>>>>>>
>>>>>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>>>>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>>>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>>>>>>> - if (ctx->is_ssl) {
>>>>>>>>> - /* If there is still some data on an existing ssl connection, now
>>>>>>>>> - * would be a good timne to get rid of it. */
>>>>>>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>>>>>>> - }
>>>>>>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>>>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>>>>>>> + * let mod_ssl detect them, and create a new connection below.
>>>>>>>>> + */
>>>>>>>>> + apr_bucket_brigade *tmp_bb;
>>>>>>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>>>>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>>>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>>>>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>>>>>>> + ctx->p_conn->close = 1;
>>>>>>>>> + }
>>>>>>>>> + apr_brigade_cleanup(tmp_bb);
>>>>>>>>> + }
>>>>>>>>>
>>>>>>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>>>>>>> * initialize the backend accordingly and determine the server
>>>>>>>>> _
>>>>>>>>>
>>>>>>>>> Stefan?
>>>>>>>>
>>>>>>
>>>>>
>>>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
Maybe if you can test current 2.4.x with this patch and it works as
expected it could be backported...
On Tue, Jun 28, 2016 at 1:46 PM, Yann Ylavic <yl...@gmail.com> wrote:
> Dunno, the issue is that reused TLS connections where data are
> immediately available from the backend may be missing some bytes...
>
> On Tue, Jun 28, 2016 at 1:43 PM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> Ah, understood. Do you want to squeeze it into 2.4.23 or can it wait?
>>
>>> Am 28.06.2016 um 13:42 schrieb Yann Ylavic <yl...@gmail.com>:
>>>
>>> I don't think trunk needs it because ap_proxy_connect_backend() is
>>> already doing this work (via ap_proxy_check_backend).
>>>
>>> That's why I proposed a 2.4.x only patch, but I can commit it to trunk
>>> temporarily if that helps (and until) backport...
>>>
>>>
>>> On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
>>> <st...@greenbytes.de> wrote:
>>>> We are talking about adding this to trunk first, right? ^^
>>>>
>>>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>>>>>
>>>>> I believe so. Highly experimental and all such...
>>>>>
>>>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>
>>>>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>>>>> mod_h2 ?
>>>>>>
>>>>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>>>>> <st...@greenbytes.de> wrote:
>>>>>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>>>>>
>>>>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>>
>>>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>>>>>> reusing a backend connection.
>>>>>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>>>>>> with (pre-)available data...
>>>>>>>>>
>>>>>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>>>>>
>>>>>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>>>>>
>>>>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>>>>> ===================================================================
>>>>>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>>>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>>>>> }
>>>>>>>>
>>>>>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>>>>>> - if (ctx->is_ssl) {
>>>>>>>> - /* If there is still some data on an existing ssl connection, now
>>>>>>>> - * would be a good timne to get rid of it. */
>>>>>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>>>>>> - }
>>>>>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>>>>>> + * let mod_ssl detect them, and create a new connection below.
>>>>>>>> + */
>>>>>>>> + apr_bucket_brigade *tmp_bb;
>>>>>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>>>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>>>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>>>>>> + ctx->p_conn->close = 1;
>>>>>>>> + }
>>>>>>>> + apr_brigade_cleanup(tmp_bb);
>>>>>>>> + }
>>>>>>>>
>>>>>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>>>>>> * initialize the backend accordingly and determine the server
>>>>>>>> _
>>>>>>>>
>>>>>>>> Stefan?
>>>>>>>
>>>>>
>>>>
>>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
Dunno, the issue is that reused TLS connections where data are
immediately available from the backend may be missing some bytes...
On Tue, Jun 28, 2016 at 1:43 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
> Ah, understood. Do you want to squeeze it into 2.4.23 or can it wait?
>
>> Am 28.06.2016 um 13:42 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> I don't think trunk needs it because ap_proxy_connect_backend() is
>> already doing this work (via ap_proxy_check_backend).
>>
>> That's why I proposed a 2.4.x only patch, but I can commit it to trunk
>> temporarily if that helps (and until) backport...
>>
>>
>> On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
>> <st...@greenbytes.de> wrote:
>>> We are talking about adding this to trunk first, right? ^^
>>>
>>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>>>>
>>>> I believe so. Highly experimental and all such...
>>>>
>>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>
>>>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>>>> mod_h2 ?
>>>>>
>>>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>>>> <st...@greenbytes.de> wrote:
>>>>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>>>>
>>>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>>
>>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>>>>> reusing a backend connection.
>>>>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>>>>> with (pre-)available data...
>>>>>>>>
>>>>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>>>>
>>>>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>>>>
>>>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>>>> ===================================================================
>>>>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>>>> }
>>>>>>>
>>>>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>>>>> - if (ctx->is_ssl) {
>>>>>>> - /* If there is still some data on an existing ssl connection, now
>>>>>>> - * would be a good timne to get rid of it. */
>>>>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>>>>> - }
>>>>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>>>>> + * let mod_ssl detect them, and create a new connection below.
>>>>>>> + */
>>>>>>> + apr_bucket_brigade *tmp_bb;
>>>>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>>>>> + ctx->p_conn->close = 1;
>>>>>>> + }
>>>>>>> + apr_brigade_cleanup(tmp_bb);
>>>>>>> + }
>>>>>>>
>>>>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>>>>> * initialize the backend accordingly and determine the server
>>>>>>> _
>>>>>>>
>>>>>>> Stefan?
>>>>>>
>>>>
>>>
>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
Ah, understood. Do you want to squeeze it into 2.4.23 or can it wait?
> Am 28.06.2016 um 13:42 schrieb Yann Ylavic <yl...@gmail.com>:
>
> I don't think trunk needs it because ap_proxy_connect_backend() is
> already doing this work (via ap_proxy_check_backend).
>
> That's why I proposed a 2.4.x only patch, but I can commit it to trunk
> temporarily if that helps (and until) backport...
>
>
> On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> We are talking about adding this to trunk first, right? ^^
>>
>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>>>
>>> I believe so. Highly experimental and all such...
>>>
>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>
>>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>>> mod_h2 ?
>>>>
>>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>>> <st...@greenbytes.de> wrote:
>>>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>>>
>>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>>
>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>>
>>>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>>>> reusing a backend connection.
>>>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>>>> with (pre-)available data...
>>>>>>>
>>>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>>>
>>>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>>>
>>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>>> ===================================================================
>>>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>>> }
>>>>>>
>>>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>>>> - if (ctx->is_ssl) {
>>>>>> - /* If there is still some data on an existing ssl connection, now
>>>>>> - * would be a good timne to get rid of it. */
>>>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>>>> - }
>>>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>>>> + * let mod_ssl detect them, and create a new connection below.
>>>>>> + */
>>>>>> + apr_bucket_brigade *tmp_bb;
>>>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>>>> + ctx->p_conn->close = 1;
>>>>>> + }
>>>>>> + apr_brigade_cleanup(tmp_bb);
>>>>>> + }
>>>>>>
>>>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>>>> * initialize the backend accordingly and determine the server
>>>>>> _
>>>>>>
>>>>>> Stefan?
>>>>>
>>>
>>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
I don't think trunk needs it because ap_proxy_connect_backend() is
already doing this work (via ap_proxy_check_backend).
That's why I proposed a 2.4.x only patch, but I can commit it to trunk
temporarily if that helps (and until) backport...
On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
> We are talking about adding this to trunk first, right? ^^
>
>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>>
>> I believe so. Highly experimental and all such...
>>
>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>>
>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>> mod_h2 ?
>>>
>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>> <st...@greenbytes.de> wrote:
>>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>>
>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>>
>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>
>>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>>> reusing a backend connection.
>>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>>> with (pre-)available data...
>>>>>>
>>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>>
>>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>>
>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>> ===================================================================
>>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>> }
>>>>>
>>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>>> - if (ctx->is_ssl) {
>>>>> - /* If there is still some data on an existing ssl connection, now
>>>>> - * would be a good timne to get rid of it. */
>>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>>> - }
>>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>>> + * let mod_ssl detect them, and create a new connection below.
>>>>> + */
>>>>> + apr_bucket_brigade *tmp_bb;
>>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>>> + ctx->p_conn->close = 1;
>>>>> + }
>>>>> + apr_brigade_cleanup(tmp_bb);
>>>>> + }
>>>>>
>>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>>> * initialize the backend accordingly and determine the server
>>>>> _
>>>>>
>>>>> Stefan?
>>>>
>>
>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
We are talking about adding this to trunk first, right? ^^
> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <st...@greenbytes.de>:
>
> I believe so. Highly experimental and all such...
>
>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>> mod_h2 ?
>>
>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>> <st...@greenbytes.de> wrote:
>>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>>
>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>
>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>
>>>>>> The possible issue if r1750414 were backported, is that without
>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>>> reusing a backend connection.
>>>>>> If it's not backported, it may close a legitimate backend connection
>>>>>> with (pre-)available data...
>>>>>
>>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>>
>>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>>
>>>> Index: modules/http2/mod_proxy_http2.c
>>>> ===================================================================
>>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>>> @@ -520,11 +520,19 @@ run_connect:
>>>> }
>>>>
>>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>>> - if (ctx->is_ssl) {
>>>> - /* If there is still some data on an existing ssl connection, now
>>>> - * would be a good timne to get rid of it. */
>>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>>> - }
>>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>>> + * let mod_ssl detect them, and create a new connection below.
>>>> + */
>>>> + apr_bucket_brigade *tmp_bb;
>>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>>> + ctx->p_conn->close = 1;
>>>> + }
>>>> + apr_brigade_cleanup(tmp_bb);
>>>> + }
>>>>
>>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>>> * initialize the backend accordingly and determine the server
>>>> _
>>>>
>>>> Stefan?
>>>
>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
I believe so. Highly experimental and all such...
> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <yl...@gmail.com>:
>
> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
> mod_h2 ?
>
> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> Looks good to me. Can you commit this, then I quickly run my tests with it...
>>
>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>>
>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>
>>>>> The possible issue if r1750414 were backported, is that without
>>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>>> reusing a backend connection.
>>>>> If it's not backported, it may close a legitimate backend connection
>>>>> with (pre-)available data...
>>>>
>>>> I meant: it may discard (pre-)available data (not closing the connection).
>>>
>>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>>
>>> Index: modules/http2/mod_proxy_http2.c
>>> ===================================================================
>>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>>> +++ modules/http2/mod_proxy_http2.c (working copy)
>>> @@ -520,11 +520,19 @@ run_connect:
>>> }
>>>
>>> ctx->p_conn->is_ssl = ctx->is_ssl;
>>> - if (ctx->is_ssl) {
>>> - /* If there is still some data on an existing ssl connection, now
>>> - * would be a good timne to get rid of it. */
>>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>>> - }
>>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>>> + /* If there are some metadata on the connection (e.g. TLS alert),
>>> + * let mod_ssl detect them, and create a new connection below.
>>> + */
>>> + apr_bucket_brigade *tmp_bb;
>>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>>> + ctx->p_conn->close = 1;
>>> + }
>>> + apr_brigade_cleanup(tmp_bb);
>>> + }
>>>
>>> /* Step One: Determine the URL to connect to (might be a proxy),
>>> * initialize the backend accordingly and determine the server
>>> _
>>>
>>> Stefan?
>>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
I can, but is mod_proxy_h2 CTR (Commit Then Review) like
mod_h2 ?
On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
> Looks good to me. Can you commit this, then I quickly run my tests with it...
>
>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>
>>>> The possible issue if r1750414 were backported, is that without
>>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>>> reusing a backend connection.
>>>> If it's not backported, it may close a legitimate backend connection
>>>> with (pre-)available data...
>>>
>>> I meant: it may discard (pre-)available data (not closing the connection).
>>
>> A possible solution for 2.4.x (needed only there AFAICT), could be:
>>
>> Index: modules/http2/mod_proxy_http2.c
>> ===================================================================
>> --- modules/http2/mod_proxy_http2.c (revision 1750453)
>> +++ modules/http2/mod_proxy_http2.c (working copy)
>> @@ -520,11 +520,19 @@ run_connect:
>> }
>>
>> ctx->p_conn->is_ssl = ctx->is_ssl;
>> - if (ctx->is_ssl) {
>> - /* If there is still some data on an existing ssl connection, now
>> - * would be a good timne to get rid of it. */
>> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
>> - }
>> + if (ctx->is_ssl && ctx->p_conn->connection) {
>> + /* If there are some metadata on the connection (e.g. TLS alert),
>> + * let mod_ssl detect them, and create a new connection below.
>> + */
>> + apr_bucket_brigade *tmp_bb;
>> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
>> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
>> + ctx->p_conn->close = 1;
>> + }
>> + apr_brigade_cleanup(tmp_bb);
>> + }
>>
>> /* Step One: Determine the URL to connect to (might be a proxy),
>> * initialize the backend accordingly and determine the server
>> _
>>
>> Stefan?
>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
Looks good to me. Can you commit this, then I quickly run my tests with it...
> Am 28.06.2016 um 09:50 schrieb Yann Ylavic <yl...@gmail.com>:
>
> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> The possible issue if r1750414 were backported, is that without
>>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>>> reusing a backend connection.
>>> If it's not backported, it may close a legitimate backend connection
>>> with (pre-)available data...
>>
>> I meant: it may discard (pre-)available data (not closing the connection).
>
> A possible solution for 2.4.x (needed only there AFAICT), could be:
>
> Index: modules/http2/mod_proxy_http2.c
> ===================================================================
> --- modules/http2/mod_proxy_http2.c (revision 1750453)
> +++ modules/http2/mod_proxy_http2.c (working copy)
> @@ -520,11 +520,19 @@ run_connect:
> }
>
> ctx->p_conn->is_ssl = ctx->is_ssl;
> - if (ctx->is_ssl) {
> - /* If there is still some data on an existing ssl connection, now
> - * would be a good timne to get rid of it. */
> - ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
> - }
> + if (ctx->is_ssl && ctx->p_conn->connection) {
> + /* If there are some metadata on the connection (e.g. TLS alert),
> + * let mod_ssl detect them, and create a new connection below.
> + */
> + apr_bucket_brigade *tmp_bb;
> + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
> + status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
> + if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
> + ctx->p_conn->close = 1;
> + }
> + apr_brigade_cleanup(tmp_bb);
> + }
>
> /* Step One: Determine the URL to connect to (might be a proxy),
> * initialize the backend accordingly and determine the server
> _
>
> Stefan?
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> The possible issue if r1750414 were backported, is that without
>> r1750392 mod_proxy_http2 may not detect a TLS close notify before
>> reusing a backend connection.
>> If it's not backported, it may close a legitimate backend connection
>> with (pre-)available data...
>
> I meant: it may discard (pre-)available data (not closing the connection).
A possible solution for 2.4.x (needed only there AFAICT), could be:
Index: modules/http2/mod_proxy_http2.c
===================================================================
--- modules/http2/mod_proxy_http2.c (revision 1750453)
+++ modules/http2/mod_proxy_http2.c (working copy)
@@ -520,11 +520,19 @@ run_connect:
}
ctx->p_conn->is_ssl = ctx->is_ssl;
- if (ctx->is_ssl) {
- /* If there is still some data on an existing ssl connection, now
- * would be a good timne to get rid of it. */
- ap_proxy_ssl_connection_cleanup(ctx->p_conn, ctx->rbase);
- }
+ if (ctx->is_ssl && ctx->p_conn->connection) {
+ /* If there are some metadata on the connection (e.g. TLS alert),
+ * let mod_ssl detect them, and create a new connection below.
+ */
+ apr_bucket_brigade *tmp_bb;
+ tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+ status = ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
+ AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
+ if (status != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(status)) {
+ ctx->p_conn->close = 1;
+ }
+ apr_brigade_cleanup(tmp_bb);
+ }
/* Step One: Determine the URL to connect to (might be a proxy),
* initialize the backend accordingly and determine the server
_
Stefan?
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Jun 28, 2016 at 12:11 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Mon, Jun 27, 2016 at 11:26 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> On Mon, Jun 27, 2016 at 10:48 AM, Stefan Eissing
>> <st...@greenbytes.de> wrote:
>>>
>>>> Am 27.06.2016 um 10:41 schrieb Yann Ylavic <yl...@gmail.com>:
>>>>
>>>> On Mon, Jun 27, 2016 at 10:23 AM, Stefan Eissing <st...@eissing.org> wrote:
>>>>> This looks nice for HTTP/1.1, but what about other protocols? Do I read it correctly that any pending data downstream will reopen the connection?
>>>>
>>>> Hmm, I did not think about mod_proxy_h2, but correct (I'd rather say
>>>> upstream though, backend side).
>>>> Are there cases where some backend data are available before the
>>>> request is sent?
>>>
>>> In HTTP/2, yes. For example a PING frame might be waiting, or a
>>> SETTINGS change. Most backends will have short timeouts for answers
>>> to these (SETTINGS is expected to be ACKed soonish), but races may
>>> happen. Also, HTTP/2 extensions with new frame types that need to be
>>> ignored when not known may get received here.
>>
>> OK, so we probably should get rid of the call to
>> ap_proxy_ssl_connection_cleanup() in proxy_http2_handler (and also in
>> proxy_http_handler with my latest changes since the same check is now
>> available in ap_proxy_check_connection).
>
> I went ahead and committed r1750414 (resp. r1750416).
> The possible issue if r1750414 were backported, is that without
> r1750392 mod_proxy_http2 may not detect a TLS close notify before
> reusing a backend connection.
> If it's not backported, it may close a legitimate backend connection
> with (pre-)available data...
I meant: it may discard (pre-)available data (not closing the connection).
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 27, 2016 at 11:26 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Mon, Jun 27, 2016 at 10:48 AM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>>
>>> Am 27.06.2016 um 10:41 schrieb Yann Ylavic <yl...@gmail.com>:
>>>
>>> On Mon, Jun 27, 2016 at 10:23 AM, Stefan Eissing <st...@eissing.org> wrote:
>>>> This looks nice for HTTP/1.1, but what about other protocols? Do I read it correctly that any pending data downstream will reopen the connection?
>>>
>>> Hmm, I did not think about mod_proxy_h2, but correct (I'd rather say
>>> upstream though, backend side).
>>> Are there cases where some backend data are available before the
>>> request is sent?
>>
>> In HTTP/2, yes. For example a PING frame might be waiting, or a
>> SETTINGS change. Most backends will have short timeouts for answers
>> to these (SETTINGS is expected to be ACKed soonish), but races may
>> happen. Also, HTTP/2 extensions with new frame types that need to be
>> ignored when not known may get received here.
>
> OK, so we probably should get rid of the call to
> ap_proxy_ssl_connection_cleanup() in proxy_http2_handler (and also in
> proxy_http_handler with my latest changes since the same check is now
> available in ap_proxy_check_connection).
I went ahead and committed r1750414 (resp. r1750416).
The possible issue if r1750414 were backported, is that without
r1750392 mod_proxy_http2 may not detect a TLS close notify before
reusing a backend connection.
If it's not backported, it may close a legitimate backend connection
with (pre-)available data...
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 27, 2016 at 10:48 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
>
>> Am 27.06.2016 um 10:41 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> On Mon, Jun 27, 2016 at 10:23 AM, Stefan Eissing <st...@eissing.org> wrote:
>>> This looks nice for HTTP/1.1, but what about other protocols? Do I read it correctly that any pending data downstream will reopen the connection?
>>
>> Hmm, I did not think about mod_proxy_h2, but correct (I'd rather say
>> upstream though, backend side).
>> Are there cases where some backend data are available before the
>> request is sent?
>
> In HTTP/2, yes. For example a PING frame might be waiting, or a
> SETTINGS change. Most backends will have short timeouts for answers
> to these (SETTINGS is expected to be ACKed soonish), but races may
> happen. Also, HTTP/2 extensions with new frame types that need to be
> ignored when not known may get received here.
OK, so we probably should get rid of the call to
ap_proxy_ssl_connection_cleanup() in proxy_http2_handler (and also in
proxy_http_handler with my latest changes since the same check is now
available in ap_proxy_check_connection).
Regards,
Yann.
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 27.06.2016 um 10:41 schrieb Yann Ylavic <yl...@gmail.com>:
>
> On Mon, Jun 27, 2016 at 10:23 AM, Stefan Eissing <st...@eissing.org> wrote:
>> This looks nice for HTTP/1.1, but what about other protocols? Do I read it correctly that any pending data downstream will reopen the connection?
>
> Hmm, I did not think about mod_proxy_h2, but correct (I'd rather say
> upstream though, backend side).
> Are there cases where some backend data are available before the
> request is sent?
In HTTP/2, yes. For example a PING frame might be waiting, or a SETTINGS change. Most backends will have short timeouts for answers to these (SETTINGS is expected to be ACKed soonish), but races may happen. Also, HTTP/2 extensions with new frame types that need to be ignored when not known may get received here.
Again, the added check is very good for HTTP/1, but not for protocols that already have request/response protection and delimiters. Can we see which proxy module is involved on the connection? Do we see the scheme there? That might be a way to whitelist the check.
-Stefan
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 27, 2016 at 10:23 AM, Stefan Eissing <st...@eissing.org> wrote:
> This looks nice for HTTP/1.1, but what about other protocols? Do I read it correctly that any pending data downstream will reopen the connection?
Hmm, I did not think about mod_proxy_h2, but correct (I'd rather say
upstream though, backend side).
Are there cases where some backend data are available before the
request is sent?
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 27, 2016 at 6:00 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> PR 57832 does not depend on this (race) condition, hence this change
> (r1750301, or actually its successor since I'll revert it for a less
> intrusive version) could be applied to 2.4.x without r1656259.
New commit is r1750392.
I'll let it linger in trunk (for review) before any backport proposal...
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy:
mod_proxy.h proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 27, 2016 at 3:17 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> Doesn't this depend on:
>
> trunk patch: http://svn.apache.org/r1656259
> http://svn.apache.org/r1656359 (CHANGES entry)
>
> which, in STATUS, is tagged as still being worked?
I don't think so, r1656259 is more about reducing the time frame
between the backend connection check and its effective reuse (for
requests with a body which may be long to read or retained by some
input filter).
PR 57832 does not depend on this (race) condition, hence this change
(r1750301, or actually its successor since I'll revert it for a less
intrusive version) could be applied to 2.4.x without r1656259.
No hurry though, this is mainly to mitigate a (potential) defect on
the backend side only, it could be included in > 2.4.23 IMHO.
Regards,
Yann.
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
Doesn't this depend on:
trunk patch: http://svn.apache.org/r1656259
http://svn.apache.org/r1656359 (CHANGES entry)
which, in STATUS, is tagged as still being worked?
> On Jun 27, 2016, at 4:23 AM, Stefan Eissing <st...@eissing.org> wrote:
>
> This looks nice for HTTP/1.1, but what about other protocols? Do I read it correctly that any pending data downstream will reopen the connection?
>
>> Am 27.06.2016 um 10:00 schrieb ylavic@apache.org:
>>
>> Author: ylavic
>> Date: Mon Jun 27 08:00:30 2016
>> New Revision: 1750301
>>
>> URL: http://svn.apache.org/viewvc?rev=1750301&view=rev
>> Log:
>> mod_proxy: don't reuse backend connections with data available before the
>> request is sent. PR 57832.
>>
>> Modified:
>> httpd/httpd/trunk/modules/proxy/mod_proxy.h
>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1750301&r1=1750300&r2=1750301&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Mon Jun 27 08:00:30 2016
>> @@ -271,6 +271,7 @@ typedef struct {
>> unsigned int inreslist:1; /* connection in apr_reslist? */
>> const char *uds_path; /* Unix domain socket path */
>> const char *ssl_hostname;/* Hostname (SNI) in use by SSL connection */
>> + apr_bucket_brigade *tmp_bb;
>> } proxy_conn_rec;
>>
>> typedef struct {
>>
>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1750301&r1=1750300&r2=1750301&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Mon Jun 27 08:00:30 2016
>> @@ -2487,7 +2487,7 @@ ap_proxy_determine_connection(apr_pool_t
>> #endif
>>
>> #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
>> -PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
>> +static int get_socket_connected(apr_socket_t *socket)
>> {
>> apr_pollfd_t pfds[1];
>> apr_status_t status;
>> @@ -2514,7 +2514,7 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
>> status = apr_socket_recvfrom(&unused, socket, APR_MSG_PEEK,
>> &buf[0], &len);
>> if (status == APR_SUCCESS && len)
>> - return 1;
>> + return 2;
>> else
>> return 0;
>> }
>> @@ -2525,7 +2525,7 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
>>
>> }
>> #else
>> -PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
>> +static int is_socket_connnected(apr_socket_t *socket)
>>
>> {
>> apr_size_t buffer_len = 1;
>> @@ -2544,12 +2544,19 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
>> || APR_STATUS_IS_ECONNRESET(socket_status)) {
>> return 0;
>> }
>> + else if (status == APR_SUCCESS && buffer_len) {
>> + return 2;
>> + }
>> else {
>> return 1;
>> }
>> }
>> #endif /* USE_ALTERNATE_IS_CONNECTED */
>>
>> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
>> +{
>> + return get_socket_connected(socket) != 0;
>> +}
>>
>> /*
>> * Send a HTTP CONNECT request to a forward proxy.
>> @@ -2716,7 +2723,35 @@ PROXY_DECLARE(int) ap_proxy_connect_back
>> (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
>>
>> if (conn->sock) {
>> - if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
>> + conn_rec *c = conn->connection;
>> + if (!c) {
>> + connected = get_socket_connected(conn->sock);
>> + }
>> + else {
>> + if (conn->tmp_bb == NULL) {
>> + conn->tmp_bb = apr_brigade_create(c->pool, c->bucket_alloc);
>> + }
>> + rv = ap_get_brigade(c->input_filters, conn->tmp_bb,
>> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
>> + if (rv == APR_SUCCESS) {
>> + apr_off_t len = 0;
>> + apr_brigade_length(conn->tmp_bb, 0, &len);
>> + if (len) {
>> + connected = 2;
>> + }
>> + else {
>> + connected = 1;
>> + }
>> + }
>> + else if (APR_STATUS_IS_EAGAIN(rv)) {
>> + connected = 1;
>> + }
>> + else {
>> + connected = 0;
>> + }
>> + apr_brigade_cleanup(conn->tmp_bb);
>> + }
>> + if (connected != 1) {
>> /* This clears conn->scpool (and associated data), so backup and
>> * restore any ssl_hostname for this connection set earlier by
>> * ap_proxy_determine_connection().
>> @@ -2728,9 +2763,17 @@ PROXY_DECLARE(int) ap_proxy_connect_back
>> }
>>
>> socket_cleanup(conn);
>> - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
>> - "%s: backend socket is disconnected.",
>> - proxy_function);
>> + if (!connected) {
>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
>> + "%s: backend socket is disconnected.",
>> + proxy_function);
>> + }
>> + else {
>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO()
>> + "%s: reusable backend connection is not empty: "
>> + "forcibly closed", proxy_function);
>> + connected = 0;
>> + }
>>
>> if (ssl_hostname[0]) {
>> conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
>>
>>
>
Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
Posted by Stefan Eissing <st...@eissing.org>.
This looks nice for HTTP/1.1, but what about other protocols? Do I read it correctly that any pending data downstream will reopen the connection?
> Am 27.06.2016 um 10:00 schrieb ylavic@apache.org:
>
> Author: ylavic
> Date: Mon Jun 27 08:00:30 2016
> New Revision: 1750301
>
> URL: http://svn.apache.org/viewvc?rev=1750301&view=rev
> Log:
> mod_proxy: don't reuse backend connections with data available before the
> request is sent. PR 57832.
>
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_proxy.h
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1750301&r1=1750300&r2=1750301&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Mon Jun 27 08:00:30 2016
> @@ -271,6 +271,7 @@ typedef struct {
> unsigned int inreslist:1; /* connection in apr_reslist? */
> const char *uds_path; /* Unix domain socket path */
> const char *ssl_hostname;/* Hostname (SNI) in use by SSL connection */
> + apr_bucket_brigade *tmp_bb;
> } proxy_conn_rec;
>
> typedef struct {
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1750301&r1=1750300&r2=1750301&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Mon Jun 27 08:00:30 2016
> @@ -2487,7 +2487,7 @@ ap_proxy_determine_connection(apr_pool_t
> #endif
>
> #if USE_ALTERNATE_IS_CONNECTED && defined(APR_MSG_PEEK)
> -PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> +static int get_socket_connected(apr_socket_t *socket)
> {
> apr_pollfd_t pfds[1];
> apr_status_t status;
> @@ -2514,7 +2514,7 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
> status = apr_socket_recvfrom(&unused, socket, APR_MSG_PEEK,
> &buf[0], &len);
> if (status == APR_SUCCESS && len)
> - return 1;
> + return 2;
> else
> return 0;
> }
> @@ -2525,7 +2525,7 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
>
> }
> #else
> -PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> +static int is_socket_connnected(apr_socket_t *socket)
>
> {
> apr_size_t buffer_len = 1;
> @@ -2544,12 +2544,19 @@ PROXY_DECLARE(int) ap_proxy_is_socket_co
> || APR_STATUS_IS_ECONNRESET(socket_status)) {
> return 0;
> }
> + else if (status == APR_SUCCESS && buffer_len) {
> + return 2;
> + }
> else {
> return 1;
> }
> }
> #endif /* USE_ALTERNATE_IS_CONNECTED */
>
> +PROXY_DECLARE(int) ap_proxy_is_socket_connected(apr_socket_t *socket)
> +{
> + return get_socket_connected(socket) != 0;
> +}
>
> /*
> * Send a HTTP CONNECT request to a forward proxy.
> @@ -2716,7 +2723,35 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
>
> if (conn->sock) {
> - if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> + conn_rec *c = conn->connection;
> + if (!c) {
> + connected = get_socket_connected(conn->sock);
> + }
> + else {
> + if (conn->tmp_bb == NULL) {
> + conn->tmp_bb = apr_brigade_create(c->pool, c->bucket_alloc);
> + }
> + rv = ap_get_brigade(c->input_filters, conn->tmp_bb,
> + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1);
> + if (rv == APR_SUCCESS) {
> + apr_off_t len = 0;
> + apr_brigade_length(conn->tmp_bb, 0, &len);
> + if (len) {
> + connected = 2;
> + }
> + else {
> + connected = 1;
> + }
> + }
> + else if (APR_STATUS_IS_EAGAIN(rv)) {
> + connected = 1;
> + }
> + else {
> + connected = 0;
> + }
> + apr_brigade_cleanup(conn->tmp_bb);
> + }
> + if (connected != 1) {
> /* This clears conn->scpool (and associated data), so backup and
> * restore any ssl_hostname for this connection set earlier by
> * ap_proxy_determine_connection().
> @@ -2728,9 +2763,17 @@ PROXY_DECLARE(int) ap_proxy_connect_back
> }
>
> socket_cleanup(conn);
> - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> - "%s: backend socket is disconnected.",
> - proxy_function);
> + if (!connected) {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> + "%s: backend socket is disconnected.",
> + proxy_function);
> + }
> + else {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO()
> + "%s: reusable backend connection is not empty: "
> + "forcibly closed", proxy_function);
> + connected = 0;
> + }
>
> if (ssl_hostname[0]) {
> conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
>
>