LINK and UNLINK HTTP Methods are not-supported

[jasnell]

Sending LINK and UNLINK requests to a node.js server causes the connection to be terminated due to an error in the way http_parser.c handles HTTP methods. Both LINK and UNLINK were defined as extensions to HTTP 1.0 and are included in the list of registered methods [1].

The lack of support for these two methods appears to be due to a larger issue with the way Node.js parses HTTP methods in general. See [2] for details and test cases.

[1] http://tools.ietf.org/html/draft-ietf-httpbis-method-registrations-12
[2] http://www.chmod777self.com/2013/08/sigh.html

[othiym23]

I'm with Hixie on this one -- I know the HTTP parser is one of the most performance-critical chunks of Node, but this is sort of an embarrassing hack. I too would really like to see this addressed.

[tsloughter]

Might I suggest the optimizations done in NGINX for parsing methods: http://lxr.evanmiller.org/http/source/http/ngx_http_parse.c

[tsloughter]

Well, doesn't solve the LINK/UNLINK issue actually.. but at least the ones it knows about it actually gets right :)

[juliangruber]

I wonder how much this optimization actually buys us

[jezell]

This is being tracked already issue #3192

[isaacs]

@jezell This is a bit different than #3192.

But both should be tracked over at https://github.com/joyent/http-parser. @jasnell Would you mind posting the issue over there?

[chrisdickinson]

It appears to be caused by a missing else { goto error; } case here.

[bnoordhuis]

This is a bit different than #3192.

No, it's the same issue - custom HTTP methods aren't currently supported. LINK and UNLINK are still drafts so they're definitely custom.

[isaacs]

No, it's the same issue - custom HTTP methods aren't currently supported. LINK and UNLINK are still drafts so they're definitely custom.

Well, if we made custom methods allowed, then it would not be an issue. But "add these two methods" is definitely not the same as "let me use whatever methods I want".

[bnoordhuis]

Oh, see my comment on nodejs/http-parser#156 - those methods are drafts, I don't intend to take pull requests for them unless someone can provide a very compelling reason.

[jasnell]

@bnoordhuis Let's ignore LINK and UNLINK for a minute and talk about extension verbs in general. What is the harm in allowing any valid HTTP method to be passed and just setting a default handler that returns a 405 response? Then providing some means of allowing developers to override that default behavior? Doing so would eliminate the entire issue and you'd no longer have to worry about whether or not a particular method is a draft or not. Leave it up to the developers building on top of NodeJS.

[chrisdickinson]

@jasnell I am by no means an expert on this topic, but allowing any valid HTTP method implies the following of http-parser:

1. It should accept <token>, according to the HTTP grammar, which, as outlined by the spec, is not required to be conditionally compliant.
2. By accepting <token> vs. a whitelist of methods, it can no longer rely on the lookup table of HTTP methods.
3. That means that it would have to accept an arbitrary number of characters (after all, it can't know where the space will be a priori).
4. http-parser suddenly has to make allocations to accommodate these extension verbs, which was outside of its original design constraints.
5. In the worst case, by making allocations and accepting arbitrary methods, we open ourselves up to clients attacking with never-ending methods. In any case, we lose an invalidation state that lets us bail early on malformed traffic.

In short, the constraints that you desire from an http parser is that it be "unconditionally compliant", and controllable-by-userland, perhaps at the cost of performance. Those are different -- though valid -- constraints than the ones that http-parser was designed under. That does not imply http-parser is bad or need be replaced, it implies that it does not meet your particular constraints. From my perspective, at least, the constraints under which http-parser was designed are in line with Node's goals itself, and it makes sense to push more configurable http servers towards Node's userland.