TE
TechEcho
Home24h TopNewestBestAskShowJobs
GitHubTwitter
Home

TechEcho

A tech news platform built with Next.js, providing global tech news and discussions.

GitHubTwitter

Home

HomeNewestBestAskShowJobs

Resources

HackerNews APIOriginal HackerNewsNext.js

© 2025 TechEcho. All rights reserved.

Node.js incorrectly parses HTTP methods

165 pointsby willlllalmost 12 years ago

18 comments

chrismorganover 11 years ago
I&#x27;ve been working on implementing a solid HTTP library in Rust, currently at <a href="http://github.com/chris-morgan/rust-http" rel="nofollow">http:&#x2F;&#x2F;github.com&#x2F;chris-morgan&#x2F;rust-http</a>. Servo has been using Joyent&#x27;s HTTP parser and this is a problem that I had observed with it. Yes, it is pretty badly implemented, but it&#x27;s not a mess because of the style of code—that&#x27;s for performance. It&#x27;s only a mess because it&#x27;s inconsistent and incorrect.<p>Reading the HTTP method in a high-performance way does lead to superficially ugly code. That&#x27;s why code generation is good. Ragel has been mentioned and I intend to seriously consider using it, but for the moment my own HTTP method reading code is generated with:<p><pre><code> generate_branchified_method( writer, branchify!(case sensitive, &quot;CONNECT&quot; =&gt; Connect, &quot;DELETE&quot; =&gt; Delete, &quot;GET&quot; =&gt; Get, &quot;HEAD&quot; =&gt; Head, &quot;OPTIONS&quot; =&gt; Options, &quot;PATCH&quot; =&gt; Patch, &quot;POST&quot; =&gt; Post, &quot;PUT&quot; =&gt; Put, &quot;TRACE&quot; =&gt; Trace ), 1, &quot;self.stream.read_byte()&quot;, &quot;SP&quot;, &quot;MAX_METHOD_LEN&quot;, &quot;is_token_item(b)&quot;, &quot;ExtensionMethod(%s)&quot;); </code></pre> This is pleasantly easy to read and meaningful.<p>This generates the high performance artwork shown at <a href="http://sprunge.us/HdTH" rel="nofollow">http:&#x2F;&#x2F;sprunge.us&#x2F;HdTH</a>, which supports extension methods correctly. (Rust&#x27;s algebraic data types are marvellous for many things in implementing such a spec.)
评论 #6242413 未加载
kiwidrewover 11 years ago
The most interesting part (to me) is that the server will handle e.g. &quot;PUN&quot; as though it actually said &quot;PUT&quot;. I wonder if this could be used as an attack vector?<p>Sounds a lot like a &quot;confused deputy&quot; situation: imagine that your L7 firewall has a rule to reject any PUT request, but it sees PUN and thus allows the request to pass through to node.js, which then treats it as though it were actually PUT.
评论 #6241342 未加载
评论 #6242311 未加载
hosay123over 11 years ago
The level of bikeshedded micro-optimization going on in that file is hilarious. The whole thing could be swapped out with a Ragel parser and nobody would notice a thing
评论 #6241299 未加载
评论 #6242716 未加载
评论 #6241403 未加载
评论 #6243547 未加载
评论 #6242364 未加载
评论 #6254747 未加载
chrismorganover 11 years ago
LINK was added <i>after</i> HTTP&#x2F;1.0, and <i>removed</i> before HTTP&#x2F;1.1. <a href="http://tools.ietf.org/html/draft-ietf-httpbis-method-registrations-12" rel="nofollow">http:&#x2F;&#x2F;tools.ietf.org&#x2F;html&#x2F;draft-ietf-httpbis-method-registr...</a> is being cited, but that is still a draft and the registry that refers to does not yet exist. I believe it is thus fair to say that LINK is not a standard method?
评论 #6242123 未加载
general_failureover 11 years ago
To people saying this is all micro optimization - have you guys actually measured? If so, please post the numbers.<p>It&#x27;s presumptuous to simply say &#x27;all this is unnecessary&#x27; unless you have measured it and we have no reason to believe the author hasn&#x27;t measured it.<p>BTW, the file is copyright nginx.
andreineculauover 11 years ago
A few points to make:<p>- I value software that keeps to the spec, because it&#x27;s the spec that I (as a dev or non-dev) refer to. You never hear &quot;NodeJS has HTTPish module&quot;, nor do you read documentation of that module&#x27;s concepts and behaviour. Those are defined in the spec, and the __fill_in_with_any_language__ HTTP module just implements those definitions.<p>- Optimizations, simplifications, corrections should be done in the spec, whenever the you find them at implementation-time.<p>But until now there has not been <i>ONE</i> HTTP server that grasps and handles the HTTP specs in their whole. So then, I find it hilarious to read that about optimizations when neither of us have the whole picture.<p>That said, I don&#x27;t think it&#x27;s Node.js to blame here (albeit they do have weird views of standards: <a href="https://github.com/joyent/node/issues/4850" rel="nofollow">https:&#x2F;&#x2F;github.com&#x2F;joyent&#x2F;node&#x2F;issues&#x2F;4850</a>) but HTTP itself because the spec&#x27;s abstraction levels have been far away from the implementations&#x27; reach. HTTPs concepts are gorgeous but they are worth nil if implementation is &quot;hard&quot; and never done properly.<p>Longer story at: <a href="http://andreineculau.github.io/hyperrest/2013-06-10-http-hell-no/" rel="nofollow">http:&#x2F;&#x2F;andreineculau.github.io&#x2F;hyperrest&#x2F;2013-06-10-http-hel...</a>
spullaraover 11 years ago
There are plenty of good ways to optimize this code. They didn&#x27;t pick any of them. What I find more surprising than anything is that they didn&#x27;t just optimize GET and handle the rest generically.
sandfoxalmost 12 years ago
It should probably be noted that: a) you don&#x27;t have use node&#x27;s built in HTTP server (yeah, I know, nearly everyone will), you are more thn free to write your own or use one from it&#x27;s module repository (npm) b) The entire HTTP module is currently being over-hauled in the 0.11.X development branch and the changes should appear in the stable 0.12.x releases.<p>Out of interest has anyone seen what other web servers support for these more &#x27;esoteric&#x27; verbs is like?
评论 #6241149 未加载
评论 #6241479 未加载
评论 #6240864 未加载
评论 #6242597 未加载
评论 #6242385 未加载
nkuttleralmost 12 years ago
Nice catch, reading the nodejs code is really discouraging. But the post points to the relevant specs, so I guess this should be fixed rather sooner than later. Initial response looks good: <a href="https://github.com/joyent/node/issues/6078" rel="nofollow">https:&#x2F;&#x2F;github.com&#x2F;joyent&#x2F;node&#x2F;issues&#x2F;6078</a>
lnanek2over 11 years ago
Not a big deal. Even the REST people have long been just sending normal GET and POST and including some indicator that they really want it treated as some other verb.
评论 #6243303 未加载
tszmingover 11 years ago
No one remembered the conversations between Ryan Dahl and Zed Shaw on the http parser two years ago?<p>[1] <a href="https://news.ycombinator.com/item?id=2549403" rel="nofollow">https:&#x2F;&#x2F;news.ycombinator.com&#x2F;item?id=2549403</a><p>[2] <a href="https://twitter.com/zedshaw/status/15714602817" rel="nofollow">https:&#x2F;&#x2F;twitter.com&#x2F;zedshaw&#x2F;status&#x2F;15714602817</a>
评论 #6244696 未加载
bslatkinover 11 years ago
Yet another reason to remove verbs from the HTTP spec: <a href="http://www.onebigfluke.com/2013/08/lets-remove-verbs-from-http-20.html" rel="nofollow">http:&#x2F;&#x2F;www.onebigfluke.com&#x2F;2013&#x2F;08&#x2F;lets-remove-verbs-from-ht...</a>
评论 #6242301 未加载
lambyover 11 years ago
Hm, didn&#x27;t nginx do something similarly &quot;ugly&quot; for a while? Like inspecting the (completely making it up here) second character to see if it&#x27;s &#x27;E&#x27;?<p>A quick look now suggests they are using a parser generator now.
评论 #6242829 未加载
smackfuover 11 years ago
I&#x27;ve seen this kind of thing before. Someone falls in love with their optimization (&quot;we can parse GET with a single int switch!&quot;) and when it is pointed out that it doesn&#x27;t handle the spec correctly, and that handling the spec correctly would make this as slow as the naive way, it still ends up in the code because &quot;it&#x27;s good enough.&quot;
verroqover 11 years ago
Why not use a trie?
评论 #6241370 未加载
mulokaover 11 years ago
I imagine this could this be used to kill a running instance of node.js in production?
评论 #6241026 未加载
评论 #6241021 未加载
Xorlevover 11 years ago
At least they&#x27;re looking at fixing it.
评论 #6242181 未加载
escaped_hnover 11 years ago
Clearly a case of premature optimization. What is the overhead of testing equality on one char vs the entire string?
评论 #6242048 未加载