`method` is incorrectly parsed when not in compat mode
Prinzhorn opened this issue · 5 comments
This line will cause method
to be the index of the method inside HTTPParser.methods
Line 237 in 3db72ae
I surely must be doing something wrong or everybody else is always triggering _compatMode0_11
mode all these years and nobody ever was in non-compat mode before me?
The following code will cause info.method
to be 1
(GET
is the second in the HTTPParser.methods
array, use POST
and method
becomes 3
)
const {HTTPParser} = require('http-parser-js');
const rawHTTP = `GET / HTTP/1.1
Host: www.example.com
`
const parser = new HTTPParser(HTTPParser.REQUEST);
parser[HTTPParser.kOnHeadersComplete] = function (req) {
console.log(req);
console.log(req.method);
};
parser.execute(Buffer.from(rawHTTP));
parser.finish();
Logs
{
headers: [ 'Host', 'www.example.com' ],
upgrade: false,
method: 1,
url: '/',
versionMajor: 1,
versionMinor: 1,
shouldKeepAlive: true
}
1
Or is that actually expected behavior? That thought hadn't occurred to me 🤔 . Given the lack of documentation I thought info
would produce usable data right away.
Duh, I looked at the tests and yes...
Yeah, sorry for the lack of documentation, the module pretty much returns just exactly what Node's internal http.js
stuff needs to function. compatMode0_12
is for Node v0.12+, and since we've now long passed even Node v12.0, yeah, it's been that way for many years (when Node changed their internal parser), that variable should probably call the old one "compat mode" at this point ^_^.
Thanks for the reply. I'm using the library to parse HTTP traffic I have stored somewhere to analyze it, so this is not actually hooked to a network or live traffic. So I personally don't care about the Node.js internal http stuff and I'm not using it to replace the internal parser. This just seems to be the only well maintained HTTP parser on npm. And for this use-case docs would be quite nice, but the code is easy enough to read. Only the compat stuff makes it a little harder to navigate around. I just switched from parser.onHeadersComplete
to kOnHeadersComplete
which triggered the method
change (and made my tests fail) because I'm no longer in compat mode (I think I originally had a PoC for parsing a HTTP buffer from StackOverflow and just now dug into the code and realized what's actually happening. So you can say the SO post was pretty outdated, shocking I know).
I hope you don't mind asking another question here instead of opening a new issues: kOnHeaders
is kOnTrailers
right? Looking at the code this is just misleadingly named. It doesn't appear to be related to headers or kOnHeadersComplete
at all and the order is also messed up, because it would make more sense for it to be between kOnBody
and kOnMessageComplete
because that's when it fires. But it appears that's just what the Node.js internal parser does as well, oh well. I think docs would make sense at least to have a rough documentation of the public API and these events and when they appear (or not)
Yeah, I think what you said all seems correct, including some rough documentation making sense, although the issue there is whenever Node changes their API, someone scrambles to fix this to keep working, and it's now a little unclear what the "API" even is anymore ^_^. If you've got a nice example of how to use this module directly (not as a monkey-patched replacement), adding it as standalone-example.js
or, if concise enough, a section in the README would be welcome =). I, like most contributors on this project, have only used this module as a monkey-patched replacement to prevent Node.js from crashing when it crawls old/misbehaving web servers, but I know the original purpose included being useful as a standalone parser too, and some projects do use it for that.