You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by je...@apache.org on 2002/05/29 01:38:31 UTC
cvs commit: httpd-2.0/modules/http http_protocol.c http_request.c
jerenkrantz 02/05/28 16:38:31
Modified: . CHANGES
modules/http http_protocol.c http_request.c
Log:
Correctly return 413 when an invalid chunk size is given on input.
- If get_chunk_size() returns a negative number, that probably implies
an overflow. So, create a 413 error and pass it to the output filters.
- Modify ap_discard_request_body() to return OK quickly if we're a subreq
or our status code implies that we will be dropping the connection.
- Modify ap_die() so that if the new status implies that we will drop
the connection, that we correctly indicate that we can not keepalive
this connection. (Without this, the error is returned, but the connection
is not closed.)
Revision Changes Path
1.794 +5 -0 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.793
retrieving revision 1.794
diff -u -r1.793 -r1.794
--- CHANGES 28 May 2002 21:51:13 -0000 1.793
+++ CHANGES 28 May 2002 23:38:31 -0000 1.794
@@ -1,5 +1,10 @@
Changes with Apache 2.0.37
+ *) Correctly return 413 when an invalid chunk size is given on
+ input. Also modify ap_discard_request_body to not do anything
+ on sub-requests or when the connection will be dropped.
+ [Justin Erenkrantz]
+
*) Fix the TIME_* SSL var lookups to be threadsafe. PR 9469.
[Cliff Woolley]
1.422 +33 -0 httpd-2.0/modules/http/http_protocol.c
Index: http_protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.421
retrieving revision 1.422
diff -u -r1.421 -r1.422
--- http_protocol.c 17 May 2002 11:33:10 -0000 1.421
+++ http_protocol.c 28 May 2002 23:38:31 -0000 1.422
@@ -852,6 +852,17 @@
apr_brigade_flatten(bb, line, &len);
ctx->remaining = get_chunk_size(line);
+ /* Detect invalid chunk sizes. */
+ if (ctx->remaining < 0) {
+ apr_brigade_cleanup(bb);
+ e = ap_bucket_error_create(HTTP_REQUEST_ENTITY_TOO_LARGE, NULL,
+ f->r->connection->pool,
+ f->r->connection->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ e = apr_bucket_eos_create(f->r->connection->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ return ap_pass_brigade(f->r->output_filters, bb);
+ }
}
}
@@ -890,6 +901,17 @@
apr_brigade_flatten(bb, line, &len);
ctx->remaining = get_chunk_size(line);
+ /* Detect invalid chunk sizes. */
+ if (ctx->remaining < 0) {
+ apr_brigade_cleanup(bb);
+ e = ap_bucket_error_create(HTTP_REQUEST_ENTITY_TOO_LARGE,
+ NULL, c->pool, c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ e = apr_bucket_eos_create(c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ return ap_pass_brigade(f->r->output_filters, bb);
+ }
+
if (!ctx->remaining) {
/* Handle trailers by calling ap_get_mime_headers again! */
ctx->state = BODY_NONE;
@@ -1776,6 +1798,17 @@
AP_DECLARE(int) ap_discard_request_body(request_rec *r)
{
int 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
+ * that's the case, don't read the data as that is what we're trying
+ * to avoid.
+ *
+ * This function is also a no-op on a subrequest.
+ */
+ if (r->main || ap_status_drops_connection(r->status)) {
+ return OK;
+ }
if (r->read_length == 0) { /* if not read already */
if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK))) {
1.144 +9 -6 httpd-2.0/modules/http/http_request.c
Index: http_request.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_request.c,v
retrieving revision 1.143
retrieving revision 1.144
diff -u -r1.143 -r1.144
--- http_request.c 17 May 2002 11:33:10 -0000 1.143
+++ http_request.c 28 May 2002 23:38:31 -0000 1.144
@@ -139,13 +139,16 @@
r->status = HTTP_PROXY_AUTHENTICATION_REQUIRED;
}
- /*
- * If we want to keep the connection, be sure that the request body
- * (if any) has been read.
+ /* If we don't want to keep the connection, make sure we mark that the
+ * connection is not eligible for keepalive. If we want to keep the
+ * connection, be sure that the request body (if any) has been read.
*/
- if ((r->status != HTTP_NOT_MODIFIED) && (r->status != HTTP_NO_CONTENT)
- && !ap_status_drops_connection(r->status)
- && r->connection && (r->connection->keepalive != -1)) {
+ if (ap_status_drops_connection(r->status)) {
+ r->connection->keepalive = 0;
+ }
+ else if ((r->status != HTTP_NOT_MODIFIED) &&
+ (r->status != HTTP_NO_CONTENT) &&
+ r->connection && (r->connection->keepalive != -1)) {
(void) ap_discard_request_body(r);
}
Re: cvs commit: httpd-2.0/modules/http http_protocol.c http_request.c
Posted by Aaron Bannert <aa...@clove.org>.
On Tue, May 28, 2002 at 11:38:31PM -0000, Justin Erenkrantz wrote:
> jerenkrantz 02/05/28 16:38:31
>
> Modified: . CHANGES
> modules/http http_protocol.c http_request.c
> Log:
> Correctly return 413 when an invalid chunk size is given on input.
>
> - If get_chunk_size() returns a negative number, that probably implies
> an overflow. So, create a 413 error and pass it to the output filters.
I think get_chunk_size() should make sure that it's not overflowing
a long, and then return failure if that happens. I'll see if I can
produce a patch soonish.
-aaron
Re: cvs commit: httpd-2.0/modules/http http_protocol.c http_request.c
Posted by Justin Erenkrantz <je...@apache.org>.
On Tue, May 28, 2002 at 11:38:31PM -0000, jerenkrantz@apache.org wrote:
> jerenkrantz 02/05/28 16:38:31
>
> Modified: . CHANGES
> modules/http http_protocol.c http_request.c
> Log:
> Correctly return 413 when an invalid chunk size is given on input.
>
> - If get_chunk_size() returns a negative number, that probably implies
> an overflow. So, create a 413 error and pass it to the output filters.
> - Modify ap_discard_request_body() to return OK quickly if we're a subreq
> or our status code implies that we will be dropping the connection.
> - Modify ap_die() so that if the new status implies that we will drop
> the connection, that we correctly indicate that we can not keepalive
> this connection. (Without this, the error is returned, but the connection
> is not closed.)
This should resolve the invalid chunk line problem by returning 413.
I ran with httpd-test and all of the tests passed - except for some
SSL tests which are still broken for me due to my OpenSSL version.
Please let me know if you find any problems with this. This took
a LOT longer than it should have. =( -- justin