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
<[email protected]> wrote:
> We are talking about adding this to trunk first, right? ^^
>
>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing <[email protected]>:
>>
>> I believe so. Highly experimental and all such...
>>
>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <[email protected]>:
>>>
>>> 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
>>> <[email protected]> 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 <[email protected]>:
>>>>>
>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <[email protected]> 
>>>>> 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?
>>>>
>>
>

Reply via email to