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
httpd-2.4.x-mod_proxy_http2-ssl_reuse_cleanup-v2.diff
Description: Binary data
> Am 28.06.2016 um 13:49 schrieb Yann Ylavic <ylavic....@gmail.com>:
>
> Patch as a file attached.
>
> On Tue, Jun 28, 2016 at 1:48 PM, Yann Ylavic <ylavic....@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 <ylavic....@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
>>> <stefan.eiss...@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 <ylavic....@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
>>>>> <stefan.eiss...@greenbytes.de> wrote:
>>>>>> We are talking about adding this to trunk first, right? ^^
>>>>>>
>>>>>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing
>>>>>>> <stefan.eiss...@greenbytes.de>:
>>>>>>>
>>>>>>> I believe so. Highly experimental and all such...
>>>>>>>
>>>>>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic <ylavic....@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
>>>>>>>> <stefan.eiss...@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 <ylavic....@gmail.com>:
>>>>>>>>>>
>>>>>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic <ylavic....@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>
