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