[Pkg-apache-commits] r1166 - in /trunk/apache2: changelog patches/00list patches/080_mod_reqtimeout_fixes.dpatch

sf at alioth.debian.org sf at alioth.debian.org
Wed Mar 10 19:54:52 UTC 2010


Author: sf
Date: Wed Mar 10 19:54:42 2010
New Revision: 1166

URL: http://svn.debian.org/wsvn/pkg-apache/?sc=1&rev=1166
Log:
mod_reqtimeout: Various bug fixes, including:
- Don't mess up timeouts of mod_proxy's backend connections.

Added:
    trunk/apache2/patches/080_mod_reqtimeout_fixes.dpatch   (with props)
Modified:
    trunk/apache2/changelog
    trunk/apache2/patches/00list

Modified: trunk/apache2/changelog
URL: http://svn.debian.org/wsvn/pkg-apache/trunk/apache2/changelog?rev=1166&op=diff
==============================================================================
--- trunk/apache2/changelog (original)
+++ trunk/apache2/changelog Wed Mar 10 19:54:42 2010
@@ -1,3 +1,11 @@
+apache2 (2.2.15-2) UNRELEASED; urgency=low
+
+  * mod_reqtimeout: Various bug fixes, including:
+    - Don't mess up timeouts of mod_proxy's backend connections.
+      Closes: #573163
+
+ -- Stefan Fritsch <sf at debian.org>  Wed, 10 Mar 2010 20:51:34 +0100
+
 apache2 (2.2.15-1) unstable; urgency=low
 
   * New upstream version:

Modified: trunk/apache2/patches/00list
URL: http://svn.debian.org/wsvn/pkg-apache/trunk/apache2/patches/00list?rev=1166&op=diff
==============================================================================
--- trunk/apache2/patches/00list (original)
+++ trunk/apache2/patches/00list Wed Mar 10 19:54:42 2010
@@ -23,6 +23,7 @@
 076_apxs2_a2enmod.dpatch
 077_CacheIgnoreURLSessionIdentifiers.dpatch
 079_polish_translation.dpatch
+080_mod_reqtimeout_fixes.dpatch
 099_config_guess_sub_update
 200_cp_suexec.dpatch
 201_build_suexec-custom.dpatch

Added: trunk/apache2/patches/080_mod_reqtimeout_fixes.dpatch
URL: http://svn.debian.org/wsvn/pkg-apache/trunk/apache2/patches/080_mod_reqtimeout_fixes.dpatch?rev=1166&op=file
==============================================================================
--- trunk/apache2/patches/080_mod_reqtimeout_fixes.dpatch (added)
+++ trunk/apache2/patches/080_mod_reqtimeout_fixes.dpatch Wed Mar 10 19:54:42 2010
@@ -1,0 +1,340 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+##
+## DP: r921378 and r921526 from upstream trunk:
+## DP: - Move initialization to process_connection hook, right before
+## DP:   ap_process_http_request.  This ensures that we are not inserted for other
+## DP:   protocol handlers (like mod_ftp) and mod_proxy's backend connections.
+## DP: - Enforce request timeout even for AP_MODE_GETLINE.
+## DP: - Abort connection when timeout occurs. Otherwise the thread may be blocked
+## DP:   another 30s doing a lingering close. The downside is that the client will
+## DP:   not receive the error message.
+ at DPATCH@
+diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' trunk~/modules/filters/mod_reqtimeout.c trunk/modules/filters/mod_reqtimeout.c
+--- trunk~/modules/filters/mod_reqtimeout.c	2010-03-10 20:46:14.000000000 +0100
++++ trunk/modules/filters/mod_reqtimeout.c	2010-03-10 20:46:40.284322045 +0100
+@@ -20,9 +20,11 @@
+ #include "http_connection.h"
+ #include "http_protocol.h"
+ #include "http_log.h"
++#include "http_core.h"
+ #include "util_filter.h"
+ #define APR_WANT_STRFUNC
+ #include "apr_strings.h"
++#include "apr_support.h"
+ 
+ module AP_MODULE_DECLARE_DATA reqtimeout_module;
+ 
+@@ -38,6 +40,7 @@
+     apr_time_t body_rate_factor;
+ } reqtimeout_srv_cfg;
+ 
++/* this struct is used both as conn_config and as filter context */
+ typedef struct
+ {
+     apr_time_t timeout_at;
+@@ -47,14 +50,11 @@
+     int new_max_timeout;
+     int in_keep_alive;
+     char *type;
++    apr_socket_t *socket;
+     apr_time_t rate_factor;
++    apr_bucket_brigade *tmpbb;
+ } reqtimeout_con_cfg;
+ 
+-typedef struct
+-{
+-    apr_socket_t *socket;
+-} reqtimeout_ctx;
+-
+ static const char *const reqtimeout_filter_name = "reqtimeout";
+ 
+ static void extend_timeout(reqtimeout_con_cfg *ccfg, apr_bucket_brigade *bb)
+@@ -74,24 +74,60 @@
+     }
+ }
+ 
++static apr_status_t check_time_left(reqtimeout_con_cfg *ccfg,
++                                    apr_time_t *time_left_p)
++{
++    *time_left_p = ccfg->timeout_at - apr_time_now();
++    if (*time_left_p <= 0)
++        return APR_TIMEUP;
++    
++    if (*time_left_p < apr_time_from_sec(1)) {
++        *time_left_p = apr_time_from_sec(1);
++    }
++    return APR_SUCCESS;
++}
++
++static apr_status_t have_lf_or_eos(apr_bucket_brigade *bb)
++{
++    apr_bucket *b = APR_BRIGADE_LAST(bb);
++
++    for ( ; b != APR_BRIGADE_SENTINEL(bb) ; b = APR_BUCKET_PREV(b) ) {
++    	const char *str;
++    	apr_size_t len;
++    	apr_status_t rv;
++
++        if (APR_BUCKET_IS_EOS(b))
++            return APR_SUCCESS;
++
++        if (APR_BUCKET_IS_METADATA(b))
++            continue;
++
++        rv = apr_bucket_read(b, &str, &len, APR_BLOCK_READ);
++        if (rv != APR_SUCCESS)
++            return rv;
++
++        if (len == 0)
++            continue;
++
++        if (str[len-1] == APR_ASCII_LF)
++            return APR_SUCCESS;
++    }
++    return APR_INCOMPLETE;
++}
++
++
++#define MIN(x,y) ((x) < (y) ? (x) : (y))
+ static apr_status_t reqtimeout_filter(ap_filter_t *f,
+                                       apr_bucket_brigade *bb,
+                                       ap_input_mode_t mode,
+                                       apr_read_type_e block,
+                                       apr_off_t readbytes)
+ {
+-    reqtimeout_ctx *ctx;
+     apr_time_t time_left;
+     apr_time_t now;
+     apr_status_t rv;
+     apr_interval_time_t saved_sock_timeout = -1;
+-    reqtimeout_con_cfg *ccfg;
+-
+-    ctx = f->ctx;
+-    AP_DEBUG_ASSERT(ctx != NULL);
+-
+-    ccfg = ap_get_module_config(f->c->conn_config, &reqtimeout_module);
+-    AP_DEBUG_ASSERT(ccfg != NULL);
++    reqtimeout_con_cfg *ccfg = f->ctx;
+ 
+     if (ccfg->in_keep_alive) {
+         /* For this read, the normal keep-alive timeout must be used */
+@@ -114,13 +150,28 @@
+         return ap_get_brigade(f->next, bb, mode, block, readbytes);
+     }
+ 
+-    time_left = ccfg->timeout_at - now;
+-    if (time_left <= 0) {
+-        ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, f->c,
+-                      "Request %s read timeout", ccfg->type);
+-        return APR_TIMEUP;
++    if (!ccfg->socket) {
++        core_net_rec *net_rec;
++        ap_filter_t *core_in = f->next;
++
++        while (core_in && core_in->frec != ap_core_input_filter_handle)
++            core_in = core_in->next;
++
++        if (!core_in) {
++            ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, f->c,
++                          "mod_reqtimeout: Can't get socket "
++                          "handle from core_input_filter");
++            ap_remove_input_filter(f);
++            return ap_get_brigade(f->next, bb, mode, block, readbytes);
++        }
++        net_rec = core_in->ctx;
++        ccfg->socket = net_rec->client_socket;
+     }
+ 
++    rv = check_time_left(ccfg, &time_left);
++    if (rv != APR_SUCCESS)
++        goto out;
++
+     if (block == APR_NONBLOCK_READ || mode == AP_MODE_INIT
+         || mode == AP_MODE_EATCRLF) {
+         rv = ap_get_brigade(f->next, bb, mode, block, readbytes);
+@@ -130,41 +181,104 @@
+         return rv;
+     }
+ 
+-    if (time_left < apr_time_from_sec(1)) {
+-        time_left = apr_time_from_sec(1);
+-    }
++    rv = apr_socket_timeout_get(ccfg->socket, &saved_sock_timeout);
++    AP_DEBUG_ASSERT(rv == APR_SUCCESS);
+ 
+-    rv = apr_socket_timeout_get(ctx->socket, &saved_sock_timeout);
++    rv = apr_socket_timeout_set(ccfg->socket, MIN(time_left, saved_sock_timeout));
+     AP_DEBUG_ASSERT(rv == APR_SUCCESS);
+ 
+-    if (saved_sock_timeout >= time_left) {
+-        rv = apr_socket_timeout_set(ctx->socket, time_left);
+-        AP_DEBUG_ASSERT(rv == APR_SUCCESS);
+-    }
+-    else {
+-        saved_sock_timeout = -1;
+-    }
++    if (mode == AP_MODE_GETLINE) {
++        /*
++         * For a blocking AP_MODE_GETLINE read, apr_brigade_split_line()
++         * would loop until a whole line has been read. As this would make it
++         * impossible to enforce a total timeout, we only do non-blocking
++         * reads.
++         */
++        apr_size_t remaining = HUGE_STRING_LEN;
++        do {
++            apr_off_t bblen;
+ 
+-    rv = ap_get_brigade(f->next, bb, mode, block, readbytes);
++            rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE, APR_NONBLOCK_READ, remaining);
++            if (APR_STATUS_IS_EAGAIN(rv)) {
++                rv = APR_SUCCESS;
++            }
++            else if (rv != APR_SUCCESS) {
++                break;
++            }
+ 
+-    if (saved_sock_timeout != -1) {
+-        apr_socket_timeout_set(ctx->socket, saved_sock_timeout);
+-    }
++            if (!APR_BRIGADE_EMPTY(bb)) {
++                rv = have_lf_or_eos(bb);
++                if (rv != APR_INCOMPLETE) {
++                    break;
++                }
+ 
+-    if (ccfg->min_rate > 0 && rv == APR_SUCCESS) {
+-        extend_timeout(ccfg, bb);
++                if (ccfg->min_rate > 0) {
++                    extend_timeout(ccfg, bb);
++                }
++
++                rv = apr_brigade_length(bb, 1, &bblen);
++                if (rv != APR_SUCCESS) {
++                    break;
++                }
++                remaining -= bblen;
++                if (remaining <= 0) {
++                    break;
++                }
++
++                /* Haven't got a whole line yet, save what we have ... */
++                if (!ccfg->tmpbb) {
++                    ccfg->tmpbb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);
++                }
++                APR_BRIGADE_CONCAT(ccfg->tmpbb, bb);
++            }
++
++            /* ... and wait for more */
++            rv = apr_wait_for_io_or_timeout(NULL, ccfg->socket, 1);
++            if (rv != APR_SUCCESS)
++                break;
++
++            rv = check_time_left(ccfg, &time_left);
++            if (rv != APR_SUCCESS)
++                break;
++
++            rv = apr_socket_timeout_set(ccfg->socket,
++                                   MIN(time_left, saved_sock_timeout));
++            AP_DEBUG_ASSERT(rv == APR_SUCCESS);
++
++        } while (1);
++
++        if (ccfg->tmpbb)
++            APR_BRIGADE_PREPEND(bb, ccfg->tmpbb);
++
++    }
++    else {
++        /* mode != AP_MODE_GETLINE */
++        rv = ap_get_brigade(f->next, bb, mode, block, readbytes);
++        if (ccfg->min_rate > 0 && rv == APR_SUCCESS) {
++            extend_timeout(ccfg, bb);
++        }
+     }
+ 
++    apr_socket_timeout_set(ccfg->socket, saved_sock_timeout);
++
++out:
+     if (rv == APR_TIMEUP) {
+         ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, f->c,
+                       "Request %s read timeout", ccfg->type);
++        /*
++         * If we allow lingering close, the client may keep this
++         * process/thread busy for another 30s (MAX_SECS_TO_LINGER).
++         * Therefore we have to abort the connection. The downside is
++         * that the client will most likely not receive the error
++         * message.
++         */
++        f->c->aborted = 1;
+     }
+     return rv;
+ }
+ 
+-static int reqtimeout_pre_conn(conn_rec *c, void *csd)
++static int reqtimeout_init(conn_rec *c)
+ {
+-    reqtimeout_ctx *ctx;
+     reqtimeout_con_cfg *ccfg;
+     reqtimeout_srv_cfg *cfg;
+ 
+@@ -173,12 +287,9 @@
+     AP_DEBUG_ASSERT(cfg != NULL);
+     if (cfg->header_timeout <= 0 && cfg->body_timeout <= 0) {
+         /* not configured for this vhost */
+-        return OK;
++        return DECLINED;
+     }
+ 
+-    ctx = apr_pcalloc(c->pool, sizeof(reqtimeout_ctx));
+-    ctx->socket = csd;
+-
+     ccfg = apr_pcalloc(c->pool, sizeof(reqtimeout_con_cfg));
+     ccfg->new_timeout = cfg->header_timeout;
+     ccfg->new_max_timeout = cfg->header_max_timeout;
+@@ -187,8 +298,9 @@
+     ccfg->rate_factor = cfg->header_rate_factor;
+     ap_set_module_config(c->conn_config, &reqtimeout_module, ccfg);
+ 
+-    ap_add_input_filter("reqtimeout", ctx, NULL, c);
+-    return OK;
++    ap_add_input_filter("reqtimeout", ccfg, NULL, c);
++    /* we are not handling the connection, we just do initialization */
++    return DECLINED;
+ }
+ 
+ static int reqtimeout_after_headers(request_rec *r)
+@@ -198,7 +310,7 @@
+         ap_get_module_config(r->connection->conn_config, &reqtimeout_module);
+ 
+     if (ccfg == NULL) {
+-        /* not configured for this vhost */
++        /* not configured for this connection */
+         return OK;
+     }
+ 
+@@ -224,7 +336,7 @@
+         ap_get_module_config(r->connection->conn_config, &reqtimeout_module);
+ 
+     if (ccfg == NULL) {
+-        /* not configured for this vhost */
++        /* not configured for this connection */
+         return OK;
+     }
+ 
+@@ -406,7 +518,16 @@
+      */
+     ap_register_input_filter(reqtimeout_filter_name, reqtimeout_filter, NULL,
+                              AP_FTYPE_CONNECTION + 8);
+-    ap_hook_pre_connection(reqtimeout_pre_conn, NULL, NULL, APR_HOOK_MIDDLE);
++
++    /*
++     * mod_reqtimeout needs to be called before ap_process_http_request (which
++     * is run at APR_HOOK_REALLY_LAST) but after all other protocol modules.
++     * This ensures that it only influences normal http connections and not
++     * e.g. mod_ftp. Also, if mod_reqtimeout used the pre_connection hook, it
++     * would be inserted on mod_proxy's backend connections.
++     */
++    ap_hook_process_connection(reqtimeout_init, NULL, NULL, APR_HOOK_LAST);
++
+     ap_hook_post_read_request(reqtimeout_after_headers, NULL, NULL,
+                               APR_HOOK_MIDDLE);
+     ap_hook_log_transaction(reqtimeout_after_body, NULL, NULL,

Propchange: trunk/apache2/patches/080_mod_reqtimeout_fixes.dpatch
------------------------------------------------------------------------------
    svn:executable = *




More information about the Pkg-apache-commits mailing list