Fix throwing error on request body without content-length and transfer-encoding

[kylecarbs]

Fixes what I was actually seeing in #100.

[Jimbly]

Since the automated tests apparently aren't working, just to check, does node tests/iojs/test-http-parser-durability.js still pass? Also, can you add a test case to that file (ideally that fails on the old version and now passes)? They're pretty straightforward (basically just the raw HTTP data and expected response pairs).

Glancing at the code though, generally this library tries to parse everything it can (as opposed to Node's built-in one which throws errors if anything isn't exactly perfect, which of course causes problems when scraping various misbehaving websites, which is what at least brought me to using this library ^_^) - so can't we just parse the body in that case? Assuming you've got an actual case in the wild of getting a response like that from a server, is it not parseable? With no content-length nor explicit encoding, the body is probably just plain whatever the default normally is...? Not a big deal if it can't be parsed, throwing an error is at least better than getting stuck =).

[Jimbly]

Oh, since the new code is throwing an error, it might not fit the pattern of that test file, so if it's not feasible to parse it, probably don't worry about adding a new test (but, please, make sure the existing tests still pass).

[kylecarbs]

Will check now! Thanks for the thoughtful response - it didn't seem super possible to do this any other way.

[kylecarbs]

Good call on the tests - looks like I needed to add a few more checks.

[kylecarbs]

@Jimbly this actually breaks the tests in a relatively bad way... testMultiple3 isn't built to have an error thrown, which now occurs on:

GET /test HTTP/1.1
User-Agent: curl/7.18.0 (i486-pc-linux-gnu) libcurl/7.18.0 OpenSSL/0.9.8g zlib/1.2.3.3 libidn/1.1
Host: 0.0.0.0=5000
Accept: */*

GET /test HTTP/1.1
User-Agent: curl/7.18.0 (i486-pc-linux-gnu) libcurl/7.18.0 OpenSSL/0.9.8g zlib/1.2.3.3 libidn/1.1
Host: 0.0.0.0=5000
Accept: */*

GET /test HTTP/1.1
User-Agent: curl/7.18.0 (i486-pc-linux-gnu) libcurl/7.18.0 OpenSSL/0.9.8g zlib/1.2.3.3 libidn/1.1
Host: 0.0.0.0=5000
Accept: */*

Because there's another request, which is out of standard.

If you have ideas, please let me know!

[kylecarbs]

I added a few more conditions which would make it more durable, but without refactoring the test suite, I'm not sure how we could get this merged.

[Jimbly]

What exactly does the the request you're trying to handle/parse that's causing trouble look like?

[kylecarbs]

    c.write('GET /blah HTTP/1.1\r\n' +
            'Host: example.org:443\r\n' +
            'Cookie:\r\n' +
            'Origin: http://example.org\r\n' +
            '\r\n\r\nhello world'
    );

In NodeJS, this is supposed to return a 400 bad request.

[Jimbly]

That looks like it should be one good request (everything up to the hello world), followed by a bad request - the first part of that parses fine, looks like a request without a body, and then a request starting with hello world normally throws an error (although, maybe not until after a CRLF is received, as it just looks like a potentially unfinished write until then?).