From 090da4994ec5049e567ed3ebd98af22b2d6e5ee1 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Mon, 18 Feb 2019 13:14:53 +0100 Subject: [PATCH] * Fixed an issue where a proxy_http2 handler entered an infinit loop when encountering certain errors on the backend connection. See . --- ChangeLog | 6 ++++ configure.ac | 2 +- mod_http2/h2_version.h | 4 +-- mod_http2/mod_proxy_http2.c | 55 +++++++++++++++++++++---------------- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/ChangeLog b/ChangeLog index e9b86cb5..84e41e70 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +v1.12.4 +-------------------------------------------------------------------------------- + * Fixed an issue where a proxy_http2 handler entered an infinit loop when encountering + certain errors on the backend connection. + See . + v1.12.3 -------------------------------------------------------------------------------- * fixed bug in nghttp output parsing that filters now Frames sent/received inbetween diff --git a/configure.ac b/configure.ac index dabb1e56..d825ad98 100644 --- a/configure.ac +++ b/configure.ac @@ -14,7 +14,7 @@ # AC_PREREQ([2.69]) -AC_INIT([mod_http2], [1.12.3], [stefan.eissing@greenbytes.de]) +AC_INIT([mod_http2], [1.12.4], [stefan.eissing@greenbytes.de]) LT_PREREQ([2.2.6]) LT_INIT() diff --git a/mod_http2/h2_version.h b/mod_http2/h2_version.h index b6a56f2d..f51057ee 100644 --- a/mod_http2/h2_version.h +++ b/mod_http2/h2_version.h @@ -27,7 +27,7 @@ * @macro * Version number of the http2 module as c string */ -#define MOD_HTTP2_VERSION "1.12.3-git" +#define MOD_HTTP2_VERSION "1.12.4-git" /** * @macro @@ -35,7 +35,7 @@ * release. This is a 24 bit number with 8 bits for major number, 8 bits * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203. */ -#define MOD_HTTP2_VERSION_NUM 0x010c03 +#define MOD_HTTP2_VERSION_NUM 0x010c04 #endif /* mod_h2_h2_version_h */ diff --git a/mod_http2/mod_proxy_http2.c b/mod_http2/mod_proxy_http2.c index 4ac93251..35c1f547 100644 --- a/mod_http2/mod_proxy_http2.c +++ b/mod_http2/mod_proxy_http2.c @@ -362,58 +362,65 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) { "eng(%s): run session %s", ctx->engine_id, ctx->session->id); ctx->session->user_data = ctx; - while (!ctx->owner->aborted) { + while (1) { + if (ctx->owner->aborted) { + status = APR_ECONNABORTED; + goto out; + } + if (APR_SUCCESS == h2_proxy_fifo_try_pull(ctx->requests, (void**)&r)) { add_request(ctx->session, r); } - status = h2_proxy_session_process(ctx->session); if (status == APR_SUCCESS) { - apr_status_t s2; - /* ongoing processing, call again */ + /* ongoing processing, check if we have room to handle more streams, + * maybe the remote side changed their limit */ if (ctx->session->remote_max_concurrent > 0 && ctx->session->remote_max_concurrent != ctx->capacity) { ctx->capacity = H2MIN((int)ctx->session->remote_max_concurrent, h2_proxy_fifo_capacity(ctx->requests)); } - s2 = next_request(ctx, 0); - if (s2 == APR_ECONNABORTED) { - /* master connection gone */ - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, s2, ctx->owner, - APLOGNO(03374) "eng(%s): pull request", - ctx->engine_id); - /* give notice that we're leaving and cancel all ongoing - * streams. */ - next_request(ctx, 1); - h2_proxy_session_cancel_all(ctx->session); - h2_proxy_session_process(ctx->session); - status = ctx->r_status = APR_SUCCESS; - break; + /* try to pull more request, if our capacity allows it */ + if (APR_ECONNABORTED == next_request(ctx, 0)) { + status = APR_ECONNABORTED; + goto out; } + /* If we have no ongoing streams and nothing in our queue, we + * terminate processing and return to our caller. */ if ((h2_proxy_fifo_count(ctx->requests) == 0) && h2_proxy_ihash_empty(ctx->session->streams)) { - break; + goto out; } } else { - /* end of processing, maybe error */ + /* Encountered an error during session processing */ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, APLOGNO(03375) "eng(%s): end of session %s", ctx->engine_id, ctx->session->id); - /* - * Any open stream of that session needs to + /* Any open stream of that session needs to * a) be reopened on the new session iff safe to do so * b) reported as done (failed) otherwise */ h2_proxy_session_cleanup(ctx->session, session_req_done); - break; + goto out; } } +out: + if (APR_ECONNABORTED == status) { + /* master connection gone */ + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, + APLOGNO(03374) "eng(%s): master connection gone", ctx->engine_id); + /* give notice that we're leaving and cancel all ongoing streams. */ + next_request(ctx, 1); + h2_proxy_session_cancel_all(ctx->session); + h2_proxy_session_process(ctx->session); + status = ctx->r_status = APR_SUCCESS; + } + ctx->session->user_data = NULL; ctx->session = NULL; - return status; } @@ -641,7 +648,7 @@ static int proxy_http2_handler(request_rec *r, ctx->p_conn = NULL; } - /* Any requests will still have need to fail */ + /* Any requests we still have need to fail */ while (APR_SUCCESS == h2_proxy_fifo_try_pull(ctx->requests, (void**)&r)) { request_done(ctx, r, HTTP_SERVICE_UNAVAILABLE, 1); }