"post identity body world" fails with http-parser >= 2.9.3

[cbiedl]

Little surprised nobody has reported this earlier ...

Re-building http_parser.rb with the latest version of http-parser (2.9.4) I noticed the "post identity body world" check fails:

  1) HTTP::Parser should parse request: post identity body world
     Failure/Error: @parser << test['raw']

     HTTP::Parser::Error:
       Could not parse data entirely (116 != 122)
     # ./spec/parser_spec.rb:317:in `<<'
     # ./spec/parser_spec.rb:317:in `block (4 levels) in <top (required)>'

After a lengthy research I think the test is indeed flawed, i.e. in violation of RFC 7320 3.3.1. ("Transfer-Encoding").

The check sets Transfer-Encoding: identity and also Content-Length: 5

About the first, the RFC states:

   If any transfer coding
   other than chunked is applied to a request payload body, the sender
   MUST apply chunked as the final transfer coding to ensure that the
   message is properly framed.

...so this is not acceptable.

According to 3.3.3. ("Message Body Length"), combining Transfer-Encoding: and Content-Length: indicate "request smuggling" which "ought to be handled as an error" - which is what the http-parser library now does: It implemented a stricter check in commit nodejs/http-parser@7d5c99d.

Reproducer (could possibly be shorter):

require "http/parser"

parser = Http::Parser.new

parser.on_headers_complete = proc do
  p parser.http_version

  p parser.http_method # for requests
  p parser.request_url

  p parser.status_code # for responses

  p parser.headers
end

parser <<"POST /post_identity_body_world?q=search#hey HTTP/1.1\r\nAccept: */*\r\nContent-Length: 5\r\nTransfer-Encoding: identity\r\n\r\nWorld"

Solution: Please rewrite or disable that test.

[ashie]

Solution: Please rewrite or disable that test.

I confirmed it too.
I think the current implementation refers old RFC rfc2616 because

• 0.6.0 is released at December 11, 2013
• rfc7230 is published at 2014.

[ashie]

So we should increment feature version (0.x.0) of this gem when we upgrade to http-parser 2.9.3 or later.

BTW because http-parser isn't maintained anymore, I'll also not actively maintain this after resolving major remaining issues.

From https://github.com/nodejs/http-parser

http-parser is not actively maintained. New projects and projects looking to migrate should consider llhttp.