diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 19d13d862a..d0d8c889eb 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -532,11 +532,10 @@ * dav_success_proppatch. * 20160608.4 (2.5.0-dev) Add dav_acl_provider, dav_acl_provider_register * dav_get_acl_providers. - * 20160608.5 (2.5.0-dev) Add ap_proxy_check_connection(), and tmp_bb to - * struct proxy_conn_rec. + * 20160608.5 (2.5.0-dev) Add ap_proxy_check_backend(), and tmp_bb to struct + * proxy_conn_rec. * 20160608.6 (2.5.0-dev) Add ap_scan_http_field_content, ap_scan_http_token * and ap_get_http_token - * 20160608.7 (2.5.0-dev) Add ap_check_pipeline(). */ #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */ @@ -544,7 +543,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20160608 #endif -#define MODULE_MAGIC_NUMBER_MINOR 7 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 6 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/include/http_request.h b/include/http_request.h index 8107a4247f..4d82c8a3fe 100644 --- a/include/http_request.h +++ b/include/http_request.h @@ -350,21 +350,6 @@ void ap_process_async_request(request_rec *r); */ AP_DECLARE(void) ap_die(int type, request_rec *r); -/** - * Check whether a connection is still established and has data available, - * optionnaly consuming blank lines ([CR]LF). - * @param c The current connection - * @param bb The brigade to filter - * @param max_blank_lines Max number of blank lines to consume, or zero - * to consider them as data (single read). - * @return APR_SUCCESS: connection ready and empty, - * APR_ENOTEMPTY: connection ready with data available, - * APR_NOTFOUND: too much blank lines, - * APR_E*: connection/general error. - */ -AP_DECLARE(apr_status_t) ap_check_pipeline(conn_rec *c, apr_bucket_brigade *bb, - unsigned int max_blank_lines); - /* Hooks */ /** diff --git a/modules/http/http_request.c b/modules/http/http_request.c index 720eb0010c..ee50b4232a 100644 --- a/modules/http/http_request.c +++ b/modules/http/http_request.c @@ -229,45 +229,41 @@ AP_DECLARE(void) ap_die(int type, request_rec *r) ap_die_r(type, r, r->status); } -AP_DECLARE(apr_status_t) ap_check_pipeline(conn_rec *c, apr_bucket_brigade *bb, - unsigned int max_blank_lines) +static void check_pipeline(conn_rec *c, apr_bucket_brigade *bb) { - apr_status_t rv = APR_EOF; + apr_status_t rv; + int num_blank_lines = DEFAULT_LIMIT_BLANK_LINES; ap_input_mode_t mode = AP_MODE_SPECULATIVE; - unsigned int num_blank_lines = 0; apr_size_t cr = 0; char buf[2]; + c->data_in_input_filters = 0; while (c->keepalive != AP_CONN_CLOSE && !c->aborted) { apr_size_t len = cr + 1; apr_brigade_cleanup(bb); rv = ap_get_brigade(c->input_filters, bb, mode, APR_NONBLOCK_READ, len); - if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb) || !max_blank_lines) { + if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) { + /* + * Error or empty brigade: There is no data present in the input + * filter + */ if (mode == AP_MODE_READBYTES) { /* Unexpected error, stop with this connection */ ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(02967) "Can't consume pipelined empty lines"); c->keepalive = AP_CONN_CLOSE; - rv = APR_EGENERAL; - } - else if (rv == APR_SUCCESS && !APR_BRIGADE_EMPTY(bb)) { - /* Single read asked, data available */ - rv = APR_ENOTEMPTY; - } - else if (APR_STATUS_IS_EAGAIN(rv)) { - /* Pipe is empty and ready */ - rv = APR_SUCCESS; - } - else { - /* Pipe is dead, rv up to date */ - c->keepalive = AP_CONN_CLOSE; } break; } - /* Lookup and consume blank lines */ + /* Ignore trailing blank lines (which must not be interpreted as + * pipelined requests) up to the limit, otherwise we would block + * on the next read without flushing data, and hence possibly delay + * pending response(s) until the next/real request comes in or the + * keepalive timeout expires. + */ rv = apr_brigade_flatten(bb, buf, &len); if (rv != APR_SUCCESS || len != cr + 1) { int log_level; @@ -275,17 +271,14 @@ AP_DECLARE(apr_status_t) ap_check_pipeline(conn_rec *c, apr_bucket_brigade *bb, /* Unexpected error, stop with this connection */ c->keepalive = AP_CONN_CLOSE; log_level = APLOG_ERR; - if (rv == APR_SUCCESS) { - rv = APR_EGENERAL; - } } else { /* Let outside (non-speculative/blocking) read determine * where this possible failure comes from (metadata, * morphed EOF socket => empty bucket? debug only here). */ + c->data_in_input_filters = 1; log_level = APLOG_DEBUG; - rv = APR_ENOTEMPTY; } ap_log_cerror(APLOG_MARK, log_level, rv, c, APLOGNO(02968) "Can't check pipelined data"); @@ -293,49 +286,40 @@ AP_DECLARE(apr_status_t) ap_check_pipeline(conn_rec *c, apr_bucket_brigade *bb, } if (mode == AP_MODE_READBYTES) { - /* [CR]LF consumed, try next */ mode = AP_MODE_SPECULATIVE; cr = 0; } else if (cr) { AP_DEBUG_ASSERT(len == 2 && buf[0] == APR_ASCII_CR); if (buf[1] == APR_ASCII_LF) { - /* consume this CRLF */ mode = AP_MODE_READBYTES; - num_blank_lines++; + num_blank_lines--; } else { - /* CR(?!LF) is data */ - rv = APR_ENOTEMPTY; + c->data_in_input_filters = 1; break; } } else { if (buf[0] == APR_ASCII_LF) { - /* consume this LF */ mode = AP_MODE_READBYTES; - num_blank_lines++; + num_blank_lines--; } else if (buf[0] == APR_ASCII_CR) { cr = 1; } else { - /* Not [CR]LF, some data */ - rv = APR_ENOTEMPTY; + c->data_in_input_filters = 1; break; } } - if (num_blank_lines > max_blank_lines) { - /* Enough blank lines with this connection, - * stop and don't recycle it. - */ + /* Enough blank lines with this connection? + * Stop and don't recycle it. + */ + if (num_blank_lines < 0) { c->keepalive = AP_CONN_CLOSE; - rv = APR_NOTFOUND; - break; } } - - return rv; } @@ -344,7 +328,6 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r) apr_bucket_brigade *bb; apr_bucket *b; conn_rec *c = r->connection; - apr_status_t rv; /* Send an EOR bucket through the output filter chain. When * this bucket is destroyed, the request will be logged and @@ -377,15 +360,8 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r) * already by the EOR bucket's cleanup function. */ - /* Check pipeline consuming blank lines, they must not be interpreted as - * the next pipelined request, otherwise we would block on the next read - * without flushing data, and hence possibly delay pending response(s) - * until the next/real request comes in or the keepalive timeout expires. - */ - rv = ap_check_pipeline(c, bb, DEFAULT_LIMIT_BLANK_LINES); - c->data_in_input_filters = (rv == APR_ENOTEMPTY); + check_pipeline(c, bb); apr_brigade_destroy(bb); - if (c->cs) c->cs->state = (c->aborted) ? CONN_STATE_LINGER : CONN_STATE_WRITE_COMPLETION; diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 59ae8ecb17..9a4812a44b 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -671,7 +671,7 @@ PROXY_DECLARE(int) ap_proxy_checkproxyblock(request_rec *r, proxy_server_conf *c PROXY_DECLARE(int) ap_proxy_pre_http_request(conn_rec *c, request_rec *r); /* DEPRECATED (will be replaced with ap_proxy_connect_backend */ PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr_socket_t **, const char *, apr_sockaddr_t *, const char *, proxy_server_conf *, request_rec *); -/* DEPRECATED (will be replaced with ap_proxy_check_connection */ +/* DEPRECATED (will be replaced with ap_proxy_check_backend */ PROXY_DECLARE(apr_status_t) ap_proxy_ssl_connection_cleanup(proxy_conn_rec *conn, request_rec *r); PROXY_DECLARE(int) ap_proxy_ssl_enable(conn_rec *c); @@ -977,27 +977,22 @@ PROXY_DECLARE(int) ap_proxy_acquire_connection(const char *proxy_function, PROXY_DECLARE(int) ap_proxy_release_connection(const char *proxy_function, proxy_conn_rec *conn, server_rec *s); - -#define PROXY_CHECK_CONN_EMPTY (1 << 0) /** * Check a connection to the backend - * @param scheme calling proxy scheme (http, ajp, ...) - * @param conn acquired connection - * @param server current server record - * @param max_blank_lines how many blank lines to consume, - * or zero for none (considered data) - * @param flags PROXY_CHECK_* bitmask - * @return APR_SUCCESS: connection ready and empty, - * APR_ENOTEMPTY: connection ready and has data, - * APR_ENOSOCKET: not connected, - * APR_EINVAL: worker in error state (unusable), - * other: connection closed/aborted (remotely) + * @param proxy_function calling proxy scheme (http, ajp, ...) + * @param conn acquired connection + * @param s current server record + * @param expect_empty whether to check for empty (no data available) or not + * @return APR_SUCCESS or, + * APR_ENOTSOCK: not connected, + * APR_NOTFOUND: worker in error state (unusable), + * APR_ENOTEMPTY: expect_empty set but the connection has data, + * other: connection closed/aborted (remotely) */ -PROXY_DECLARE(apr_status_t) ap_proxy_check_connection(const char *scheme, - proxy_conn_rec *conn, - server_rec *server, - unsigned max_blank_lines, - int flags); +PROXY_DECLARE(apr_status_t) ap_proxy_check_backend(const char *proxy_function, + proxy_conn_rec *conn, + server_rec *s, + int expect_empty); /** * Make a connection to the backend diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c index a31afa9b1a..b8e818adb2 100644 --- a/modules/proxy/mod_proxy_ajp.c +++ b/modules/proxy/mod_proxy_ajp.c @@ -783,10 +783,8 @@ static int proxy_ajp_handler(request_rec *r, proxy_worker *worker, break; /* Step Two: Make the Connection */ - if (ap_proxy_check_connection(scheme, backend, r->server, 0, - PROXY_CHECK_CONN_EMPTY) - && ap_proxy_connect_backend(scheme, backend, worker, - r->server)) { + if (ap_proxy_check_backend(scheme, backend, r->server, 1) && + ap_proxy_connect_backend(scheme, backend, worker, r->server)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00896) "failed to make connection to backend: %s", backend->hostname); diff --git a/modules/proxy/mod_proxy_fcgi.c b/modules/proxy/mod_proxy_fcgi.c index 467bd7bf66..2a760fcb02 100644 --- a/modules/proxy/mod_proxy_fcgi.c +++ b/modules/proxy/mod_proxy_fcgi.c @@ -928,16 +928,6 @@ static int proxy_fcgi_handler(request_rec *r, proxy_worker *worker, backend->is_ssl = 0; - /* This scheme handler does not reuse connections by default, to - * avoid tying up a fastcgi that isn't expecting to work on - * parallel requests. But if the user went out of their way to - * type the default value of disablereuse=off, we'll allow it. - */ - backend->close = 1; - if (worker->s->disablereuse_set && !worker->s->disablereuse) { - backend->close = 0; - } - /* Step One: Determine Who To Connect To */ uri = apr_palloc(p, sizeof(*uri)); status = ap_proxy_determine_connection(p, r, conf, worker, backend, @@ -948,11 +938,21 @@ static int proxy_fcgi_handler(request_rec *r, proxy_worker *worker, goto cleanup; } + /* This scheme handler does not reuse connections by default, to + * avoid tying up a fastcgi that isn't expecting to work on + * parallel requests. But if the user went out of their way to + * type the default value of disablereuse=off, we'll allow it. + */ + backend->close = 1; + if (worker->s->disablereuse_set && !worker->s->disablereuse) { + backend->close = 0; + } + /* Step Two: Make the Connection */ - if (ap_proxy_check_connection(FCGI_SCHEME, backend, r->server, 0, - PROXY_CHECK_CONN_EMPTY) - && ap_proxy_connect_backend(FCGI_SCHEME, backend, worker, - r->server)) { + if ((backend->close || ap_proxy_check_backend(FCGI_SCHEME, backend, + r->server, 1)) && + ap_proxy_connect_backend(FCGI_SCHEME, backend, worker, + r->server)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01079) "failed to make connection to backend: %s", backend->hostname); diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index ae6384424c..f5731652bf 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -2068,10 +2068,9 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, } /* Step Two: Make the Connection */ - if (ap_proxy_check_connection(proxy_function, backend, r->server, 1, - PROXY_CHECK_CONN_EMPTY) - && ap_proxy_connect_backend(proxy_function, backend, worker, - r->server)) { + if (ap_proxy_check_backend(proxy_function, backend, r->server, 1) && + ap_proxy_connect_backend(proxy_function, backend, worker, + r->server)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01114) "HTTP: failed to make connection to backend: %s", backend->hostname); diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index af8479c1dd..00bac1e307 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -2699,11 +2699,10 @@ PROXY_DECLARE(apr_status_t) ap_proxy_connect_uds(apr_socket_t *sock, #endif } -PROXY_DECLARE(apr_status_t) ap_proxy_check_connection(const char *scheme, - proxy_conn_rec *conn, - server_rec *server, - unsigned max_blank_lines, - int flags) +PROXY_DECLARE(apr_status_t) ap_proxy_check_backend(const char *proxy_function, + proxy_conn_rec *conn, + server_rec *s, + int expect_empty) { apr_status_t rv = APR_SUCCESS; proxy_worker *worker = conn->worker; @@ -2714,17 +2713,25 @@ PROXY_DECLARE(apr_status_t) ap_proxy_check_connection(const char *scheme, * e.g. for a timeout or bad status. We should respect this and should * not continue with a connection via this worker even if we got one. */ - rv = APR_EINVAL; + rv = APR_NOTFOUND; } else if (!conn->sock) { - rv = APR_ENOSOCKET; + rv = APR_ENOTSOCK; } else if (conn->connection) { - rv = ap_check_pipeline(conn->connection, conn->tmp_bb, - max_blank_lines); - if (rv == APR_ENOTEMPTY && !(flags & PROXY_CHECK_CONN_EMPTY)) { + conn_rec *c = conn->connection; + rv = ap_get_brigade(c->input_filters, conn->tmp_bb, + AP_MODE_SPECULATIVE, APR_NONBLOCK_READ, 1); + if (APR_STATUS_IS_EAGAIN(rv)) { rv = APR_SUCCESS; } + else if (rv == APR_SUCCESS && expect_empty) { + apr_off_t len = 0; + apr_brigade_length(conn->tmp_bb, 0, &len); + if (len) { + rv = APR_ENOTEMPTY; + } + } apr_brigade_cleanup(conn->tmp_bb); } else if (!ap_proxy_is_socket_connected(conn->sock)) { @@ -2737,7 +2744,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_check_connection(const char *scheme, * ap_proxy_determine_connection(). */ char ssl_hostname[PROXY_WORKER_RFC1035_NAME_SIZE]; - if (rv == APR_EINVAL + if (rv == APR_NOTFOUND || !conn->ssl_hostname || PROXY_STRNCPY(ssl_hostname, conn->ssl_hostname)) { ssl_hostname[0] = '\0'; @@ -2745,13 +2752,14 @@ PROXY_DECLARE(apr_status_t) ap_proxy_check_connection(const char *scheme, socket_cleanup(conn); if (rv != APR_ENOTEMPTY) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, server, APLOGNO(00951) - "%s: backend socket is disconnected.", scheme); + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951) + "%s: backend socket is disconnected.", + proxy_function); } else { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, server, APLOGNO(03408) + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(03408) "%s: reusable backend connection is not empty: " - "forcibly closed", scheme); + "forcibly closed", proxy_function); } if (ssl_hostname[0]) { @@ -2777,8 +2785,8 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function, proxy_server_conf *conf = (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module); - rv = ap_proxy_check_connection(proxy_function, conn, s, 0, 0); - if (rv == APR_EINVAL) { + rv = ap_proxy_check_backend(proxy_function, conn, s, 0); + if (rv == APR_NOTFOUND) { return DECLINED; } @@ -2994,7 +3002,7 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function, if (rv == APR_SUCCESS) { socket_cleanup(conn); } - rv = APR_EINVAL; + rv = APR_NOTFOUND; } return rv == APR_SUCCESS ? OK : DECLINED;