View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0001296libmicrohttpdHTTP 1.1 request pipeliningpublic2007-11-27 17:182007-12-05 22:27
Reporterseangeo Assigned ToChristian Grothoff  
PriorityurgentSeverityblockReproducibilityalways
Status closedResolutionfixed 
Product Version0.1.1 
Fixed in Version0.1.2 
Summary0001296: Partially read requests get cut short
DescriptionI've been having a problem where a partially read post request would never get fully read. This only seemed to occur when lots of requests were coming in and a POST request would get partially read, i.e. only up to the headers.

My access handler would get called twice with 0 bytes of uploaded data, after returning MHD_YES it would not get called again and the request would time out.

If I queued a response on the second call with zero bytes the queue_response method would return MHD_NO and the connection would be closed with an Internal Application Error printed in the debug output.

I traced this to the section in MHD_parse_connection_headers where it checks to see if the Connection header is set to close and if it is is sets connection->read_close to MHD_YES. This causes a partially read request with a Connection: close header to stop reading data after the headers have been parsed. Commenting out the connection->read_close line fixed the problem and partially read requests would then get fully read after the second call to my access_handler that returns MHD_YES and my access handler would get called a third time with the posted content in the uploaded_data parameter.

I tested this using a Ruby script that sent 40 POST requests with a 90 byte XML document in the body. Without commenting out that line, one of the requests would invariably time out, after commenting out that line all the requests would complete. I also added some debug logging to ensure that it was a partial read of a request that was failing.

I'm not sure of the wider implications of commenting out that line, but it does fix my immediate problem.
TagsNo tags attached.
Attached Files
test.rb (244 bytes)
connection_close.patch (538 bytes)    
Index: src/daemon/connection.c
===================================================================
--- src/daemon/connection.c	(revision 5780)
+++ src/daemon/connection.c	(working copy)
@@ -699,7 +699,7 @@
               /* other side explicitly requested
                  that we close the connection after
                  this request */
-              connection->read_close = MHD_YES;
+              //connection->read_close = MHD_YES;
             }
 
           if ((0 != (MHD_USE_PEDANTIC_CHECKS & connection->daemon->options))
connection_close.patch (538 bytes)   

Activities

Christian Grothoff

2007-11-28 11:55

manager   ~0003219

Let me first clarify the API:

1) You must not queue a response until you have received all of the POST data. Given that you say that the upload size was zero in both calls, I doubt that you've received all of it. Queueing a response early is why you get back MHD_NO.

2) The "internal error" is what you get if you violate some API constrained, usually returning MHD_NO from the callback -- MHD_NO means that you had serious trouble and that the request should be aborted. If you don't want that, return MHD_YES.

3) Until the POST data upload is complete, you should just return MHD_YES (with possibly processing whatever partial data has been given to you) and only once you've seen it all, queue a response and return MHD_YES. You should be able to find out how much upload data to expect from the HTTP headers. Also (and here is where there maybe a bug), you should only be called with size "zero" once (initially), the second time the data length is zero should indicate the end of the transmission.


Now, what you describe in terms of "Connection: close" handling in this paragraph:

"I traced this to the section in MHD_parse_connection_headers where it checks to see if the Connection header is set to close and if it is is sets connection->read_close to MHD_YES. This causes a partially read request with a Connection: close header to stop reading data after the headers have been parsed. Commenting out the connection->read_close line fixed the problem and partially read requests would then get fully read after the second call to my access_handler that returns MHD_YES and my access handler would get called a third time with the posted content in the uploaded_data parameter."

sounds like it could be a bug in MHD (with respect to properly handling
"Conneciton: close"). Simply commenting this part out (as you did) is not a great idea since it'll cause some other issues. I'll be happy to investigate further -- having your Ruby script would be helpful. Could you attach it to the bug report?

seangeo

2007-11-28 17:01

reporter   ~0003221

Thanks for the clarification of the API. I had firgured that out but it is good to have the explanation.

Just to expand on the behaviour. Most of the time it works as you describe in point (3), however occasionally the access handler is called twice with size "zero", after returning MHD_YES for the second call the handler is never called again and the request times out.

Once I made the change to the "Connection: close" handling, after returning MHD_YES for the second time after getting zero data my handler would get called again but with non-zero data.

I've attached the Ruby script for testing and also the patch for the change I made to connection close handling so you know exactly which area I'm talking about.

Thanks

Christian Grothoff

2007-12-05 22:27

manager   ~0003227

Fixed in SVN 5808 and in the 0.1.2 release.

Issue History

Date Modified Username Field Change
2007-11-27 17:18 seangeo New Issue
2007-11-28 11:55 Christian Grothoff Note Added: 0003219
2007-11-28 11:56 Christian Grothoff Assigned To => Christian Grothoff
2007-11-28 11:56 Christian Grothoff Priority normal => urgent
2007-11-28 11:56 Christian Grothoff Status new => feedback
2007-11-28 17:01 seangeo Note Added: 0003221
2007-11-28 17:03 seangeo File Added: test.rb
2007-11-28 17:04 seangeo File Added: connection_close.patch
2007-11-29 18:13 Christian Grothoff Status feedback => assigned
2007-12-05 22:27 Christian Grothoff Status assigned => resolved
2007-12-05 22:27 Christian Grothoff Fixed in Version => 0.1.2
2007-12-05 22:27 Christian Grothoff Resolution open => fixed
2007-12-05 22:27 Christian Grothoff Note Added: 0003227
2007-12-05 22:27 Christian Grothoff Status resolved => closed
2013-05-06 12:54 Christian Grothoff Category request pipelining => HTTP 1.1 request pipelining