stream_unchunked_body discriminates against slow connections

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Try to upload a 1MB file w/o chunked encoding on a connection slower than 
100 KBps

What is the expected output? What do you see instead?
The upload should succeed, but Mochiweb times out if it does not receive 1MB in 
10 seconds.

What version of the product are you using? On what operating system?
I was using CouchDB's embedded version of Mochiweb (a fork of r97).  Any OS.

We discussed this a bit on the CouchDB developer list at

http://mail-archives.apache.org/mod_mbox/couchdb-dev/200911.mbox/%3cE86B310B-29C
A-
4BBC-A99E-3DBABC240372@apache.org%3e

and ended up switching to Apache's default behavior, which is to only trigger a 
timeout if no data 
is received in 5 minutes.  The patch I'm attaching that does part of this -- it 
will never trigger a 
timeout if server is still receiving data (that's the important imo), but it 
maintains MochiWeb's 
default ?IDLE_TIMEOUT of 10 seconds rather than the 5 minutes we went with for 
Couch.

Original issue reported on code.google.com by adam.kocoloski@gmail.com on 10 Nov 2009 at 2:35

Attachments:

• stream_unchunked_body_timeout.diff

[GoogleCodeExporter]

This patch doesn't look quite correct to me, what if the client sends too much 
data? I think a more robust 
solution will be more complicated.

Original comment by bob.ippo...@gmail.com on 10 Nov 2009 at 2:49

[GoogleCodeExporter]

Hi Bob, good point.  Here's an updated patch that checks the size of the 
received data before applying the Fun

Original comment by adam.kocoloski@gmail.com on 10 Nov 2009 at 3:20

[GoogleCodeExporter]

bah, bad patch ... here's the right one 

Original comment by adam.kocoloski@gmail.com on 10 Nov 2009 at 3:22

Attachments:

• stream_unchunked_body_timeout2.diff

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

But is it an error if the client pipelines two HTTP requests back to back? Most 
HTTP servers would handle this 
gracefully wouldn't they? mochiweb currently should (other than the potential 
timeout issue).

Original comment by bob.ippo...@gmail.com on 10 Nov 2009 at 3:22

[GoogleCodeExporter]

Sorry, not sure I follow ... are you thinking of GET requests with bodies, or 
PUT/POST requests that are pipelined?  
Both are unusual and I think the latter is a SHOULD NOT in the RFC.  I do see 
how pipelined requests with bodies 
would be troublesome with this patch.

Original comment by adam.kocoloski@gmail.com on 10 Nov 2009 at 3:26

[GoogleCodeExporter]

I think that it's possible to have an idempotent PUT request with a body that 
is pipelined, however rare it may be.

Original comment by bob.ippo...@gmail.com on 10 Nov 2009 at 3:35

[GoogleCodeExporter]

You're quite right, idempotence is what matters, and taking care to handle that 
use case will complicate things.

Original comment by adam.kocoloski@gmail.com on 10 Nov 2009 at 3:42

[GoogleCodeExporter]

Don't know how you feel about undocumented OTP features, but gen_tcp exports an 
unrecv/2 that seems to be 
exactly what is needed here.  This patch puts the extra data back on the socket 
if it ends up reading too much.

Still need to come up with a test that actually hits this code path.

Original comment by adam.kocoloski@gmail.com on 11 Nov 2009 at 2:32

Attachments:

• stream_unchunked_body_timeout_unrecv.diff

[GoogleCodeExporter]

applied in r113

Original comment by bob.ippo...@gmail.com on 15 Nov 2009 at 10:03

• Changed state: Fixed