From: William A. Rowe Jr Date: Wed, 8 Jul 2015 17:38:26 +0000 (+0000) Subject: mod_reqtimeout: Don't let pipelining checks and keep-alive times interfere X-Git-Tag: 2.4.16~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6defaea91c4d54113aabfc0a609f4dd48d944a85;p=thirdparty%2Fapache%2Fhttpd.git mod_reqtimeout: Don't let pipelining checks and keep-alive times interfere with the timeouts computed for subsequent requests. PR 56729. Submitted by: covener, ylavic Backports: 1621453, 1641376, 1689325 Reviewed by: ylavic, wrowe, covener git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1689922 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index ccc097fdba9..deffbee9b33 100644 --- a/CHANGES +++ b/CHANGES @@ -8,6 +8,10 @@ Changes with Apache 2.4.16 *) mod_alias: Revert expression parser support for Alias, ScriptAlias and Redirect due to a regression (introduced in 2.4.13, not released). + *) mod_reqtimeout: Don't let pipelining checks and keep-alive times interfere + with the timeouts computed for subsequent requests. PR 56729. + [Eric Covener, Yann Ylavic] + Changes with Apache 2.4.15 *) mod_ext_filter, mod_charset_lite: Avoid inadvertent filtering of protocol diff --git a/STATUS b/STATUS index cbe596b2b7b..eddb3d57c13 100644 --- a/STATUS +++ b/STATUS @@ -114,16 +114,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: 2.4.x patch: trunk works (modulo CHANGES) +1: jailletc36, ylavic, covener - *) mod_reqtimeout: Don't let pipelining checks and keep-alive times interfere - with the timeouts computed for subsequent requests. PR 56729. - trunk patch: http://svn.apache.org/r1621453 - http://svn.apache.org/r1641376 - http://svn.apache.org/r1689325 - 2.4.x patch: trunk works (modulo CHANGES) - (http://people.apache.org/~ylavic/httpd-2.4.x-reqtimeout_pre_read_request.patch - for global review) - +1: ylavic, wrowe, covener - PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/modules/filters/mod_reqtimeout.c b/modules/filters/mod_reqtimeout.c index b19d9512503..bc7789964ad 100644 --- a/modules/filters/mod_reqtimeout.c +++ b/modules/filters/mod_reqtimeout.c @@ -177,10 +177,27 @@ static apr_status_t reqtimeout_filter(ap_filter_t *f, apr_interval_time_t saved_sock_timeout = UNSET; reqtimeout_con_cfg *ccfg = f->ctx; + if (block == APR_NONBLOCK_READ && mode == AP_MODE_SPECULATIVE) { + /* The source of these above us in the core is check_pipeline(), which + * is between requests but before this filter knows to reset timeouts + * during pre_read_request(). If they appear elsewhere, just don't + * check or extend the time since they won't block and we'll see the + * bytes again later + */ + return ap_get_brigade(f->next, bb, mode, block, readbytes); + } + if (ccfg->in_keep_alive) { - /* For this read, the normal keep-alive timeout must be used */ + /* For this read[_request line()], wait for the first byte using the + * normal keep-alive timeout (hence don't take this expected idle time + * into account to setup the connection expiry below). + */ ccfg->in_keep_alive = 0; - return ap_get_brigade(f->next, bb, mode, block, readbytes); + rv = ap_get_brigade(f->next, bb, AP_MODE_SPECULATIVE, block, 1); + if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) { + return rv; + } + apr_brigade_cleanup(bb); } if (ccfg->new_timeout > 0) { @@ -301,7 +318,12 @@ static apr_status_t reqtimeout_filter(ap_filter_t *f, else { /* mode != AP_MODE_GETLINE */ rv = ap_get_brigade(f->next, bb, mode, block, readbytes); - if (ccfg->min_rate > 0 && rv == APR_SUCCESS) { + /* Don't extend the timeout in speculative mode, wait for + * the real (relevant) bytes to be asked later, within the + * currently alloted time. + */ + if (ccfg->min_rate > 0 && rv == APR_SUCCESS + && mode != AP_MODE_SPECULATIVE) { extend_timeout(ccfg, bb); } } @@ -349,11 +371,32 @@ static int reqtimeout_init(conn_rec *c) ap_set_module_config(c->conn_config, &reqtimeout_module, ccfg); ap_add_input_filter(reqtimeout_filter_name, ccfg, NULL, c); } - else { - /* subsequent request under event-like MPM */ - memset(ccfg, 0, sizeof(reqtimeout_con_cfg)); + + /* we are not handling the connection, we just do initialization */ + return DECLINED; +} + +static void reqtimeout_before_header(request_rec *r, conn_rec *c) +{ + reqtimeout_srv_cfg *cfg; + reqtimeout_con_cfg *ccfg = + ap_get_module_config(c->conn_config, &reqtimeout_module); + + if (ccfg == NULL) { + /* not configured for this connection */ + return; } + cfg = ap_get_module_config(c->base_server->module_config, + &reqtimeout_module); + AP_DEBUG_ASSERT(cfg != NULL); + + /* (Re)set the state for this new request, but ccfg->socket and + * ccfg->tmpbb which have the lifetime of the connection. + */ + ccfg->timeout_at = 0; + ccfg->max_timeout_at = 0; + ccfg->in_keep_alive = (c->keepalives > 0); ccfg->type = "header"; if (cfg->header_timeout != UNSET) { ccfg->new_timeout = cfg->header_timeout; @@ -367,12 +410,9 @@ static int reqtimeout_init(conn_rec *c) ccfg->min_rate = MRT_DEFAULT_HEADER_MIN_RATE; ccfg->rate_factor = default_header_rate_factor; } - - /* we are not handling the connection, we just do initialization */ - return DECLINED; } -static int reqtimeout_after_headers(request_rec *r) +static int reqtimeout_before_body(request_rec *r) { reqtimeout_srv_cfg *cfg; reqtimeout_con_cfg *ccfg = @@ -404,41 +444,6 @@ static int reqtimeout_after_headers(request_rec *r) return OK; } -static int reqtimeout_after_body(request_rec *r) -{ - reqtimeout_srv_cfg *cfg; - reqtimeout_con_cfg *ccfg = - ap_get_module_config(r->connection->conn_config, &reqtimeout_module); - - if (ccfg == NULL) { - /* not configured for this connection */ - return OK; - } - - cfg = ap_get_module_config(r->connection->base_server->module_config, - &reqtimeout_module); - AP_DEBUG_ASSERT(cfg != NULL); - - ccfg->timeout_at = 0; - ccfg->max_timeout_at = 0; - ccfg->in_keep_alive = 1; - ccfg->type = "header"; - if (ccfg->new_timeout != UNSET) { - ccfg->new_timeout = cfg->header_timeout; - ccfg->new_max_timeout = cfg->header_max_timeout; - ccfg->min_rate = cfg->header_min_rate; - ccfg->rate_factor = cfg->header_rate_factor; - } - else { - ccfg->new_timeout = MRT_DEFAULT_HEADER_TIMEOUT; - ccfg->new_max_timeout = MRT_DEFAULT_HEADER_MAX_TIMEOUT; - ccfg->min_rate = MRT_DEFAULT_HEADER_MIN_RATE; - ccfg->rate_factor = default_header_rate_factor; - } - - return OK; -} - static void *reqtimeout_create_srv_config(apr_pool_t *p, server_rec *s) { reqtimeout_srv_cfg *cfg = apr_pcalloc(p, sizeof(reqtimeout_srv_cfg)); @@ -609,10 +614,10 @@ static void reqtimeout_hooks(apr_pool_t *pool) */ ap_hook_process_connection(reqtimeout_init, NULL, NULL, APR_HOOK_LAST); - ap_hook_post_read_request(reqtimeout_after_headers, NULL, NULL, + ap_hook_pre_read_request(reqtimeout_before_header, NULL, NULL, + APR_HOOK_MIDDLE); + ap_hook_post_read_request(reqtimeout_before_body, NULL, NULL, APR_HOOK_MIDDLE); - ap_hook_log_transaction(reqtimeout_after_body, NULL, NULL, - APR_HOOK_MIDDLE); #if MRT_DEFAULT_HEADER_MIN_RATE > 0 default_header_rate_factor = apr_time_from_sec(1) / MRT_DEFAULT_HEADER_MIN_RATE;