diff options
| -rw-r--r-- | docs/log-message-tags/next-number | 2 | ||||
| -rw-r--r-- | include/ap_mmn.h | 3 | ||||
| -rw-r--r-- | modules/proxy/mod_proxy.h | 25 | ||||
| -rw-r--r-- | modules/proxy/mod_proxy_ajp.c | 9 | ||||
| -rw-r--r-- | modules/proxy/mod_proxy_http.c | 49 | ||||
| -rw-r--r-- | modules/proxy/mod_proxy_uwsgi.c | 61 | ||||
| -rw-r--r-- | modules/proxy/proxy_util.c | 53 |
7 files changed, 137 insertions, 65 deletions
diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number index 3254f8a85c..817b26ce91 100644 --- a/docs/log-message-tags/next-number +++ b/docs/log-message-tags/next-number @@ -1 +1 @@ -10293 +10295 diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 7cc2bcb812..01aa08b0e3 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -691,6 +691,7 @@ * (this MAJOR bump). Overall there is no MAJOR bumb * for 20210506.0 + 20210924.0, MINOR bump only for * adding ap_proxy_tunnel_conn_bytes_{in,out}(). + * 20210924.1 (2.5.1-dev) Add ap_proxy_fill_error_brigade() */ #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */ @@ -698,7 +699,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20210924 #endif -#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 76bf74c676..646d9795fd 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -1177,12 +1177,27 @@ PROXY_DECLARE(int) ap_proxy_connection_create_ex(const char *proxy_function, PROXY_DECLARE(int) ap_proxy_connection_reusable(proxy_conn_rec *conn); /** - * Signal the upstream chain that the connection to the backend broke in the - * middle of the response. This is done by sending an error bucket with - * status HTTP_BAD_GATEWAY and an EOS bucket up the filter chain. + * Fill a brigade that can be sent downstream to signal that the connection to + * the backend broke in the middle of the response. The brigade will contain + * an error bucket with the given status and eventually an EOC bucket if asked + * to or heuristically if the response header was sent already. * @param r current request record of client request - * @param brigade The brigade that is sent through the output filter chain - * @deprecated To be removed in v2.6. + * @param status the error status + * @param bb the brigade to fill + * @param eoc 0 do not add an EOC bucket, > 0 always add one, < 0 add one + * only if the response header was sent already + */ +PROXY_DECLARE(void) ap_proxy_fill_error_brigade(request_rec *r, int status, + apr_bucket_brigade *bb, + int eoc); + +/** + * Fill a brigade that can be sent downstream to signal that the connection to + * the backend broke in the middle of the response. The brigade will contain + * an error bucket with status HTTP_BAD_GATEWAY and an EOS bucket. + * @param r current request record of client request + * @param brigade the brigade to fill + * @deprecated To be removed in v2.6 (see ap_proxy_fill_error_brigade). */ PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r, apr_bucket_brigade *brigade); diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c index 838ef7bbe3..4d734588ae 100644 --- a/modules/proxy/mod_proxy_ajp.c +++ b/modules/proxy/mod_proxy_ajp.c @@ -670,7 +670,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, * upwards in the chain. */ if (data_sent) { - ap_proxy_backend_broke(r, output_brigade); + ap_proxy_fill_error_brigade(r, HTTP_BAD_GATEWAY, output_brigade, -1); } else if (!send_body && (is_idempotent(r) == METHOD_IDEMPOTENT)) { /* * This is only non fatal when we have not send (parts) of a possible @@ -706,12 +706,9 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, /* * Ensure that we sent an EOS bucket thru the filter chain, if we already - * have sent some data. Maybe ap_proxy_backend_broke was called and added - * one to the brigade already (no longer making it empty). So we should - * not do this in this case. + * have sent some data. */ - if (data_sent && !r->eos_sent && !r->connection->aborted - && APR_BRIGADE_EMPTY(output_brigade)) { + if (data_sent && !r->eos_sent && !r->connection->aborted) { e = apr_bucket_eos_create(r->connection->bucket_alloc); APR_BRIGADE_INSERT_TAIL(output_brigade, e); } diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 01436b6a2b..c729220f3b 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -1177,11 +1177,7 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) " Number of keepalives %i", backend->hostname, backend->port, c->keepalives); - e = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL, - r->pool, c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, e); - e = ap_bucket_eoc_create(c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, e); + ap_proxy_fill_error_brigade(r, HTTP_BAD_GATEWAY, bb, 1); ap_pass_brigade(r->output_filters, bb); /* Mark the backend connection for closing */ backend->close = 1; @@ -1710,11 +1706,12 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) mode = APR_BLOCK_READ; continue; } - else if (rv == APR_EOF) { + if (rv == APR_EOF) { backend->close = 1; break; } - else if (rv != APR_SUCCESS) { + if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) { + int error_status = HTTP_BAD_GATEWAY; if (rv == APR_ENOSPC) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(02475) "Response chunk/line was too large to parse"); @@ -1723,10 +1720,15 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(02476) "Response Transfer-Encoding was not recognised"); } - else { + else if (rv != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01110) "Network error reading response"); } + else { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10293) + "Unexpected empty data reading response"); + error_status = HTTP_INTERNAL_SERVER_ERROR; + } /* In this case, we are in real trouble because * our backend bailed on us. Given we're half way @@ -1734,11 +1736,7 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) * disconnect the client too. */ apr_brigade_cleanup(bb); - e = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL, - r->pool, c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, e); - e = ap_bucket_eoc_create(c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, e); + ap_proxy_fill_error_brigade(r, error_status, bb, 1); ap_pass_brigade(r->output_filters, bb); backend_broke = 1; @@ -1762,13 +1760,28 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) "readbytes: %#x", readbytes); } #endif - /* sanity check */ - if (APR_BRIGADE_EMPTY(bb)) { - break; - } /* Switch the allocator lifetime of the buckets */ - ap_proxy_buckets_lifetime_transform(r, bb, pass_bb); + rv = ap_proxy_buckets_lifetime_transform(r, bb, pass_bb); + if (rv != APR_SUCCESS) { + /* Same, half way through a response, our only option is + * to notice the output filters and then disconnect the + * client and backend. + */ + if (!APR_BRIGADE_EMPTY(pass_bb)) { + /* Pass what we have still */ + ap_pass_brigade(r->output_filters, pass_bb); + apr_brigade_cleanup(pass_bb); + } + ap_proxy_fill_error_brigade(r, HTTP_INTERNAL_SERVER_ERROR, + pass_bb, 1); + ap_pass_brigade(r->output_filters, pass_bb); + apr_brigade_cleanup(pass_bb); + + backend_broke = 1; + backend->close = 1; + break; + } /* found the last brigade? */ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(pass_bb))) { diff --git a/modules/proxy/mod_proxy_uwsgi.c b/modules/proxy/mod_proxy_uwsgi.c index 971eaa59dc..2c493cdab4 100644 --- a/modules/proxy/mod_proxy_uwsgi.c +++ b/modules/proxy/mod_proxy_uwsgi.c @@ -379,14 +379,11 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec * backend, int status = r->status; r->status = HTTP_OK; r->status_line = NULL; - - apr_brigade_cleanup(bb); - apr_brigade_cleanup(pass_bb); - return status; } while (!finish) { + apr_brigade_cleanup(bb); rv = ap_get_brigade(rp->input_filters, bb, AP_MODE_READBYTES, mode, conf->io_buffer_size); if (APR_STATUS_IS_EAGAIN(rv) @@ -396,52 +393,60 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec * backend, if (ap_pass_brigade(r->output_filters, bb) || c->aborted) { break; } - apr_brigade_cleanup(bb); mode = APR_BLOCK_READ; continue; } - else if (rv == APR_EOF) { + if (rv == APR_EOF) { break; } - else if (rv != APR_SUCCESS) { - ap_proxy_backend_broke(r, bb); + if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) { + int status = HTTP_BAD_GATEWAY; + if (rv == APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10294) + "Unexpected empty data reading uwscgi response"); + status = HTTP_INTERNAL_SERVER_ERROR; + } + apr_brigade_cleanup(bb); + ap_proxy_fill_error_brigade(r, status, bb, -1); ap_pass_brigade(r->output_filters, bb); backend_broke = 1; break; } + /* found the last brigade? */ + if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) + finish = 1; + mode = APR_NONBLOCK_READ; apr_brigade_length(bb, 0, &readbytes); backend->worker->s->read += readbytes; - if (APR_BRIGADE_EMPTY(bb)) { - apr_brigade_cleanup(bb); + rv = ap_proxy_buckets_lifetime_transform(r, bb, pass_bb); + if (rv != APR_SUCCESS) { + /* Half way through a response, our only option is + * to notice the output filters and then disconnect the + * client and backend. + */ + if (!APR_BRIGADE_EMPTY(pass_bb)) { + /* Pass what we have still */ + ap_pass_brigade(r->output_filters, pass_bb); + apr_brigade_cleanup(pass_bb); + } + ap_proxy_fill_error_brigade(r, HTTP_INTERNAL_SERVER_ERROR, + pass_bb, -1); + ap_pass_brigade(r->output_filters, pass_bb); + apr_brigade_cleanup(pass_bb); + + backend_broke = 1; break; } - - ap_proxy_buckets_lifetime_transform(r, bb, pass_bb); - - /* found the last brigade? */ - if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) - finish = 1; - - /* do not pass chunk if it is zero_sized */ - apr_brigade_length(pass_bb, 0, &readbytes); - - if ((readbytes > 0 - && ap_pass_brigade(r->output_filters, pass_bb) != APR_SUCCESS) + if (ap_pass_brigade(r->output_filters, pass_bb) != APR_SUCCESS || c->aborted) { finish = 1; } - - apr_brigade_cleanup(bb); apr_brigade_cleanup(pass_bb); } - e = apr_bucket_eos_create(c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, e); - ap_pass_brigade(r->output_filters, bb); - apr_brigade_cleanup(bb); if (c->aborted || backend_broke) { diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index c06cfe5f26..7ce2431f08 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -3542,11 +3542,54 @@ PROXY_DECLARE(int) ap_proxy_should_override(proxy_dir_conf *conf, int code) code); } +PROXY_DECLARE(void) ap_proxy_fill_error_brigade(request_rec *r, int status, + apr_bucket_brigade *bb, + int eoc) +{ + apr_bucket *e, *eos; + conn_rec *c = r->connection; + + /* + * Add an error and (eventually) EOC buckets to signal the http filter + * that it should get out of our way, BUT ensure that they are inserted + * BEFORE an EOS bucket in bb as some resource filters like mod_deflate + * pass everything up to the EOS down the chain immediately and sent the + * remainder of the brigade later (or even never). But in this case the + * ap_http_header_filter does not get out of our way soon enough. + */ + + eos = APR_BRIGADE_LAST(bb); + while (eos != APR_BRIGADE_SENTINEL(bb) && !APR_BUCKET_IS_EOS(eos)) { + eos = APR_BUCKET_PREV(eos); + } + + e = ap_bucket_error_create(status, NULL, c->pool, c->bucket_alloc); + if (eos == APR_BRIGADE_SENTINEL(bb)) { + APR_BRIGADE_INSERT_TAIL(bb, e); + eos = APR_BRIGADE_SENTINEL(bb); + } + else { + APR_BUCKET_INSERT_BEFORE(eos, e); + } + + /* If asked to (eoc > 0) or if heuristically (eoc < 0) the header was + * sent already we need to terminate the connection. + */ + if (eoc > 0 || (eoc < 0 && r->sent_bodyct)) { + e = ap_bucket_eoc_create(c->bucket_alloc); + if (eos == APR_BRIGADE_SENTINEL(bb)) { + APR_BRIGADE_INSERT_TAIL(bb, e); + } + else { + APR_BUCKET_INSERT_BEFORE(eos, e); + } + } +} + /* deprecated - to be removed in v2.6 */ PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r, apr_bucket_brigade *brigade) { - apr_bucket *e; conn_rec *c = r->connection; r->no_cache = 1; @@ -3556,11 +3599,9 @@ PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r, */ if (r->main) r->main->no_cache = 1; - e = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL, c->pool, - c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(brigade, e); - e = apr_bucket_eos_create(c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(brigade, e); + + APR_BRIGADE_INSERT_TAIL(brigade, apr_bucket_eos_create(c->bucket_alloc)); + ap_proxy_fill_error_brigade(r, HTTP_BAD_GATEWAY, brigade, 0); } /* |