logic error in read_request() which would produce INVALID failure reports when the remote would send enough data to flood the entire buffer in one pull() [fix included]

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Use a client or CGI app which sends enough (all) data all at once.
2. Code review. Reason through read_request() while assuming the pull() will 
fill the buffer entirely in one go. get_request_len() isn't called then when it 
should.

What is the expected output? What do you see instead?

mongoose should correctly locate the 2xCRLF (or 2xLF in case of CGI) which 
terminates the request/response header there, but it doesn't when 
read_request() only needs one pull() to fill the entire caller's buffer.

Related issues: issue 224.

What version of the product are you using? On what operating system?

mongoose HEAD revision, any platform

Please provide any additional information below.

https://github.com/GerHobbelt/mongoose/commit/9b7f9f9879355c2d72ae6d4094f7248b35
9a3832
-->
fix logic error in read_request() which would produce INVALID failure reports 
when the remote would send enough data to flood the entire buffer in one 
pull(): get_request_len() would not be called and hence no HTTP header would be 
detected, while there /would/ be one in the buffer.

Original issue reported on code.google.com by ger.hobbelt on 2 Jul 2012 at 6:46

[GoogleCodeExporter]

Original comment by ger.hobbelt on 2 Jul 2012 at 6:50

Attachments:

• 0001-fix-logic-error-in-read_request-which-would-produce-.patch

[GoogleCodeExporter]

And for the next time, also apply this one to get better error log output, 
covering the eventualities:

Original comment by ger.hobbelt on 2 Jul 2012 at 7:07

Attachments:

• 0002-improved-error-log-message-for-when-read_headers-fai.patch

[GoogleCodeExporter]

Gotcha! :-) : Error was introduced in 
http://code.google.com/p/mongoose/source/diff?spec=svnbe3296a79f03aa19e66b4c5c8b
1f8b16fd2222fe&r=be3296a79f03aa19e66b4c5c8b1f8b16fd2222fe&format=side&path=/mong
oose.c

Original comment by ger.hobbelt on 2 Jul 2012 at 7:10

[GoogleCodeExporter]

I believe the logic error could be fixed by changing  *nread < bufsiz  to 
*nread <= bufsiz
This was done in 
https://github.com/valenok/mongoose/commit/6c54370aa13b65a2c79f08d35fa9264e00dd7
d4d
Can you confirm that that was indeed the fix and current code does not have 
that logic error please?

Original comment by valenok on 17 Aug 2012 at 1:06

[GoogleCodeExporter]

Well, looks like it when considered naively.
(flow analysis); only your way of solving this is walking the narrow edge of 
the abyss as pull() will now get an input buffer length of zero and not 
everybody out there is going to like that: note that UNIX man pages and OpenSSL 
don't state what happens exactly when you feed recv() and friends the 
senseless/illogical buflen==0 value; in fact, recv() man pages specifically 
state that the recv() RETURN VALUE of ZERO is used to signal connection closed 
and your feeding the bugger a buflen==0 would give *return value* zero a whole 
different meaning.

Hence, expect things to break in very odd ways on various platforms; this issue 
will only happen in very particular edge cases (read_request() has to hit that 
buffer edge). It's up to you if you like to debug those probable failures later 
on; one thing's for sure: they'll be bloody hard to reproduce, once they happen.

(provided patch was done considering such edge scenarios; meanwhile the code 
has moved on, so it might be handiest if I file a pull request for this one, 
now that you're on github.)

Original comment by ger.hobbelt on 18 Aug 2012 at 12:47

[GoogleCodeExporter]

pull request filed:

https://github.com/valenok/mongoose/pull/5

Original comment by ger.hobbelt on 18 Aug 2012 at 7:49

[GoogleCodeExporter]

Merged, thank you.

Original comment by valenok on 18 Aug 2012 at 8:07

• Changed state: Fixed