waxmiguelito / mongoose

Automatically exported from code.google.com/p/mongoose
MIT License
0 stars 0 forks source link

Content-Length is missing in reply header #319

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Set "keep-alive" to "yes"
2. Load any SSI page. A simple test page with "ls"/"dir" will do.

What is the expected output? What do you see instead?
The browser does not know when the page is complete.

What version of the product are you using? On what operating system?
Both V3 (Windows binary) and current source; 
Both WinXP and Linux; 
Both X86 and ARM

Please provide any additional information below.
For HTTP/1.1 it is not allowed that the body of a message simply ends (see 
http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html). This does not seem to 
cause a problem within the client in case the connection closes 
(keep-alive=no). However, in case the connection remains opened the client 
cannot determine the end of the body data. 
Thus, HTTP/1.1 requires either a "Content-Length" header field in the reply 
(see http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4). 
Alternatively, chunk transfer encoding may be used (see 
http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1, 
http://en.wikipedia.org/wiki/Chunked_transfer_encoding). 

Original issue reported on code.google.com by bel2...@gmail.com on 27 Feb 2012 at 12:51

GoogleCodeExporter commented 9 years ago
One possible solution seems to be that, even if keep-alive is configured and 
working fine for static pages, it could be turned off for SSI pages. According 
to the HTTP specification either the server or the client may close the 
connection after setting "Connection: close" in the header, so it does not 
constitute a protocol violation.
So one could closes the connection after all SSI transfers and all CGI calls 
where the CGI process sets the "Connection: close" header field itself. 

In a test, I extended the struct mg_connection by a field "must_close" which is 
set in handle_ssi_file_request and handle_cgi_request (if get_header(&ri, 
"Connection") != 0 and != "keep-alive") and evaluated in should_keep_alive and 
the loop in process_new_connection. With these changes, it works fine.

I could provide this patch as a source. However, it seems I cannot get hg 
running ("abort: error: A connection attempt failed ... timeout ..." - maybe 
blocked?), so I can only provide a possible patch by mail or as an attachment.

Original comment by bel2...@gmail.com on 28 Feb 2012 at 10:32

GoogleCodeExporter commented 9 years ago
Yes please, patch would be fine, thanks!

Original comment by valenok on 28 Feb 2012 at 2:45

GoogleCodeExporter commented 9 years ago
Finally I succeeded in making an hg clone from another computer:
bel2125-mongoose

There I made a couple of patches, among them a patch for 319 - all changed
lines are marked with a comment why I made it. If I found an issue number I
wrote it there.

However, I made my changes to a version I extracted it from the HTTP pages
and replaced the version in the clone with my tested one (instead of
merging it), so it seems like I�ve lost one of your patches this way. I
will have a look at this and test the merged version on the three platforms
(WinXP/X86, Linux/X86, and Linux/ARM) later today or tomorrow.

What would you think about integrating the other patches to the main
stream? If you want I can give you a more detailed description for every
line I changed and describe a test case that behaves better now.

Original comment by bel2...@gmail.com on 29 Feb 2012 at 11:23

GoogleCodeExporter commented 9 years ago
The patch is available in branch bel2125-mongoose, all differences from the 
current main-line version are due to this and some other small patches.

Original comment by bel2...@gmail.com on 1 Mar 2012 at 2:39

GoogleCodeExporter commented 9 years ago
Reason this is not the best solution:

Look at the performance loss of closing the connection:
http://serverfault.com/questions/43692/how-much-of-a-performance-hit-for-https-v
s-http-for-apache

For 10000 requests:

HTTPS connection close: 107.673 seconds
HTTPS keep alive: 2.711 seconds
HUGE PERFORMANCE LOSS!

HTTP connection close: 2.664 seconds
HTTP keep alive: 1.200 seconds

Should we be using chunk transfer encoding instead?

Original comment by mcheng...@gmail.com on 10 Mar 2012 at 4:24

GoogleCodeExporter commented 9 years ago
Of course keep-alive and chunk-transfer is a better solution.

However, in the main stream version of mongoose, keep-alive did not work at 
all. The browsers got confused because they where unable to find the end of the 
page. The easiest fix is to close a connection - this is allowed by the http 
protocol specification at any time.

There are still 3 bugs in the version of March 6:
1) in handle_propfile the reply header contains "Connection: close" but the 
connection is not closed
2) should_keep_alive may return true, although must_close is true
3) there is a bug in the evaluation of the cgi generated response header which 
may keep the connection open although the reply header contains "Connection: 
close"

I think one should first reach a state where all issues are fixed and there are 
no protocol violations. With these fixes only SSI, directory listings and 
PROPFIND requests will close the connections. All static pages which should be 
the majority leave the connection open. Furthermore, cgi pages already have all 
options (send content-lengh, use chunk transfer encoding or close the 
connection). So a large majority of the pages should already not require 
closing the connection after this first fix.

Yes, chunk transfer encoding is more efficient. I thought about implementing 
chunk transfer encoding but it will only make a difference for directory 
listings, PROPFIND and SSI. I don’t use WebDav/PROPFIND so I did not test it. 
Directory listings are usually a single page anyway, so I think there is no 
real benefit in keep-alive here. If a site contains a lot SSI pages then, of 
course, there is a performance gain in using chunk transfer encoding. 

In fact I’m already testing chunk transfer encoding for cgi generated pages. 
Maybe I can provide a patch for SSI and directory listings soon.

Original comment by bel2...@gmail.com on 11 Mar 2012 at 4:40

GoogleCodeExporter commented 9 years ago
As bel has mentioned, chunked encoding would help for dir listing, PROPFIND and 
SSI.
Is it worth the effort? I think in real life I'd expect all requests against 
static content, CGI and user callbacks.
I am not sure that if one wants maximum performance, an SSI , etc is the way to 
go.
And even more then that, Mongoose might not be a right choice anyways.

Original comment by valenok on 11 Mar 2012 at 10:10

GoogleCodeExporter commented 9 years ago

Now 1) from above is fixed in the mainline, thank you.
For 2) and 3) still the conditions are not completely right, so my integration 
tests for keep-alive do not work with the mainline version.

When looking at directory listings, there are also sizes for SSI and CGI pages 
in the table. However, these sizes are the ones from the scripts, not the 
amount data of data the client will receive by opening them - which cannot by 
determined by stat. Anyway now I'm using directory listings and SSI only for 
test and service pages, e.g. sites like <html><pre><!--#exec "top -n1 -b" 
--></pre></html> and similar stuff for the admin area only. The users GUI 
contains static content and CGI only. So for web content like mine, chunk 
transfer encoding does not provide any performance advantage anymore - provided 
teh conditions 2) and 3) are fixed ;-)

By the way, performance was not my only reason to switch to keep-alive=yes, but 
also stability on embedded systems and certain features that can be realized in 
cgi.

Original comment by bel2...@gmail.com on 14 Mar 2012 at 12:44

GoogleCodeExporter commented 9 years ago
Does anyone think that the mongoose is not stable?

I posted some data to the http server, but the server fails to get the post 
data sometimes.

Original comment by chenche...@gmail.com on 3 Jun 2012 at 6:08

GoogleCodeExporter commented 9 years ago
This issue is about the keep-alive option, that is not fully implemented in the 
mongoose mainline yet, and therefore does not work at all in some usecases.

This can be fixed by changing the function should_keep_alive.

There is some discussion on missing post data in another issue 
(http://code.google.com/p/mongoose/issues/detail?id=349) and the users group 
(http://groups.google.com/group/mongoose-users/).

Original comment by bel2...@gmail.com on 4 Jun 2012 at 12:17

Attachments:

GoogleCodeExporter commented 9 years ago
See attached patch file; easier on the eyes here: 
https://github.com/GerHobbelt/mongoose/commit/97a0b624c3c4337f2ae3d2b4f2ca6a8d27
e201dc

fix should_keep_alive logic: when mongoose produces 5xx HTTP response codes you 
cannot trust the server internal state any longer so the related persistent 
connection should be closed to protect against further undefined behaviour; 
when mongoose processing goes wrong (which can happen with a faulty user coded 
event callback as well), another status code is -1 as that's the value it's 
initialized to on every request: in either case, some is rotten in the state of 
Denmark and we should quit ASAP to return to a known and valid server state.

Original comment by ger.hobbelt on 5 Jun 2012 at 10:55

Attachments:

GoogleCodeExporter commented 9 years ago
Mongoose sends Connection: close for SSI requests now.

Original comment by valenok on 22 Sep 2012 at 2:55