httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] chunking filter.
Date Wed, 16 Aug 2000 19:49:36 GMT
On Wed, Aug 16, 2000 at 12:02:11PM -0700, rbb@covalent.net wrote:
> This patch adds a chunking filter to Apache.  This is a pretty stupid
> filter, and it would probably be good to add some more smarts to the
> filter.  I think more benefit would be added by modifying the original
> handler however.

A couple suggestions are below.

> I tried to remove most of the BUFF code for chunking, but I didn't work to
> hard at that, because I expect the BUFF code to go away altogether very
> soon.

The BUFF code you removed looks about right. Basically, it is pretending
that B_CHUNK is never set. You can remove the set/clear of B_CHUNK from
http_protocol.c (the only besides modules/test/mod_rndchunk.c; this module
should be checked/revamped as appropriate).

Once B_CHUNK is gone, then you can completely toss start_chunk, end_chunk,
and large_write_chunk (plus other minor bits here and there).

> This does work as far as I can tell.  There is one extra 0 added at the
> end of response, but I think that is because I haven't completely removed
> the BUFF chunking code.  At least, I can't find any other reason for it.

The extra 0 is part of the protocol. It signifies "no more chunks". There is
also an extra CRLF pair to signify "no trailer headers".

These values are being sent by ap_finalize_request_protocol(). The code
there should be moved into chunk_filter() and sent when the EOS bucket is
seen.

>...
> +static int chunk_filter(ap_filter_t *f, ap_bucket_brigade *b)
> +{
> +    ap_bucket *dptr = b->head;
> +    int len = 0;
> +    char lenstr[6];
> +    int hit_eos = 0;
> +
> +    while (dptr) {
> +        if (dptr->type == AP_BUCKET_EOS) {
> +            hit_eos = 1;

use this to determine whether to add the "0" CRLF <trailers> CRLF sequence.

> +        } 
> +        else {
> +            len += dptr->length;
> +        }
> +        dptr = dptr->next;
> +    }
> +
> +    apr_snprintf(lenstr, 6, "%x\r\n", len);
> +    dptr = ap_bucket_transient_create(lenstr, 4, &len);
> +    b->head->prev = dptr;
> +    dptr->next = b->head;
> +    b->head = dptr;

can we get an ap_brigade_prepend_bucket() function for this?

> +    dptr = ap_bucket_heap_create("\r\n", 2, &len);

how about writing the IMMORTAL bucket first? the above bucket should really
be IMMORTAL.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message