Make HTTP_MAX_HEADER_SIZE configurable

[elmarx]

Is your feature request related to a problem? Please describe.

The recent limitation to HTTP_MAX_HEADER_SIZE (1860352) to mitigate CVE-2018-12121 is a problem for us.

We use headers internally to communicate the users' session, and sometimes (legitimate) requests from "outside" exceeed the 8 kb limit, too.
Given that JWT-strings easily exceed 1kb, I think the 8kb limit might be too little for others, too.
Or referrer-headers (especially in combination with payment-systems back-and-forth) tend to exceed 1kb, too.

Describe the solution you'd like
Have the possibility to configure the HTTP_MAX_HEADER_SIZE — at least via configuration-flag (at node-compile-time).
Setting this at run-time or at startup time would be nice, too.

Is setting this at compile time already possible? I couldn't find the option or best way to do it for node-gyp/gyp.

Describe alternatives you've considered

patching nodejs at compile time… not a good idea.

Reduce headers, yeah, would be nice, but that means completely changing parts of our architecture.

[mcjfunk]

Major +1 here. Our large enterprise is not in the position to reduce the possible headers that flow through our infrastructure to 8k. Agree that patching node at compile time is not a good solution for us.

[mpaw]

We are running into the same issue. The drop from 80kb to 8kb is forcing us to stick with the previous version of node until this is resolved.

This needs to be configurable either through a command line option, environment variable, or in code. A compile time flag is not a good solution for us (or probably 99% of other devs) either.

[cjihrig]

Sorry if I missed the conversation somewhere, but do we have the option of updating http_parser (or floating a patch on it) to make the max header size configurable at the parser level?

I did a little experimenting (definitely not PR ready at this point) at cjihrig@e55765d, and was able to adjust the max header size. It breaks ABI, but I'm not sure if we worry about that with the http parser.

cc: @bnoordhuis

EDIT: I don't think I would attach the max header size to the parser, but maybe add a function that sets the global max header size. That shouldn't break ABI. (cjihrig@fb615c5)

[foo4u]

It's also a major issue that this went out as a patch level fix to LTS editions. This is a breaking change.

Ideally this 8K hard limit commit should be reverted and new LTS releases cut.

[brycesteinhoff]

This affects us also, as our headers are right around 8k usually, but sometimes more (we have JWT tokens which account for > half of this).

Our base Docker images were locked to a Node major version, so this breaking change appeared out of nowhere when the upstream image updated the Node minor version.

[bnoordhuis]

It breaks ABI, but I'm not sure if we worry about that with the http parser.

http-parser does, as do distros that link node to http-parser dynamically.

A possible way forward: split p->nread into two, p->nread and p->nread_max, possibly with better names. :-)

Drawback: there are some downstream projects that read p->nread, even though it's marked as private.

[cjihrig]

@bnoordhuis what about a function that would allow setting the global max header size? It's not necessarily the most elegant solution ever, but I don't think it would break anything.

[bnoordhuis]

That could work. Two issues with the patch:

• Don't use a k prefix if it's not actually constant.
• Set the limit once, not every time a new parser is instantiated.

[elad-yosifon]

A week of debugging finally brought me here....
could of been easily solved if there was a simple debug log line that indicates that the request was blocked