On 01/06/2008 11:39 AM, Ruediger Pluem wrote:
>
> On 01/06/2008 02:20 AM, Nick Kew wrote:
>> On Sat, 05 Jan 2008 20:28:33 +0100
>> Ruediger Pluem <[EMAIL PROTECTED]> wrote:
>>
>>> On 01/05/2008 07:04 PM, Nick Kew wrote:
>>>> On Sat, 05 Jan 2008 12:38:58 +0100
>>>> Ruediger Pluem <[EMAIL PROTECTED]> wrote:
>>>>
>>>>> Ok. Can you setup a tcpdump between proxy and server and between
>>>>> client and proxy? I guess the network traces would be very helpful
>>>>> in finding out where things are starting to get wrong.
>>>> One testcase with its tcpdump at
>>>> http://people.apache.org/~niq/2.2.7/
>>> Thanks for this, but I think this is not sufficient:
>>>
>>> 1. It seems the dump is incomplete as I cannot see a 0 chunk at the
>>> end. 2. I would prefer the binary dump as it offers more
>>> possibilities to analyse it with wireshark.
>>>
>>> Sorry for being that demanding :-)
>> Do you mean as in tcpdump -x? I've uploaded a pair of dumps
>> (one of client-proxy, the other of proxy-server) at the same
>> location.
>
>
Ok, next one. I missed to set the correct state in some situations.
Can you please give it a try again?
Regards
Rüdiger
Index: modules/http/http_filters.c
===================================================================
--- modules/http/http_filters.c (Revision 609355)
+++ 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,17 @@
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);
+ ctx->state = BODY_CHUNK_PART;
+ return rv;
+ }
+ if (rv == APR_SUCCESS) {
+ ctx->remaining = get_chunk_size(ctx->chunk_ln);
+ }
}
}
}
@@ -308,9 +351,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 +362,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 +394,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 +402,15 @@
http_error = HTTP_SERVICE_UNAVAILABLE;
}
else {
- ctx->remaining = get_chunk_size(line);
+ rv = get_chunk_line(ctx, len);
+ if (APR_STATUS_IS_EAGAIN(rv)) {
+ ctx->state = BODY_CHUNK_PART;
+ apr_brigade_cleanup(bb);
+ return rv;
+ }
+ if (rv == APR_SUCCESS) {
+ ctx->remaining = get_chunk_size(ctx->chunk_ln);
+ }
}
}
}
