On 01/05/2008 12:27 PM, Nick Kew wrote:
> On Sat, 05 Jan 2008 12:11:07 +0100
> Ruediger Pluem <[EMAIL PROTECTED]> wrote:
>
>>>> Can you please try if the following patch against the vanilla tar
>>>> ball fixes the problem?
>>> Nope. Causes both test cases to give just one byte of response.
>> Sorry my first patch was bogus of course :-(. Please try the
>> following:
>
> Back to square one, with 433 bytes of response.
>
> (FWIW, I already tried moving the cleanup last night).
>
Ok, next round. I think that I ironed out various edge cases. One remains
(see XXX in get_chunk_line) but I guess this one could be addressed later.
So can you please test 2.2.7 vanilla + attached patch?
Regards
Rüdiger
Index: modules/http/http_filters.c
===================================================================
--- modules/http/http_filters.c (Revision 609114)
+++ modules/http/http_filters.c (Arbeitskopie)
@@ -68,8 +68,43 @@
BODY_CHUNK_PART
} state;
int eos_sent;
+ char chunk_ln[30];
+ char *pos;
} http_ctx_t;
+static apr_status_t get_chunk_line(http_ctx_t *ctx, int len)
+{
+ /*
+ * Check if we do not overflow our chunksize / empty line buffer
+ * (ctx->chunk_ln). If we do the chunksize / empty line is bogus anyway so
+ * bail out in this case.
+ * XXX: Currently we are very limited in accepting chunk-extensions. If
+ * this is needed the chunk_ln buffer must be much larger or we must
+ * find a different way to discard them as we do not process them anyway.
+ */
+ if ((ctx->pos - ctx->chunk_ln) + len < sizeof(ctx->chunk_ln)) {
+ ctx->pos += len;
+ *(ctx->pos) = '\0';
+ /*
+ * Check if we really got a full line. If yes the
+ * last char in the just read buffer must be LF.
+ * If not advance the buffer and return APR_EAGAIN.
+ * We do not start processing until we have the
+ * full line.
+ */
+ if (ctx->pos[-1] != APR_ASCII_LF) {
+ return APR_EAGAIN;
+ }
+ /*
+ * Line is complete. So reset ctx->pos for next round.
+ */
+ ctx->pos = ctx->chunk_ln;
+ return APR_SUCCESS;
+ }
+ return APR_ENOSPC;
+}
+
+
/* This is the HTTP_INPUT filter for HTTP requests and responses from
* proxied servers (mod_proxy). It handles chunked and content-length
* bodies. This can only be inserted/used after the headers
@@ -96,6 +131,7 @@
ctx->remaining = 0;
ctx->limit_used = 0;
ctx->eos_sent = 0;
+ ctx->pos = ctx->chunk_ln;
/* LimitRequestBody does not apply to proxied responses.
* Consider implementing this check in its own filter.
@@ -227,9 +263,8 @@
/* We can't read the chunk until after sending 100 if required. */
if (ctx->state == BODY_CHUNK) {
- char line[30];
apr_bucket_brigade *bb;
- apr_size_t len = 30;
+ apr_size_t len = sizeof(ctx->chunk_ln) - (ctx->pos - ctx->chunk_ln);
apr_off_t brigade_length;
bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
@@ -256,9 +291,16 @@
rv = APR_ENOSPC;
}
if (rv == APR_SUCCESS) {
- rv = apr_brigade_flatten(bb, line, &len);
+ rv = apr_brigade_flatten(bb, ctx->pos, &len);
if (rv == APR_SUCCESS) {
- ctx->remaining = get_chunk_size(line);
+ rv = get_chunk_line(ctx, len);
+ if (APR_STATUS_IS_EAGAIN(rv)) {
+ apr_brigade_cleanup(bb);
+ return rv;
+ }
+ if (rv == APR_SUCCESS) {
+ ctx->remaining = get_chunk_size(ctx->chunk_ln);
+ }
}
}
}
@@ -308,9 +350,9 @@
case BODY_CHUNK:
case BODY_CHUNK_PART:
{
- char line[30];
apr_bucket_brigade *bb;
- apr_size_t len = 30;
+ apr_size_t len = sizeof(ctx->chunk_ln)
+ - (ctx->pos - ctx->chunk_ln);
apr_status_t http_error = HTTP_REQUEST_ENTITY_TOO_LARGE;
bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
@@ -319,11 +361,21 @@
if (ctx->state == BODY_CHUNK) {
rv = ap_get_brigade(f->next, bb, AP_MODE_GETLINE,
block, 0);
- apr_brigade_cleanup(bb);
if (block == APR_NONBLOCK_READ &&
- (APR_STATUS_IS_EAGAIN(rv))) {
+ ( (rv == APR_SUCCESS && APR_BRIGADE_EMPTY(bb)) ||
+ (APR_STATUS_IS_EAGAIN(rv)) )) {
return APR_EAGAIN;
}
+ rv = apr_brigade_flatten(bb, ctx->pos, &len);
+ apr_brigade_cleanup(bb);
+ if (rv == APR_SUCCESS) {
+ rv = get_chunk_line(ctx, len);
+ if (APR_STATUS_IS_EAGAIN(rv)) {
+ return rv;
+ }
+ }
+ /* Reset len */
+ len = sizeof(ctx->chunk_ln) - (ctx->pos - ctx->chunk_ln);
} else {
rv = APR_SUCCESS;
}
@@ -341,7 +393,7 @@
}
ctx->state = BODY_CHUNK;
if (rv == APR_SUCCESS) {
- rv = apr_brigade_flatten(bb, line, &len);
+ rv = apr_brigade_flatten(bb, ctx->pos, &len);
if (rv == APR_SUCCESS) {
/* Wait a sec, that's a blank line! Oh no. */
if (!len) {
@@ -349,7 +401,14 @@
http_error = HTTP_SERVICE_UNAVAILABLE;
}
else {
- ctx->remaining = get_chunk_size(line);
+ rv = get_chunk_line(ctx, len);
+ if (APR_STATUS_IS_EAGAIN(rv)) {
+ apr_brigade_cleanup(bb);
+ return rv;
+ }
+ if (rv == APR_SUCCESS) {
+ ctx->remaining = get_chunk_size(ctx->chunk_ln);
+ }
}
}
}
