Commited in r1750505.

> Am 28.06.2016 um 15:13 schrieb Yann Ylavic <[email protected]>:
> 
> 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
> <[email protected]> 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 <[email protected]>:
>>> 
>>> Patch as a file attached.
>>> 
>>> On Tue, Jun 28, 2016 at 1:48 PM, Yann Ylavic <[email protected]> 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 <[email protected]> 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
>>>>> <[email protected]> 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 <[email protected]>:
>>>>>>> 
>>>>>>> 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?
>>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>> <httpd-2.4.x-mod_proxy_http2-ssl_reuse_cleanup.patch>
>> 
>> 

Reply via email to