You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2013/05/14 20:58:06 UTC
svn commit: r1482522 - in /httpd/httpd/trunk: ./ docs/log-message-tags/
include/ modules/dav/main/ modules/generators/ modules/http/ modules/proxy/
server/
Author: minfrin
Date: Tue May 14 18:58:06 2013
New Revision: 1482522
URL: http://svn.apache.org/r1482522
Log:
core: Stop the HTTP_IN filter from attempting to write error buckets
to the output filters, which is bogus in the proxy case. Create a
clean mapping from APR codes to HTTP status codes, and use it where
needed.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/docs/log-message-tags/next-number
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/include/http_protocol.h
httpd/httpd/trunk/modules/dav/main/mod_dav.c
httpd/httpd/trunk/modules/generators/mod_cgi.c
httpd/httpd/trunk/modules/generators/mod_cgid.c
httpd/httpd/trunk/modules/http/http_filters.c
httpd/httpd/trunk/modules/proxy/mod_proxy.h
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
httpd/httpd/trunk/modules/proxy/proxy_util.c
httpd/httpd/trunk/server/util.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue May 14 18:58:06 2013
@@ -1,6 +1,11 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) core: Stop the HTTP_IN filter from attempting to write error buckets
+ to the output filters, which is bogus in the proxy case. Create a
+ clean mapping from APR codes to HTTP status codes, and use it where
+ needed. [Graham Leggett]
+
*) mod_proxy: Ensure we don't attempt to amend a table we are iterating
through, ensuring that all headers listed by Connection are removed.
[Graham Leggett, Co-Advisor <coad measurement-factory.com>]
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=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Tue May 14 18:58:06 2013
@@ -1 +1 @@
-2475
+2478
Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Tue May 14 18:58:06 2013
@@ -429,6 +429,7 @@
* ap_condition_if_modified_since(),
* ap_condition_if_range()
* 20121222.13 (2.5.0-dev) Add ap_proxy_clear_connection()
+ * 20121222.14 (2.5.0-dev) Add ap_map_http_request_error()
*/
#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -436,7 +437,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20121222
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 13 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 14 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Modified: httpd/httpd/trunk/include/http_protocol.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_protocol.h?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_protocol.h (original)
+++ httpd/httpd/trunk/include/http_protocol.h Tue May 14 18:58:06 2013
@@ -502,6 +502,23 @@ AP_DECLARE(int) ap_should_client_block(r
*/
AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, apr_size_t bufsiz);
+/*
+ * Map specific APR codes returned by the filter stack to HTTP error
+ * codes, or the default status code provided. Use it as follows:
+ *
+ * return ap_map_http_response(rv, HTTP_BAD_REQUEST);
+ *
+ * If the filter has already handled the error, AP_FILTER_ERROR will
+ * be returned, which is cleanly passed through.
+ *
+ * These mappings imply that the filter stack is reading from the
+ * downstream client, the proxy will map these codes differently.
+ * @param rv APR status code
+ * @param status Default HTTP code should the APR code not be recognised
+ * @return Mapped HTTP status code
+ */
+AP_DECLARE(int) ap_map_http_request_error(apr_status_t rv, int status);
+
/**
* In HTTP/1.1, any method can have a body. However, most GET handlers
* wouldn't know what to do with a request body if they received one.
Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original)
+++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Tue May 14 18:58:06 2013
@@ -1001,9 +1001,11 @@ static int dav_method_put(request_rec *r
}
else {
/* XXX: should this actually be HTTP_BAD_REQUEST? */
- http_err = HTTP_INTERNAL_SERVER_ERROR;
- msg = apr_psprintf(r->pool, "An error occurred while reading"
- " the request body (URI: %s)", msg);
+ http_err = ap_map_http_request_error(rc,
+ HTTP_INTERNAL_SERVER_ERROR);
+ msg = apr_psprintf(r->pool,
+ "An error occurred while reading"
+ " the request body (URI: %s)", msg);
}
err = dav_new_error(r->pool, http_err, 0, rc, msg);
break;
Modified: httpd/httpd/trunk/modules/generators/mod_cgi.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgi.c?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_cgi.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgi.c Tue May 14 18:58:06 2013
@@ -857,7 +857,7 @@ static int cgi_handler(request_rec *r)
}
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01225)
"Error reading request entity data");
- return HTTP_INTERNAL_SERVER_ERROR;
+ return ap_map_http_request_error(rv, HTTP_INTERNAL_SERVER_ERROR);
}
for (bucket = APR_BRIGADE_FIRST(bb);
Modified: httpd/httpd/trunk/modules/generators/mod_cgid.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgid.c?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_cgid.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgid.c Tue May 14 18:58:06 2013
@@ -1473,7 +1473,7 @@ static int cgid_handler(request_rec *r)
}
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01270)
"Error reading request entity data");
- return HTTP_INTERNAL_SERVER_ERROR;
+ return ap_map_http_request_error(rv, HTTP_INTERNAL_SERVER_ERROR);
}
for (bucket = APR_BRIGADE_FIRST(bb);
Modified: httpd/httpd/trunk/modules/http/http_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_filters.c (original)
+++ httpd/httpd/trunk/modules/http/http_filters.c Tue May 14 18:58:06 2013
@@ -78,30 +78,6 @@ typedef struct http_filter_ctx {
apr_bucket_brigade *bb;
} http_ctx_t;
-/* bail out if some error in the HTTP input filter happens */
-static apr_status_t bail_out_on_error(http_ctx_t *ctx,
- ap_filter_t *f,
- int http_error)
-{
- apr_bucket *e;
- apr_bucket_brigade *bb = ctx->bb;
-
- apr_brigade_cleanup(bb);
- e = ap_bucket_error_create(http_error,
- NULL, f->r->pool,
- f->c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- e = apr_bucket_eos_create(f->c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- ctx->eos_sent = 1;
- /* If chunked encoding / content-length are corrupt, we may treat parts
- * of this request's body as the next one's headers.
- * To be safe, disable keep-alive.
- */
- f->r->connection->keepalive = AP_CONN_CLOSE;
- return ap_pass_brigade(f->r->output_filters, bb);
-}
-
static apr_status_t get_remaining_chunk_line(http_ctx_t *ctx,
apr_bucket_brigade *b,
int linelimit)
@@ -269,7 +245,7 @@ apr_status_t ap_http_filter(ap_filter_t
*/
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01585)
"Unknown Transfer-Encoding: %s", tenc);
- return bail_out_on_error(ctx, f, HTTP_NOT_IMPLEMENTED);
+ return APR_ENOTIMPL;
}
else {
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, f->r, APLOGNO(01586)
@@ -292,7 +268,7 @@ apr_status_t ap_http_filter(ap_filter_t
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01587)
"Invalid Content-Length");
- return bail_out_on_error(ctx, f, HTTP_REQUEST_ENTITY_TOO_LARGE);
+ return APR_ENOSPC;
}
/* If we have a limit in effect and we know the C-L ahead of
@@ -303,7 +279,7 @@ apr_status_t ap_http_filter(ap_filter_t
"Requested content-length of %" APR_OFF_T_FMT
" is larger than the configured limit"
" of %" APR_OFF_T_FMT, ctx->remaining, ctx->limit);
- return bail_out_on_error(ctx, f, HTTP_REQUEST_ENTITY_TOO_LARGE);
+ return APR_ENOSPC;
}
}
@@ -396,10 +372,7 @@ apr_status_t ap_http_filter(ap_filter_t
(ctx->remaining < 0) ? "(overflow)" : "");
ctx->remaining = 0; /* Reset it in case we have to
* come back here later */
- if (APR_STATUS_IS_TIMEUP(rv)) {
- http_error = HTTP_REQUEST_TIME_OUT;
- }
- return bail_out_on_error(ctx, f, http_error);
+ return rv;
}
if (!ctx->remaining) {
@@ -502,10 +475,7 @@ apr_status_t ap_http_filter(ap_filter_t
(ctx->remaining < 0) ? "(overflow)" : "");
ctx->remaining = 0; /* Reset it in case we have to
* come back here later */
- if (APR_STATUS_IS_TIMEUP(rv)) {
- http_error = HTTP_REQUEST_TIME_OUT;
- }
- return bail_out_on_error(ctx, f, http_error);
+ return rv;
}
if (!ctx->remaining) {
@@ -1422,6 +1392,39 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
return ap_pass_brigade(f->next, b);
}
+/*
+ * Map specific APR codes returned by the filter stack to HTTP error
+ * codes, or the default status code provided. Use it as follows:
+ *
+ * return ap_map_http_response(rv, HTTP_BAD_REQUEST);
+ *
+ * If the filter has already handled the error, AP_FILTER_ERROR will
+ * be returned, which is cleanly passed through.
+ *
+ * These mappings imply that the filter stack is reading from the
+ * downstream client, the proxy will map these codes differently.
+ */
+AP_DECLARE(int) ap_map_http_request_error(apr_status_t rv, int status)
+{
+ switch (rv) {
+ case AP_FILTER_ERROR: {
+ return AP_FILTER_ERROR;
+ }
+ case APR_ENOSPC: {
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+ case APR_ENOTIMPL: {
+ return HTTP_NOT_IMPLEMENTED;
+ }
+ case APR_ETIMEDOUT: {
+ return HTTP_REQUEST_TIME_OUT;
+ }
+ default: {
+ return status;
+ }
+ }
+}
+
/* In HTTP/1.1, any method can have a body. However, most GET handlers
* wouldn't know what to do with a request body if they received one.
* This helper routine tests for and reads any message body in the request,
@@ -1439,7 +1442,8 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
AP_DECLARE(int) ap_discard_request_body(request_rec *r)
{
apr_bucket_brigade *bb;
- int rv, seen_eos;
+ int seen_eos;
+ apr_status_t rv;
/* Sometimes we'll get in a state where the input handling has
* detected an error where we want to drop the connection, so if
@@ -1462,21 +1466,8 @@ AP_DECLARE(int) ap_discard_request_body(
APR_BLOCK_READ, HUGE_STRING_LEN);
if (rv != APR_SUCCESS) {
- /* FIXME: If we ever have a mapping from filters (apr_status_t)
- * to HTTP error codes, this would be a good place for them.
- *
- * If we received the special case AP_FILTER_ERROR, it means
- * that the filters have already handled this error.
- * Otherwise, we should assume we have a bad request.
- */
- if (rv == AP_FILTER_ERROR) {
- apr_brigade_destroy(bb);
- return rv;
- }
- else {
- apr_brigade_destroy(bb);
- return HTTP_BAD_REQUEST;
- }
+ apr_brigade_destroy(bb);
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
}
for (bucket = APR_BRIGADE_FIRST(bb);
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Tue May 14 18:58:06 2013
@@ -863,6 +863,7 @@ PROXY_DECLARE(int) ap_proxy_connection_c
* 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
+ * @deprecated To be removed in v2.6.
*/
PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r,
apr_bucket_brigade *brigade);
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=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue May 14 18:58:06 2013
@@ -1207,11 +1207,10 @@ apr_status_t ap_proxygetline(apr_bucket_
#endif
static
-apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
- proxy_conn_rec **backend_ptr,
- proxy_worker *worker,
- proxy_server_conf *conf,
- char *server_portstr) {
+int ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
+ proxy_conn_rec **backend_ptr, proxy_worker *worker,
+ proxy_server_conf *conf, char *server_portstr)
+{
conn_rec *c = r->connection;
char buffer[HUGE_STRING_LEN];
const char *buf;
@@ -1308,36 +1307,18 @@ apr_status_t ap_proxy_http_process_respo
*/
if (r->proxyreq == PROXYREQ_REVERSE && c->keepalives &&
!APR_STATUS_IS_TIMEUP(rc)) {
- apr_bucket *eos;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01104)
"Closing connection to client because"
" reading from backend server %s:%d failed."
" Number of keepalives %i", backend->hostname,
backend->port, c->keepalives);
- ap_proxy_backend_broke(r, bb);
- /*
- * Add an EOC bucket to signal the ap_http_header_filter
- * that it should get out of our way, BUT ensure that the
- * EOC bucket is 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.
- */
+
+ e = ap_bucket_error_create(HTTP_GATEWAY_TIME_OUT, NULL,
+ r->pool, c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
e = ap_bucket_eoc_create(c->bucket_alloc);
- eos = APR_BRIGADE_LAST(bb);
- while ((APR_BRIGADE_SENTINEL(bb) != eos)
- && !APR_BUCKET_IS_EOS(eos)) {
- eos = APR_BUCKET_PREV(eos);
- }
- if (eos == APR_BRIGADE_SENTINEL(bb)) {
- APR_BRIGADE_INSERT_TAIL(bb, e);
- }
- else {
- APR_BUCKET_INSERT_BEFORE(eos, e);
- }
+ APR_BRIGADE_INSERT_TAIL(bb, e);
ap_pass_brigade(r->output_filters, bb);
/* Mark the backend connection for closing */
backend->close = 1;
@@ -1698,14 +1679,31 @@ apr_status_t ap_proxy_http_process_respo
break;
}
else if (rv != APR_SUCCESS) {
+ if (rv == APR_ENOSPC) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(02475)
+ "Response chunk/line was too large to parse");
+ }
+ else if (rv == APR_ENOTIMPL) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(02476)
+ "Response Transfer-Encoding was not recognised");
+ }
+ else {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01110)
+ "Network error reading response");
+ }
+
/* In this case, we are in real trouble because
- * our backend bailed on us. Pass along a 504 error
- * error bucket
+ * our backend bailed on us. Given we're half way
+ * through a response, our only option is to
+ * disconnect the client too.
*/
- ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01110)
- "error reading response");
- ap_proxy_backend_broke(r, bb);
+ e = ap_bucket_error_create(HTTP_GATEWAY_TIME_OUT, 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_pass_brigade(r->output_filters, bb);
+
backend_broke = 1;
backend->close = 1;
break;
@@ -2024,7 +2022,7 @@ static int proxy_http_post_config(apr_po
ap_proxy_clear_connection_fn =
APR_RETRIEVE_OPTIONAL_FN(ap_proxy_clear_connection);
if (!ap_proxy_clear_connection_fn) {
- ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO()
+ ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02477)
"mod_proxy must be loaded for mod_proxy_http");
return !OK;
}
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue May 14 18:58:06 2013
@@ -2754,6 +2754,7 @@ int ap_proxy_lb_workers(void)
return lb_workers_limit;
}
+/* deprecated - to be removed in v2.6 */
PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r,
apr_bucket_brigade *brigade)
{
Modified: httpd/httpd/trunk/server/util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1482522&r1=1482521&r2=1482522&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util.c (original)
+++ httpd/httpd/trunk/server/util.c Tue May 14 18:58:06 2013
@@ -2529,7 +2529,7 @@ AP_DECLARE(int) ap_parse_form_data(reque
APR_BLOCK_READ, HUGE_STRING_LEN);
if (rv != APR_SUCCESS) {
apr_brigade_destroy(bb);
- return (rv == AP_FILTER_ERROR) ? rv : HTTP_BAD_REQUEST;
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
}
for (bucket = APR_BRIGADE_FIRST(bb);