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? >>>>>> >>>> >>> >
