http: whitespace in header field name violation

[jasnell]

Per RFC 7230: "A server MUST reject any received request message that contains whitespace between a header field-name and colon with a response code of 400 (Bad Request)."

We are currently ignoring this. To test, create a simple server:

http.createServer(function(req,res) {
  res.end('ok');
}).listen(8080);

Then send a simple request via telnet, inserting whitespace between the header field name and the colon:

$ telnet localhost 8080
GET / HTTP/1.1
Test    : ing

HTTP/1.1 200 OK
Date: ...

The server will respond with a 200 (OK) rather than the required 400 (Bad Request)

[jasnell]

I believe the fix actually needs to be made in http-parser and not Node, but opening this to track so I don't forget

[Trott]

Refs: nodejs/http-parser#315

[Trott]

This has been sitting here for almost two years, so I'm going to put a help wanted label on it. Maybe we should add mentor available label as well if you're available to mentor on it @jasnell?

[Trott]

@nodejs/http

[jasnell]

It's an extremely non-trivial issue that requires pretty deep knowledge of the http-parser. It also has a major impact on performance. It's not going to be easy by any measure.

[bnoordhuis]

Should this actually be fixed though? Extraneous whitespace is common enough that http-parser users will want a way to turn off rejection, even in strict mode.

It's also harmless, IMO. The parser skips over the whitespace; there is no observable difference to users because they never see it. You can't use it as a vector for smuggling attacks.

Besides to-the-letter compliance with the spec, what is the point of making it an error?

[apapirovski]

Does anyone feel about this strongly one way or another? The more time passes, the less likely we'll be able to change it without causing breakage so it would be good to make a decision one way or another.

[bnoordhuis]

I might have been wrong about this part:

The parser skips over the whitespace; there is no observable difference to users because they never see it.

A header like X-Foo : bar shows up as 'x-foo ' in req.headers. That should be fixed.

[wellingguzman]

Is this ticket taken? I would like to try this one.

[bnoordhuis]

@wellingguzman You're welcome to but it should probably be fixed in nodejs/http-parser, not nodejs/node.