www/apache
1
0
Fork 0
mirror of https://github.com/apache/httpd.git synced 2025-08-08 15:02:10 +03:00
Code Activity

core: Stop the HTTP_IN filter from attempting to write error buckets

Browse Source 
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.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1482522 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Graham Leggett
2013-05-14 18:58:06 +00:00
parent e929fb1ee5
commit 9bc9d79079
12 changed files with 109 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
CHANGES
View File

@@ -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>]

2
docs/log-message-tags/next-number
View File

@@ -1 +1 @@
2475
2478

3
include/ap_mmn.h
View File

@@ -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

17
include/http_protocol.h
View File

@@ -502,6 +502,23 @@ AP_DECLARE(int) ap_should_client_block(request_rec *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.

8
modules/dav/main/mod_dav.c
View File

@@ -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;

2
modules/generators/mod_cgi.c
View File

@@ -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);

2
modules/generators/mod_cgid.c
View File

@@ -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);

93
modules/http/http_filters.c
View File

@@ -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 *f, apr_bucket_brigade *b,
*/
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 *f, apr_bucket_brigade *b,
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 *f, apr_bucket_brigade *b,
"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 *f, apr_bucket_brigade *b,
(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 *f, apr_bucket_brigade *b,
(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_http_header_filter(ap_filter_t *f,
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_http_header_filter(ap_filter_t *f,
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(request_rec *r)
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);

1
modules/proxy/mod_proxy.h
View File

@@ -863,6 +863,7 @@ PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function,
* 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);

66
modules/proxy/mod_proxy_http.c
View File

@@ -1207,11 +1207,10 @@ apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, request_rec
#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_response(apr_pool_t * p, request_rec *r,
*/
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_response(apr_pool_t * p, request_rec *r,
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_pool_t *pconf, apr_pool_t *plog,
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;
}

1
modules/proxy/proxy_util.c
View File

@@ -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)
{

2
server/util.c
View File

@@ -2529,7 +2529,7 @@ AP_DECLARE(int) ap_parse_form_data(request_rec *r, ap_filter_t *f,
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);