Chunked encoding and HTTP/1.0

[Christopher Lenz]

Hey all,

we're now using MochiWeb in CouchDB, and we're using chunked encoding
via mochiweb_request:respond(chunked). Now we're getting reports from
people behind HTTP/1.0 proxies (such as apparently nginx) that CouchDB
is sending chunked responses even to HTTP/1.0 clients. Doh :P

So we'd need to fallback to closing connections or setting the Content-
Length for such clients. The question is where that'd be best done.
I.e. should MochiWeb do the fallback itself, while still letting the
application use the chunked API? Or should CouchDB be doing this, the
disadvantage being that all MochiWeb-based applications would need the
same fallback logic.

Thoughts?
--
Christopher Lenz
cmlenz at gmx.de
http://www.cmlenz.net/

[Bob Ippolito]

I think that if you want to use chunked responses then you should be
either willing to deal with HTTP/1.0 separately or not at all. It
would probably be a good idea to check for HTTP/1.0 and die miserably
with some HTTP error response if the server tries to send a chunked
response (patches accepted!).

I don't think mochiweb should try and buffer chunked responses
indefinitely in order to determine the Content-Length since that could
be used to craft a denial of service attack on an application that
doesn't know better.

[Christopher Lenz]

On 25.04.2008, at 16:59, Bob Ippolito wrote:
> I think that if you want to use chunked responses then you should be
> either willing to deal with HTTP/1.0 separately or not at all. It
> would probably be a good idea to check for HTTP/1.0 and die miserably
> with some HTTP error response if the server tries to send a chunked
> response (patches accepted!).

I suspect lacking HTTP/1.1 support is more widespread for proxies than
I personally had thought/hoped. See:

<https://issues.apache.org/jira/browse/COUCHDB-40>

which explicitly mentions using nginx as the HTTP proxy. I have not
tried to reproduce the problems myself, but heard a couple others
having them on our mailing list. Anyway...

> I don't think mochiweb should try and buffer chunked responses
> indefinitely in order to determine the Content-Length since that could
> be used to craft a denial of service attack on an application that
> doesn't know better.

Good point. But what about the other fallback option: omit the Content-
Length for HTTP/1.0, and keep sending data without buffering (but also
without chunk delimiters), and finally simply close the connection at
the last "chunk" to clearly mark the end of the entity body?

That seems to be what google.com does for HTTP/1.0 requests, as far as
I can tell.

Thanks,

[Bob Ippolito]

That seems reasonable. A patch with tests would make this happen
faster. We don't need it because we don't use chunked anywhere in
production.

-bob

[Christopher Lenz]

On 26.04.2008, at 00:58, Bob Ippolito wrote:
>
>> Good point. But what about the other fallback option: omit the
>> Content-
>> Length for HTTP/1.0, and keep sending data without buffering (but
>> also
>> without chunk delimiters), and finally simply close the connection at
>> the last "chunk" to clearly mark the end of the entity body?
>>
>> That seems to be what google.com does for HTTP/1.0 requests, as far
>> as
>> I can tell.
>
> That seems reasonable. A patch with tests would make this happen
> faster. We don't need it because we don't use chunked anywhere in
> production.

I've put together a simple patch that seems to do the trick and
uploaded it to the CouchDB issue tracker:

<https://issues.apache.org/jira/secure/attachment/12381004/mochiweb_chunking_http10.diff
>

I'm hoping this will attract feedback from the people actually running
into the issue in the real world.

Not sure how to go about writing a test for this :/ Any hints?

BTW, the MochiWeb code seems to (increasingly?) contain mixed tabs and
spaces, which makes it kind of hairy to produce clean patches (and of
course screws up visual indentation in many cases). See the
mochiweb_request:respond(chunked) function that is modified by the
patch.

Cheers,

[Christopher Lenz]

On 27.04.2008, at 18:37, Christopher Lenz wrote:
> On 26.04.2008, at 00:58, Bob Ippolito wrote:
>> That seems reasonable. A patch with tests would make this happen
>> faster. We don't need it because we don't use chunked anywhere in
>> production.
>
> I've put together a simple patch that seems to do the trick and
> uploaded it to the CouchDB issue tracker:
>
> <https://issues.apache.org/jira/secure/attachment/12381004/mochiweb_chunking_http10.diff

And here's another version that also works when the request specifies
a Connection: Keep-Alive header:

<https://issues.apache.org/jira/secure/attachment/12381058/mochiweb_chunking_http10_2.diff
>

This adds a FORCE_CLOSE flag to the process dictionary. Not sure if
there's a better way to go about this.

[Bob Ippolito]

Ok, this is committed in r65. I also detabbed the source. The reason
being is that we changed our emacs config to use only spaces at some
point after mochiweb was written... so anything before that was Emacs'
weird idea of tab+spaces, and anything after that was just spaces. Of
course everything looks fine in Emacs either way :)

[Christopher Lenz]

On 29.04.2008, at 04:16, Bob Ippolito wrote:
> Ok, this is committed in r65. I also detabbed the source. The reason
> being is that we changed our emacs config to use only spaces at some
> point after mochiweb was written... so anything before that was Emacs'
> weird idea of tab+spaces, and anything after that was just spaces. Of
> course everything looks fine in Emacs either way :)

Thanks!