From: Jim Jagielski Date: Thu, 30 Mar 2006 17:46:44 +0000 (+0000) Subject: Merge accepted backport... X-Git-Tag: 2.2.1~50 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b2a530354a46ba848f28905b417bdce6d9544510;p=thirdparty%2Fapache%2Fhttpd.git Merge accepted backport... git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@390190 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 16348f81a21..49772716718 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,10 @@ -*- coding: utf-8 -*- Changes with Apache 2.2.1 + *) mod_proxy: If we get an error reading the upstream response, + close the connection. [Justin Erenkrantz, Roy T. Fielding, + Jim Jagielski, Ruediger Pluem] + *) mod_proxy_ajp: Support common headers of the AJP protocol in responses. PR 38340. [Aleksey Pesternikov ] diff --git a/STATUS b/STATUS index 6d3589b9c3a..20f6e03c812 100644 --- a/STATUS +++ b/STATUS @@ -92,40 +92,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: +1: rpluem, niq, jerenkrantz NOTE: this also supersedes previous fix to PR#37790 - * mod_proxy: Correctly error out if we get an error from upstream - response. Otherwise, we'll leave the client in a bad state. - http://svn.apache.org/viewcvs.cgi?rev=354628&view=rev - http://svn.apache.org/viewcvs.cgi?rev=354636&view=rev (comment only delta) - Message-ID: <4395A056.2070000@web.turner.com> - +1: jerenkrantz, jim, mturk - -1: roy - rpluem says: The patch mentioned above had been vetoed by Roy on the list - (http://mail-archives.apache.org/mod_mbox/httpd-dev/200512.mbox/ajax/%3cA1E95672-E36F-40FB-B906-41A447D72504@gbiv.com%3e) - Several changes have been done to this patch to fix the - problem and to address Roys veto (see also - http://mail-archives.apache.org/mod_mbox/httpd-dev/200601.mbox/%3c43BF83EB.8090703@apache.org%3e) - The patch now consists of the following revisions on - trunk: - - http://svn.apache.org/viewcvs.cgi?rev=354628&view=rev - http://svn.apache.org/viewcvs.cgi?rev=354636&view=rev - http://svn.apache.org/viewcvs.cgi?rev=357461&view=rev - http://svn.apache.org/viewcvs.cgi?rev=357519&view=rev - http://svn.apache.org/viewcvs.cgi?rev=358022&view=rev - http://svn.apache.org/viewcvs.cgi?rev=365374&view=rev - http://svn.apache.org/viewcvs.cgi?rev=366181&view=rev - http://svn.apache.org/viewcvs.cgi?rev=366554&view=rev - http://svn.apache.org/viewcvs.cgi?rev=366558&view=rev - - A consolidated version of these revisions can be found at - http://people.apache.org/~rpluem/patches/partial_2.2.diff - - As it is fundamentally different from the first approach I - would suggest to restart the voting. - - http://people.apache.org/~rpluem/patches/partial_2.2.diff - +1: jim, rpluem, jerenkrantz - PATCHES PROPOSED TO BACKPORT FROM TRUNK: * mod_dbd: When threaded, create a private pool in child_init diff --git a/modules/cache/mod_disk_cache.c b/modules/cache/mod_disk_cache.c index 1fa9bc1472e..79ed6f14b48 100644 --- a/modules/cache/mod_disk_cache.c +++ b/modules/cache/mod_disk_cache.c @@ -1010,7 +1010,7 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r, * sanity checks. */ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { - if (r->connection->aborted) { + if (r->connection->aborted || r->no_cache) { ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server, "disk_cache: Discarding body for URL %s " "because connection has been aborted.", diff --git a/modules/http/chunk_filter.c b/modules/http/chunk_filter.c index 71f4662fe4c..d6e8ca3aa74 100644 --- a/modules/http/chunk_filter.c +++ b/modules/http/chunk_filter.c @@ -39,6 +39,12 @@ #include "mod_core.h" +/* + * A pointer to this is used to memorize in the filter context that a bad + * gateway error bucket had been seen. It is used as an invented unique pointer. + */ +static char bad_gateway_seen; + apr_status_t ap_http_chunk_filter(ap_filter_t *f, apr_bucket_brigade *b) { #define ASCII_CRLF "\015\012" @@ -67,6 +73,16 @@ apr_status_t ap_http_chunk_filter(ap_filter_t *f, apr_bucket_brigade *b) eos = e; break; } + if (AP_BUCKET_IS_ERROR(e) + && (((ap_bucket_error *)(e->data))->status + == HTTP_BAD_GATEWAY)) { + /* + * We had a broken backend. Memorize this in the filter + * context. + */ + f->ctx = &bad_gateway_seen; + continue; + } if (APR_BUCKET_IS_FLUSH(e)) { flush = e; more = apr_brigade_split(b, APR_BUCKET_NEXT(e)); @@ -146,13 +162,20 @@ apr_status_t ap_http_chunk_filter(ap_filter_t *f, apr_bucket_brigade *b) * 2) the trailer * 3) the end-of-chunked body CRLF * - * If there is no EOS bucket, then do nothing. + * We only do this if we have not seen an error bucket with + * status HTTP_BAD_GATEWAY. We have memorized an + * error bucket that we had seen in the filter context. + * The error bucket with status HTTP_BAD_GATEWAY indicates that the + * connection to the backend (mod_proxy) broke in the middle of the + * response. In order to signal the client that something went wrong + * we do not create the last-chunk marker and set c->keepalive to + * AP_CONN_CLOSE in the core output filter. * * XXX: it would be nice to combine this with the end-of-chunk * marker above, but this is a bit more straight-forward for * now. */ - if (eos != NULL) { + if (eos && !f->ctx) { /* XXX: (2) trailers ... does not yet exist */ e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF /* */ diff --git a/modules/http/http_core.c b/modules/http/http_core.c index 2766b8b85f9..c7691f53556 100644 --- a/modules/http/http_core.c +++ b/modules/http/http_core.c @@ -39,6 +39,7 @@ AP_DECLARE_DATA ap_filter_rec_t *ap_http_input_filter_handle; AP_DECLARE_DATA ap_filter_rec_t *ap_http_header_filter_handle; AP_DECLARE_DATA ap_filter_rec_t *ap_chunk_filter_handle; +AP_DECLARE_DATA ap_filter_rec_t *ap_http_outerror_filter_handle; AP_DECLARE_DATA ap_filter_rec_t *ap_byterange_filter_handle; static const char *set_keep_alive_timeout(cmd_parms *cmd, void *dummy, @@ -202,6 +203,8 @@ static int http_create_request(request_rec *r) NULL, r, r->connection); ap_add_output_filter_handle(ap_http_header_filter_handle, NULL, r, r->connection); + ap_add_output_filter_handle(ap_http_outerror_filter_handle, + NULL, r, r->connection); } return OK; @@ -237,6 +240,9 @@ static void register_hooks(apr_pool_t *p) ap_chunk_filter_handle = ap_register_output_filter("CHUNK", ap_http_chunk_filter, NULL, AP_FTYPE_TRANSCODE); + ap_http_outerror_filter_handle = + ap_register_output_filter("HTTP_OUTERROR", ap_http_outerror_filter, + NULL, AP_FTYPE_PROTOCOL); ap_byterange_filter_handle = ap_register_output_filter("BYTERANGE", ap_byterange_filter, NULL, AP_FTYPE_PROTOCOL); diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c index fda87cf8fbb..28811623478 100644 --- a/modules/http/http_filters.c +++ b/modules/http/http_filters.c @@ -1338,3 +1338,29 @@ AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, return bufsiz; } +/* Filter to handle any error buckets on output */ +apr_status_t ap_http_outerror_filter(ap_filter_t *f, + apr_bucket_brigade *b) +{ + request_rec *r = f->r; + apr_bucket *e; + + for (e = APR_BRIGADE_FIRST(b); + e != APR_BRIGADE_SENTINEL(b); + e = APR_BUCKET_NEXT(e)) + { + if (AP_BUCKET_IS_ERROR(e)) { + /* + * Start of error handling state tree. Just one condition + * right now :) + */ + if (((ap_bucket_error *)(e->data))->status == HTTP_BAD_GATEWAY) { + /* stream aborted and we have not ended it yet */ + r->connection->keepalive = AP_CONN_CLOSE; + } + } + } + + return ap_pass_brigade(f->next, b); +} + diff --git a/modules/http/mod_core.h b/modules/http/mod_core.h index 1f5c1d8c9a9..1cc7a67d393 100644 --- a/modules/http/mod_core.h +++ b/modules/http/mod_core.h @@ -42,6 +42,7 @@ extern "C" { extern AP_DECLARE_DATA ap_filter_rec_t *ap_http_input_filter_handle; extern AP_DECLARE_DATA ap_filter_rec_t *ap_http_header_filter_handle; extern AP_DECLARE_DATA ap_filter_rec_t *ap_chunk_filter_handle; +extern AP_DECLARE_DATA ap_filter_rec_t *ap_http_outerror_filter_handle; extern AP_DECLARE_DATA ap_filter_rec_t *ap_byterange_filter_handle; /* @@ -54,6 +55,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, /* HTTP/1.1 chunked transfer encoding filter. */ apr_status_t ap_http_chunk_filter(ap_filter_t *f, apr_bucket_brigade *b); +/* Filter to handle any error buckets on output */ +apr_status_t ap_http_outerror_filter(ap_filter_t *f, + apr_bucket_brigade *b); + char *ap_response_code_string(request_rec *r, int error_index); /** diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h index 63ed1afe2cf..7bc092eac04 100644 --- a/modules/proxy/mod_proxy.h +++ b/modules/proxy/mod_proxy.h @@ -669,6 +669,15 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function, PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function, proxy_conn_rec *conn, conn_rec *c, server_rec *s); +/** + * 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. + * @param r current request record of client request + * @param brigade The brigade that is sent through the output filter chain + */ +PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r, + apr_bucket_brigade *brigade); /* Scoreboard */ #if MODULE_MAGIC_NUMBER_MAJOR > 20020903 diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c index 22e7cefcd7d..3c97162cced 100644 --- a/modules/proxy/mod_proxy_ajp.c +++ b/modules/proxy/mod_proxy_ajp.c @@ -138,6 +138,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, int havebody = 1; int isok = 1; apr_off_t bb_len; + int data_sent = 0; + int rv = 0; #ifdef FLUSHING_BANDAID apr_int32_t conn_poll_fd; apr_pollfd_t *conn_poll; @@ -348,6 +350,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, "proxy: error processing body"); isok = 0; } + data_sent = 1; apr_brigade_cleanup(output_brigade); } else { @@ -363,6 +366,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, "proxy: error processing body"); isok = 0; } + data_sent = 1; break; default: isok = 0; @@ -400,7 +404,11 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, } apr_brigade_destroy(input_brigade); - apr_brigade_destroy(output_brigade); + /* + * Clear output_brigade to remove possible buckets that remained there + * after an error. + */ + apr_brigade_cleanup(output_brigade); if (status != APR_SUCCESS) { /* We had a failure: Close connection to backend */ @@ -409,9 +417,38 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, "proxy: send body failed to %pI (%s)", conn->worker->cp->addr, conn->worker->hostname); - return HTTP_SERVICE_UNAVAILABLE; + /* + * If we already send data, signal a broken backend connection + * upwards in the chain. + */ + if (data_sent) { + ap_proxy_backend_broke(r, output_brigade); + /* Return DONE to avoid error messages being added to the stream */ + rv = DONE; + } else + rv = HTTP_SERVICE_UNAVAILABLE; + } + + /* + * 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. + */ + if (data_sent && !r->eos_sent && APR_BRIGADE_EMPTY(output_brigade)) { + e = apr_bucket_eos_create(r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(output_brigade, e); } + /* If we have added something to the brigade above, sent it */ + if (!APR_BRIGADE_EMPTY(output_brigade)) + ap_pass_brigade(r->output_filters, output_brigade); + + apr_brigade_destroy(output_brigade); + + if (rv) + return rv; + /* Nice we have answer to send to the client */ if (result == CMD_AJP13_END_RESPONSE && isok) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 9150727d625..40455fb85ba 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -1199,6 +1199,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, * are being read. */ int pread_len = 0; apr_table_t *save_table; + int backend_broke = 0; bb = apr_brigade_create(p, c->bucket_alloc); @@ -1480,8 +1481,16 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, break; } else if (rv != APR_SUCCESS) { + /* In this case, we are in real trouble because + * our backend bailed on us. Pass along a 502 error + * error bucket + */ ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, "proxy: error reading response"); + ap_proxy_backend_broke(r, bb); + ap_pass_brigade(r->output_filters, bb); + backend_broke = 1; + backend->close = 1; break; } /* next time try a non-blocking read */ @@ -1547,6 +1556,11 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, } } while (interim_response); + /* If our connection with the client is to be aborted, return DONE. */ + if (c->aborted || backend_broke) { + return DONE; + } + if (conf->error_override) { /* the code above this checks for 'OK' which is what the hook expects */ if (ap_is_HTTP_SUCCESS(r->status)) diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 410f501711c..1fa9b2f7843 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -2129,3 +2129,23 @@ int ap_proxy_lb_workers(void) lb_workers_limit = proxy_lb_workers + PROXY_DYNAMIC_BALANCER_LIMIT; return lb_workers_limit; } + +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; + /* + * If this is a subrequest, then prevent also caching of the main + * request. + */ + 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); +}