You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2011/05/06 15:14:27 UTC
svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS
modules/http/http_filters.c server/protocol.c
Author: covener
Date: Fri May 6 13:14:27 2011
New Revision: 1100200
URL: http://svn.apache.org/viewvc?rev=1100200&view=rev
Log:
Merge r820760, r919323, r937858, r938265 from trunk:
Reviewed By: sf, trawick, covener
core: Treat timeout reading request as 408 error, not 400.
Log 408 errors in access log as was done in Apache 1.3.x.
PR: 39785
Submitted by: Nobutaka Mantani, Stefan Fritsch
Reviewed and added to by: Dan Poirier
* Only log a 408 if it is no keepalive timeout.
PR: 39785
Submitted by: Mark Montague <markmont umich.edu>, rpluem
Reviewed by: rpluem
PR49167, unexpected 413 and double-errordoc during a timeout reading a
chunk-size.
Use the more specific 408 (timed out) instead of a generic 400 during a timeout
reading a chunk-length.
Modified:
httpd/httpd/branches/2.2.x/CHANGES
httpd/httpd/branches/2.2.x/STATUS
httpd/httpd/branches/2.2.x/modules/http/http_filters.c
httpd/httpd/branches/2.2.x/server/protocol.c
Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=1100200&r1=1100199&r2=1100200&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Fri May 6 13:14:27 2011
@@ -1,6 +1,19 @@
-*- coding: utf-8 -*-
Changes with Apache 2.2.18
+ *) Log an error for failures to read a chunk-size, and return 408 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
+
+ *) core: Only log a 408 if it is no keepalive timeout. PR 39785
+ [Ruediger Pluem, Mark Montague <markmont umich.edu>]
+
+ *) core: Treat timeout reading request as 408 error, not 400.
+ Log 408 errors in access log as was done in Apache 1.3.x.
+ PR 39785 [Nobutaka Mantani <nobutaka nobutaka.org>, Stefan Fritsch,
+ Dan Poirier]
+
*) Core HTTP: disable keepalive when the Client has sent
Expect: 100-continue
but we respond directly with a non-100 response. Keepalive here led
Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1100200&r1=1100199&r2=1100200&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Fri May 6 13:14:27 2011
@@ -108,14 +108,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
2.2.x patch: Trunk patch applies with offset
+1: trawick, wrowe, covener
- * core: Send error 408 instead of 400 or 413 when appropriate
- Trunk patches: http://svn.apache.org/viewvc?view=revision&revision=820760
- http://svn.apache.org/viewvc?view=revision&revision=919323
- http://svn.apache.org/viewvc?view=revision&revision=937858
- http://svn.apache.org/viewvc?view=revision&revision=938265
- 2.2.x patch: http://people.apache.org/~sf/408.diff
- +1: sf, trawick, covener
-
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
[ New proposals should be added at the end of the list ]
Modified: httpd/httpd/branches/2.2.x/modules/http/http_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/http/http_filters.c?rev=1100200&r1=1100199&r2=1100200&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/http/http_filters.c (original)
+++ httpd/httpd/branches/2.2.x/modules/http/http_filters.c Fri May 6 13:14:27 2011
@@ -384,8 +384,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_REQUEST_TIME_OUT;
+ }
return bail_out_on_error(ctx, f, http_error);
}
@@ -485,10 +490,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_REQUEST_TIME_OUT;
+ }
+ return bail_out_on_error(ctx, f, http_error);
}
if (!ctx->remaining) {
Modified: httpd/httpd/branches/2.2.x/server/protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200&r1=1100199&r2=1100200&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/server/protocol.c (original)
+++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May 6 13:14:27 2011
@@ -608,6 +608,9 @@ static int read_request_line(request_rec
r->proto_num = HTTP_VERSION(1,0);
r->protocol = apr_pstrdup(r->pool, "HTTP/1.0");
}
+ else if (rv == APR_TIMEUP) {
+ r->status = HTTP_REQUEST_TIME_OUT;
+ }
return 0;
}
} while ((len <= 0) && (++num_blank_lines < max_blank_lines));
@@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
&len, r, 0, bb);
if (rv != APR_SUCCESS) {
- r->status = HTTP_BAD_REQUEST;
+ if (rv == APR_TIMEUP) {
+ r->status = HTTP_REQUEST_TIME_OUT;
+ }
+ else {
+ r->status = HTTP_BAD_REQUEST;
+ }
/* ap_rgetline returns APR_ENOSPC if it fills up the buffer before
* finding the end-of-line. This is only going to happen if it
@@ -877,7 +885,7 @@ request_rec *ap_read_request(conn_rec *c
r->read_length = 0;
r->read_body = REQUEST_NO_BODY;
- r->status = HTTP_REQUEST_TIME_OUT; /* Until we get a request */
+ r->status = HTTP_OK; /* Until further notice */
r->the_request = NULL;
/* Begin by presuming any module can make its own path_info assumptions,
@@ -898,6 +906,14 @@ request_rec *ap_read_request(conn_rec *c
apr_brigade_destroy(tmp_bb);
return r;
}
+ else if (r->status == HTTP_REQUEST_TIME_OUT) {
+ ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
+ if (!r->connection->keepalives) {
+ ap_run_log_transaction(r);
+ }
+ apr_brigade_destroy(tmp_bb);
+ return r;
+ }
apr_brigade_destroy(tmp_bb);
return NULL;
@@ -916,7 +932,7 @@ request_rec *ap_read_request(conn_rec *c
if (!r->assbackwards) {
ap_get_mime_headers_core(r, tmp_bb);
- if (r->status != HTTP_REQUEST_TIME_OUT) {
+ if (r->status != HTTP_OK) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"request failed: error reading the headers");
ap_send_error_response(r, 0);
@@ -957,8 +973,6 @@ request_rec *ap_read_request(conn_rec *c
apr_brigade_destroy(tmp_bb);
- r->status = HTTP_OK; /* Until further notice. */
-
/* update what we think the virtual host is based on the headers we've
* now read. may update status.
*/
Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES
STATUS modules/http/http_filters.c server/protocol.c
Posted by Eric Covener <co...@gmail.com>.
On Sat, May 7, 2011 at 7:08 AM, Rainer Jung <ra...@kippdata.de> wrote:
> On 07.05.2011 11:57, William A. Rowe Jr. wrote:
>>
>> On 5/7/2011 12:20 AM, Ruediger Pluem wrote:
>>>
>>>
>>> On 05/06/2011 03:14 PM, covener@apache.org wrote:
>>>>
>>>> Author: covener
>>>> Date: Fri May 6 13:14:27 2011
>>>> New Revision: 1100200
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1100200&view=rev
>>>> Log:
>>>> Merge r820760, r919323, r937858, r938265 from trunk:
>>>>
>>>> Reviewed By: sf, trawick, covener
>>
>>>> Modified: httpd/httpd/branches/2.2.x/server/protocol.c
>>>> URL:
>>>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200&r1=1100199&r2=1100200&view=diff
>>>>
>>>> ==============================================================================
>>>> --- httpd/httpd/branches/2.2.x/server/protocol.c (original)
>>>> +++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May 6 13:14:27
>>>> 2011
>>>> @@ -608,6 +608,9 @@ static int read_request_line(request_rec
>>>> r->proto_num = HTTP_VERSION(1,0);
>>>> r->protocol = apr_pstrdup(r->pool, "HTTP/1.0");
>>>> }
>>>> + else if (rv == APR_TIMEUP) {
>>
>>>> @@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
>>>> &len, r, 0, bb);
>>>>
>>>> if (rv != APR_SUCCESS) {
>>>> - r->status = HTTP_BAD_REQUEST;
>>>> + if (rv == APR_TIMEUP) {
>>>
>>> As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
>>> Didn't we have a security issue on Windows and Netware because of this?
>>
>> Absolutely; +1 to expedite this patch; with a third +1 I'll commit.
>>
>> Bill
>
> +1 to change from comparison with APR_TIMEUP to APR_STATUS_IS_TIMEUP in both
> places in protocol.c. Note this applies to trunk and 2.2.
>
> Two more recent APR_TIMEUP additions are in trunk, Ruediger commented on
> them Re r1092076 on APril 23rd. I'd say they should be fixed as well.
>
> Regards,
>
> Rainer
>
third +1 and will backport and link to this thread.
--
Eric Covener
covener@gmail.com
Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES
STATUS modules/http/http_filters.c server/protocol.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 07.05.2011 11:57, William A. Rowe Jr. wrote:
> On 5/7/2011 12:20 AM, Ruediger Pluem wrote:
>>
>>
>> On 05/06/2011 03:14 PM, covener@apache.org wrote:
>>> Author: covener
>>> Date: Fri May 6 13:14:27 2011
>>> New Revision: 1100200
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1100200&view=rev
>>> Log:
>>> Merge r820760, r919323, r937858, r938265 from trunk:
>>>
>>> Reviewed By: sf, trawick, covener
>
>>> Modified: httpd/httpd/branches/2.2.x/server/protocol.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200&r1=1100199&r2=1100200&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/branches/2.2.x/server/protocol.c (original)
>>> +++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May 6 13:14:27 2011
>>> @@ -608,6 +608,9 @@ static int read_request_line(request_rec
>>> r->proto_num = HTTP_VERSION(1,0);
>>> r->protocol = apr_pstrdup(r->pool, "HTTP/1.0");
>>> }
>>> + else if (rv == APR_TIMEUP) {
>
>>> @@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
>>> &len, r, 0, bb);
>>>
>>> if (rv != APR_SUCCESS) {
>>> - r->status = HTTP_BAD_REQUEST;
>>> + if (rv == APR_TIMEUP) {
>>
>> As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
>> Didn't we have a security issue on Windows and Netware because of this?
>
> Absolutely; +1 to expedite this patch; with a third +1 I'll commit.
>
> Bill
+1 to change from comparison with APR_TIMEUP to APR_STATUS_IS_TIMEUP in
both places in protocol.c. Note this applies to trunk and 2.2.
Two more recent APR_TIMEUP additions are in trunk, Ruediger commented on
them Re r1092076 on APril 23rd. I'd say they should be fixed as well.
Regards,
Rainer
Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES
STATUS modules/http/http_filters.c server/protocol.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 5/7/2011 12:20 AM, Ruediger Pluem wrote:
>
>
> On 05/06/2011 03:14 PM, covener@apache.org wrote:
>> Author: covener
>> Date: Fri May 6 13:14:27 2011
>> New Revision: 1100200
>>
>> URL: http://svn.apache.org/viewvc?rev=1100200&view=rev
>> Log:
>> Merge r820760, r919323, r937858, r938265 from trunk:
>>
>> Reviewed By: sf, trawick, covener
>> Modified: httpd/httpd/branches/2.2.x/server/protocol.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200&r1=1100199&r2=1100200&view=diff
>> ==============================================================================
>> --- httpd/httpd/branches/2.2.x/server/protocol.c (original)
>> +++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May 6 13:14:27 2011
>> @@ -608,6 +608,9 @@ static int read_request_line(request_rec
>> r->proto_num = HTTP_VERSION(1,0);
>> r->protocol = apr_pstrdup(r->pool, "HTTP/1.0");
>> }
>> + else if (rv == APR_TIMEUP) {
>> @@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
>> &len, r, 0, bb);
>>
>> if (rv != APR_SUCCESS) {
>> - r->status = HTTP_BAD_REQUEST;
>> + if (rv == APR_TIMEUP) {
>
> As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
> Didn't we have a security issue on Windows and Netware because of this?
Absolutely; +1 to expedite this patch; with a third +1 I'll commit.
Bill
Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES
STATUS modules/http/http_filters.c server/protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/06/2011 03:14 PM, covener@apache.org wrote:
> Author: covener
> Date: Fri May 6 13:14:27 2011
> New Revision: 1100200
>
> URL: http://svn.apache.org/viewvc?rev=1100200&view=rev
> Log:
> Merge r820760, r919323, r937858, r938265 from trunk:
>
> Reviewed By: sf, trawick, covener
>
> core: Treat timeout reading request as 408 error, not 400.
> Log 408 errors in access log as was done in Apache 1.3.x.
>
> PR: 39785
> Submitted by: Nobutaka Mantani, Stefan Fritsch
> Reviewed and added to by: Dan Poirier
>
>
> * Only log a 408 if it is no keepalive timeout.
>
> PR: 39785
> Submitted by: Mark Montague <markmont umich.edu>, rpluem
> Reviewed by: rpluem
>
>
> PR49167, unexpected 413 and double-errordoc during a timeout reading a
> chunk-size.
>
>
>
> Use the more specific 408 (timed out) instead of a generic 400 during a timeout
> reading a chunk-length.
>
>
>
> Modified:
> httpd/httpd/branches/2.2.x/CHANGES
> httpd/httpd/branches/2.2.x/STATUS
> httpd/httpd/branches/2.2.x/modules/http/http_filters.c
> httpd/httpd/branches/2.2.x/server/protocol.c
>
>
> Modified: httpd/httpd/branches/2.2.x/server/protocol.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200&r1=1100199&r2=1100200&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/server/protocol.c (original)
> +++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May 6 13:14:27 2011
> @@ -608,6 +608,9 @@ static int read_request_line(request_rec
> r->proto_num = HTTP_VERSION(1,0);
> r->protocol = apr_pstrdup(r->pool, "HTTP/1.0");
> }
> + else if (rv == APR_TIMEUP) {
As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
Didn't we have a security issue on Windows and Netware because of this?
*) SECURITY: CVE-2010-2068 (cve.mitre.org)
mod_proxy_ajp, mod_proxy_http, mod_reqtimeout: Fix timeout detection
for platforms Windows, Netware and OS2. PR: 49417. [Rainer Jung]
> + r->status = HTTP_REQUEST_TIME_OUT;
> + }
> return 0;
> }
> } while ((len <= 0) && (++num_blank_lines < max_blank_lines));
> @@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
> &len, r, 0, bb);
>
> if (rv != APR_SUCCESS) {
> - r->status = HTTP_BAD_REQUEST;
> + if (rv == APR_TIMEUP) {
As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
Didn't we have a security issue on Windows and Netware because of this?
*) SECURITY: CVE-2010-2068 (cve.mitre.org)
mod_proxy_ajp, mod_proxy_http, mod_reqtimeout: Fix timeout detection
for platforms Windows, Netware and OS2. PR: 49417. [Rainer Jung]
> + r->status = HTTP_REQUEST_TIME_OUT;
> + }
> + else {
> + r->status = HTTP_BAD_REQUEST;
> + }
>
> /* ap_rgetline returns APR_ENOSPC if it fills up the buffer before
> * finding the end-of-line. This is only going to happen if it
Regards
Rüdiger