From 305c215b7618a13eb1b86ed99ea37f6fce25b906 Mon Sep 17 00:00:00 2001 From: Justin Erenkrantz Date: Fri, 31 May 2002 07:19:04 +0000 Subject: [PATCH] Fix the case where if there is no ErrorDocument specified for an error, the error would be sent to the client *twice* because both the filter and the core would trigger error responses. The problem is that the filters have already handled some errors (say 413) and due to the ap_get_client_block API, the error was morphed into 400. Therefore, ap_discard_request_body must use brigades directly rather than the ap_get_client_block API so that any potential errors are not dropped. The special value AP_FILTER_ERROR indicates that the lower level has already dealt with this problem (ap_die() will realize this). Otherwise, we'll error with HTTP_BAD_REQUEST and ap_die() will take it from there. This also prevents needless memory copies when we are just going to discard it anyway. Thanks to Cliff Woolley who found this wacky problem. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@95424 13f79535-47bb-0310-9956-ffa450edef68 --- modules/http/http_protocol.c | 67 ++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c index 935fc0b9d8..cd99c64bb7 100644 --- a/modules/http/http_protocol.c +++ b/modules/http/http_protocol.c @@ -1851,7 +1851,8 @@ AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, */ AP_DECLARE(int) ap_discard_request_body(request_rec *r) { - int rv; + apr_bucket_brigade *bb; + int rv, seen_eos; /* Sometimes we'll get in a state where the input handling has * detected an error where we want to drop the connection, so if @@ -1864,33 +1865,55 @@ AP_DECLARE(int) ap_discard_request_body(request_rec *r) return OK; } - if (r->read_length == 0) { /* if not read already */ - if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK))) { - return rv; + bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); + seen_eos = 0; + do { + apr_bucket *bucket; + + rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES, + 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) { + return rv; + } + else { + return HTTP_BAD_REQUEST; + } } - } - /* In order to avoid sending 100 Continue when we already know the - * final response status, and yet not kill the connection if there is - * no request body to be read, we need to duplicate the test from - * ap_should_client_block() here negated rather than call it directly. - */ - if ((r->read_length == 0) && (r->read_chunked || (r->remaining > 0))) { - char dumpbuf[HUGE_STRING_LEN]; + APR_BRIGADE_FOREACH(bucket, bb) { + const char *data; + apr_size_t len; - if (r->expecting_100) { - r->connection->keepalive = -1; - return OK; - } + if (APR_BUCKET_IS_EOS(bucket)) { + seen_eos = 1; + break; + } - while ((rv = ap_get_client_block(r, dumpbuf, HUGE_STRING_LEN)) > 0) { - continue; - } + /* These are metadata buckets. */ + if (bucket->length == 0) { + continue; + } - if (rv < 0) { - return HTTP_BAD_REQUEST; + /* We MUST read because in case we have an unknown-length + * bucket or one that morphs, we want to exhaust it. + */ + rv = apr_bucket_read(bucket, &data, &len, APR_BLOCK_READ); + if (rv != APR_SUCCESS) { + return HTTP_BAD_REQUEST; + } } - } + apr_brigade_cleanup(bb); + } while (!seen_eos); + return OK; } -- 2.50.1