You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2021/09/24 11:25:43 UTC
svn commit: r1893595 - in /httpd/httpd/trunk:
docs/log-message-tags/next-number include/ap_mmn.h
modules/proxy/mod_proxy.h modules/proxy/mod_proxy_ajp.c
modules/proxy/mod_proxy_http.c modules/proxy/mod_proxy_uwsgi.c
modules/proxy/proxy_util.c
Author: ylavic
Date: Fri Sep 24 11:25:42 2021
New Revision: 1893595
URL: http://svn.apache.org/viewvc?rev=1893595&view=rev
Log:
mod_proxy: Handle ap_proxy_buckets_lifetime_transform() errors.
* modules/proxy/mod_proxy.h,modules/proxy/proxy_util.c:
Add ap_proxy_fill_error_brigade() to factorize proxy error handling
on the client connection side.
* modules/proxy/mod_proxy_{http,ajp,uwsgi}.c:
Use ap_proxy_fill_error_brigade() where needed, including when an
empty brigade is returned on the backend side or when calling
ap_proxy_buckets_lifetime_transform fails.
Modified:
httpd/httpd/trunk/docs/log-message-tags/next-number
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/modules/proxy/mod_proxy.h
httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1893595&r1=1893594&r2=1893595&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Sep 24 11:25:42 2021
@@ -1 +1 @@
-10293
+10295
Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1893595&r1=1893594&r2=1893595&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Fri Sep 24 11:25:42 2021
@@ -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
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1893595&r1=1893594&r2=1893595&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Fri Sep 24 11:25:42 2021
@@ -1177,12 +1177,27 @@ PROXY_DECLARE(int) ap_proxy_connection_c
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);
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c?rev=1893595&r1=1893594&r2=1893595&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c Fri Sep 24 11:25:42 2021
@@ -670,7 +670,7 @@ static int ap_proxy_ajp_request(apr_pool
* 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
/*
* 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);
}
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1893595&r1=1893594&r2=1893595&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Sep 24 11:25:42 2021
@@ -1177,11 +1177,7 @@ int ap_proxy_http_process_response(proxy
" 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
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
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
* 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
"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))) {
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c?rev=1893595&r1=1893594&r2=1893595&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c Fri Sep 24 11:25:42 2021
@@ -379,14 +379,11 @@ static int uwsgi_response(request_rec *r
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
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) {
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1893595&r1=1893594&r2=1893595&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Sep 24 11:25:42 2021
@@ -3542,11 +3542,54 @@ PROXY_DECLARE(int) ap_proxy_should_overr
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_bro
*/
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);
}
/*