You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2009/08/27 18:22:45 UTC
DO NOT REPLY [Bug 47087] Incorrect request body handling with
Expect: 100-continue if the client does not receive a transmitted 300 or 400
response prior to sending its body
https://issues.apache.org/bugzilla/show_bug.cgi?id=47087
--- Comment #1 from Stuart A. Malone <sa...@llamagraphics.com> 2009-08-27 09:22:42 PDT ---
I also ran into this bug when trying to write a simple WebDAV client using
Microsoft's .NET Framework. I have a very simple C# test program that
demonstrates the problem, and have Wireshark network traces that demonstrate
that the error is in Apache rather than .NET.
You can read the discussion and find links to the network traces on the MSDN
forums at:
<http://social.msdn.microsoft.com/Forums/en-US/ncl/thread/5c576a2d-2f13-485c-8ada-b4c3ee127d3c/>
I just re-ran the test against Apache 2.2.13 on Mac OS X Snow Leopard, and the
problem is still there.
It appears that Apache is violating this paragraph from RFC 2616:
- Upon receiving a request which includes an Expect request-header
field with the "100-continue" expectation, an origin server MUST
either respond with 100 (Continue) status and continue to read
from the input stream, or respond with a final status code. The
origin server MUST NOT wait for the request body before sending
the 100 (Continue) response. If it responds with a final status
code, it MAY close the transport connection or it MAY continue
to read and discard the rest of the request. It MUST NOT
perform the requested method if it returns a final status code.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
Re: [Bug 47087] Incorrect request body handling with Expect: 100-continue if the client does not receive a transmitted 300 or 400 response prior to sending its body
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Aug 29, 2009, at 11:03 PM, Graham Dumpleton wrote:
> 2009/8/30 Nick Kew <ni...@webthing.com>:
>>
>> On 27 Aug 2009, at 17:22, bugzilla@apache.org wrote:
>>
>>> It appears that Apache is violating this paragraph from RFC 2616:
>>>
>>> - Upon receiving a request which includes an Expect request-
>>> header
>>> field with the "100-continue" expectation, an origin
>>> server MUST
>>> either respond with 100 (Continue) status and continue to
>>> read
>>> from the input stream, or respond with a final status
>>> code. The
>>> origin server MUST NOT wait for the request body before
>>> sending
>>> the 100 (Continue) response. If it responds with a final
>>> status
>>> code, it MAY close the transport connection or it MAY
>>> continue
>>> to read and discard the rest of the request. It MUST NOT
>>> perform the requested method if it returns a final status
>>> code.
>>
>> Looks like we have a problem with the sequence:
>> Client asks for 100-continue
>> We reply with a final status - e.g. 3xx
>> [delay somewhere on the wire]
>> Client sends a request body
>> We read body as a new request - oops!
>>
>> It seems to me that keeping the connection open in this
>> instance means inevitable ambiguity over interpretation
>> of subsequent data, and the safe course of action is to
>> close it. Otherwise we can read subsequent data line-
>> by-line and discard anything that isn't a valid request
>> line, at the risk of encountering a false positive in a
>> request body.
>>
>> +1 for closing the connection. Any divergent opinions?
>
> FWIW, this area of code was change somewhere between 2.2.6 and 2.2.9
> already. Prior to the change if a handler sent a 20x response and then
> only after sending it did it attempt to read request input, the 100
> was being emitted as part of the response content.
>
> The old code in http_filters.c was:
>
> /* Since we're about to read data, send 100-Continue if
> needed.
> * Only valid on chunked and C-L bodies where the C-L is >
> 0. */
> if ((ctx->state == BODY_CHUNK ||
> (ctx->state == BODY_LENGTH && ctx->remaining > 0)) &&
> f->r->expecting_100 && f->r->proto_num >= HTTP_VERSION
> (1,1)) {
> char *tmp;
> apr_bucket_brigade *bb;
>
> tmp = apr_pstrcat(f->r->pool, AP_SERVER_PROTOCOL, " ",
> ap_get_status_line(100), CRLF CRLF,
> NULL);
> bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
> e = apr_bucket_pool_create(tmp, strlen(tmp), f->r->pool,
> f->c->bucket_alloc);
> APR_BRIGADE_INSERT_HEAD(bb, e);
> e = apr_bucket_flush_create(f->c->bucket_alloc);
> APR_BRIGADE_INSERT_TAIL(bb, e);
>
> ap_pass_brigade(f->c->output_filters, bb);
> }
>
> and is now:
>
> /* Since we're about to read data, send 100-Continue if
> needed.
> * Only valid on chunked and C-L bodies where the C-L is >
> 0. */
> if ((ctx->state == BODY_CHUNK ||
> (ctx->state == BODY_LENGTH && ctx->remaining > 0)) &&
> f->r->expecting_100 && f->r->proto_num >= HTTP_VERSION
> (1,1) &&
> !(f->r->eos_sent || f->r->bytes_sent)) {
> if (!ap_is_HTTP_SUCCESS(f->r->status)) {
> ctx->state = BODY_NONE;
> ctx->eos_sent = 1;
> } else {
> char *tmp;
>
> tmp = apr_pstrcat(f->r->pool, AP_SERVER_PROTOCOL, " ",
> ap_get_status_line(100), CRLF
> CRLF, NULL);
> apr_brigade_cleanup(bb);
> e = apr_bucket_pool_create(tmp, strlen(tmp), f->r-
> >pool,
> f->c->bucket_alloc);
> APR_BRIGADE_INSERT_HEAD(bb, e);
> e = apr_bucket_flush_create(f->c->bucket_alloc);
> APR_BRIGADE_INSERT_TAIL(bb, e);
>
> ap_pass_brigade(f->c->output_filters, bb);
> }
> }
>
> The fix was needed for case where handler needs to start streaming a
> response before it starts processing request content.
Well, that explains why the code doesn't work any more.
> If there is no valid reason why for non 20x response that a handler
> should be able to read request content after having sent a response,
> then closing the connection seems logical thing to do to avoid Apache
> having to read the whole request content and discard it, just to
> handle a potential request over same connection after it.
The problem with that theory is that TCP reset behavior requires
us to read a substantial amount of the request anyway, just to
ensure that the client receives the final response. However,
it is probably safer to tell the client to use a new connection
for the next request.
In any case, if we failed to read the request then we must mark
the connection as soiled. What the above does is pretend that
there isn't a body and then leaves the connection in a
half-read state.
The solution should be something like
if (!ap_is_HTTP_SUCCESS(f->r->status)) {
ctx->state = BODY_NONE;
ctx->eos_sent = 1;
f->c->keepalive = AP_CONN_CLOSE;
}
but I haven't tested that.
....Roy
Re: DO NOT REPLY [Bug 47087] Incorrect request body handling with
Expect: 100-continue if the client does not receive a transmitted 300 or 400
response prior to sending its body
Posted by Graham Dumpleton <gr...@gmail.com>.
2009/8/30 Nick Kew <ni...@webthing.com>:
>
> On 27 Aug 2009, at 17:22, bugzilla@apache.org wrote:
>
>> It appears that Apache is violating this paragraph from RFC 2616:
>>
>> - Upon receiving a request which includes an Expect request-header
>> field with the "100-continue" expectation, an origin server MUST
>> either respond with 100 (Continue) status and continue to read
>> from the input stream, or respond with a final status code. The
>> origin server MUST NOT wait for the request body before sending
>> the 100 (Continue) response. If it responds with a final status
>> code, it MAY close the transport connection or it MAY continue
>> to read and discard the rest of the request. It MUST NOT
>> perform the requested method if it returns a final status code.
>
> Looks like we have a problem with the sequence:
> Client asks for 100-continue
> We reply with a final status - e.g. 3xx
> [delay somewhere on the wire]
> Client sends a request body
> We read body as a new request - oops!
>
> It seems to me that keeping the connection open in this
> instance means inevitable ambiguity over interpretation
> of subsequent data, and the safe course of action is to
> close it. Otherwise we can read subsequent data line-
> by-line and discard anything that isn't a valid request
> line, at the risk of encountering a false positive in a
> request body.
>
> +1 for closing the connection. Any divergent opinions?
FWIW, this area of code was change somewhere between 2.2.6 and 2.2.9
already. Prior to the change if a handler sent a 20x response and then
only after sending it did it attempt to read request input, the 100
was being emitted as part of the response content.
The old code in http_filters.c was:
/* Since we're about to read data, send 100-Continue if needed.
* Only valid on chunked and C-L bodies where the C-L is > 0. */
if ((ctx->state == BODY_CHUNK ||
(ctx->state == BODY_LENGTH && ctx->remaining > 0)) &&
f->r->expecting_100 && f->r->proto_num >= HTTP_VERSION(1,1)) {
char *tmp;
apr_bucket_brigade *bb;
tmp = apr_pstrcat(f->r->pool, AP_SERVER_PROTOCOL, " ",
ap_get_status_line(100), CRLF CRLF, NULL);
bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
e = apr_bucket_pool_create(tmp, strlen(tmp), f->r->pool,
f->c->bucket_alloc);
APR_BRIGADE_INSERT_HEAD(bb, e);
e = apr_bucket_flush_create(f->c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, e);
ap_pass_brigade(f->c->output_filters, bb);
}
and is now:
/* Since we're about to read data, send 100-Continue if needed.
* Only valid on chunked and C-L bodies where the C-L is > 0. */
if ((ctx->state == BODY_CHUNK ||
(ctx->state == BODY_LENGTH && ctx->remaining > 0)) &&
f->r->expecting_100 && f->r->proto_num >= HTTP_VERSION(1,1) &&
!(f->r->eos_sent || f->r->bytes_sent)) {
if (!ap_is_HTTP_SUCCESS(f->r->status)) {
ctx->state = BODY_NONE;
ctx->eos_sent = 1;
} else {
char *tmp;
tmp = apr_pstrcat(f->r->pool, AP_SERVER_PROTOCOL, " ",
ap_get_status_line(100), CRLF CRLF, NULL);
apr_brigade_cleanup(bb);
e = apr_bucket_pool_create(tmp, strlen(tmp), f->r->pool,
f->c->bucket_alloc);
APR_BRIGADE_INSERT_HEAD(bb, e);
e = apr_bucket_flush_create(f->c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, e);
ap_pass_brigade(f->c->output_filters, bb);
}
}
The fix was needed for case where handler needs to start streaming a
response before it starts processing request content.
If there is no valid reason why for non 20x response that a handler
should be able to read request content after having sent a response,
then closing the connection seems logical thing to do to avoid Apache
having to read the whole request content and discard it, just to
handle a potential request over same connection after it.
Graham
Re: DO NOT REPLY [Bug 47087] Incorrect request body handling with Expect: 100-continue if the client does not receive a transmitted 300 or 400 response prior to sending its body
Posted by Nick Kew <ni...@webthing.com>.
On 27 Aug 2009, at 17:22, bugzilla@apache.org wrote:
> It appears that Apache is violating this paragraph from RFC 2616:
>
> - Upon receiving a request which includes an Expect request-
> header
> field with the "100-continue" expectation, an origin server
> MUST
> either respond with 100 (Continue) status and continue to read
> from the input stream, or respond with a final status code.
> The
> origin server MUST NOT wait for the request body before
> sending
> the 100 (Continue) response. If it responds with a final
> status
> code, it MAY close the transport connection or it MAY continue
> to read and discard the rest of the request. It MUST NOT
> perform the requested method if it returns a final status
> code.
Looks like we have a problem with the sequence:
Client asks for 100-continue
We reply with a final status - e.g. 3xx
[delay somewhere on the wire]
Client sends a request body
We read body as a new request - oops!
It seems to me that keeping the connection open in this
instance means inevitable ambiguity over interpretation
of subsequent data, and the safe course of action is to
close it. Otherwise we can read subsequent data line-
by-line and discard anything that isn't a valid request
line, at the risk of encountering a false positive in a
request body.
+1 for closing the connection. Any divergent opinions?
--
Nick Kew