2009/8/30 Nick Kew <[email protected]>:
>
> On 27 Aug 2009, at 17:22, [email protected] 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
