On 26.04.2010 08:04, Roy T. Fielding wrote: > I am confused. Why is this a good idea? Why is it unexpected to > encounter a 413 response after a timeout due to a read of chunked > body, and how does changing it to a 400 somehow prevent a double > errordoc? Why not just fix the double errordoc bug instead of > the case that triggers it?
Changing to 400 does not fix the double error message problem. In fact this patch addresses two things in one and it might have been worth doing two commits for it. I am partly to blame for this as I encouraged Eric in a Bugzilla entry to commit this patch. The part that fixes the double error messages is: >> - bail_out_on_error(ctx, f, http_error); >> - return rv; >> + return bail_out_on_error(ctx, f, http_error); Regards Rüdiger > > ....Roy > > On Apr 25, 2010, at 12:10 PM, [email protected] wrote: > >> Author: covener >> Date: Sun Apr 25 19:10:45 2010 >> New Revision: 937858 >> >> URL: http://svn.apache.org/viewvc?rev=937858&view=rev >> Log: >> PR49167, unexpected 413 and double-errordoc during a timeout reading a >> chunk-size. >> >> >> Modified: >> httpd/httpd/trunk/CHANGES >> httpd/httpd/trunk/modules/http/http_filters.c >> >> Modified: httpd/httpd/trunk/CHANGES >> URL: >> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=937858&r1=937857&r2=937858&view=diff >> ============================================================================== >> --- httpd/httpd/trunk/CHANGES [utf-8] (original) >> +++ httpd/httpd/trunk/CHANGES [utf-8] Sun Apr 25 19:10:45 2010 >> @@ -28,6 +28,11 @@ Changes with Apache 2.3.7 >> processing is completed, avoiding orphaned callback pointers. >> [Brett Gervasoni <brettg senseofsecurity.com>, Jeff Trawick] >> >> + *) Log an error for failures to read a chunk-size, and return 400 instead >> + 413 when this is due to a read timeout. This change also fixes some >> cases >> + of two error documents being sent in the response for the same >> scenario. >> + [Eric Covener] PR49167 >> + >> *) mod_proxy_balancer: Add new directive BalancerNonce to allow admin >> to control/set the nonce used in the balancer-manager application. >> [Jim Jagielski] >> >> Modified: httpd/httpd/trunk/modules/http/http_filters.c >> URL: >> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=937858&r1=937857&r2=937858&view=diff >> ============================================================================== >> --- httpd/httpd/trunk/modules/http/http_filters.c (original) >> +++ httpd/httpd/trunk/modules/http/http_filters.c Sun Apr 25 19:10:45 2010 >> @@ -383,8 +383,13 @@ apr_status_t ap_http_filter(ap_filter_t >> >> /* Detect chunksize error (such as overflow) */ >> if (rv != APR_SUCCESS || ctx->remaining < 0) { >> + ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, f->r, "Error >> reading first chunk %s ", >> + (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_BAD_REQUEST; >> + } >> return bail_out_on_error(ctx, f, http_error); >> } >> >> @@ -484,10 +489,14 @@ apr_status_t ap_http_filter(ap_filter_t >> >> /* Detect chunksize error (such as overflow) */ >> if (rv != APR_SUCCESS || ctx->remaining < 0) { >> + ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, f->r, "Error >> reading chunk %s ", >> + (ctx->remaining < 0) ? "(overflow)" : ""); >> ctx->remaining = 0; /* Reset it in case we have to >> * come back here later */ >> - bail_out_on_error(ctx, f, http_error); >> - return rv; >> + if (APR_STATUS_IS_TIMEUP(rv)) { >> + http_error = HTTP_BAD_REQUEST; >> + } >> + return bail_out_on_error(ctx, f, http_error); >> } >> >> if (!ctx->remaining) { >> >> > >
