`method` is incorrectly parsed when not in compat mode

This line will cause method to be the index of the method inside HTTPParser.methods

Line 237 in 3db72ae

this.info.method = this._compatMode0_11 ? match[1] : methods.indexOf(match[1]);

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.