-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Missing API docs for recent changes in HTTP module #24693
Comments
We picked 8KB as its the default of NGINX to protect from the same type of attack. Unfortunately making this configurable is not an easy feat, as it is a build-time configuration, unfortunately http_parser does this check via a macro. We deemed this as a good solution. I would suggest that if we want to make this a runtime configuration, a serious refactoring of http_parser is necessary. I would recommend this to happen in llhttp instead. It is possible to change the max size by changing this define directive: |
@mcollina thanks for the info. Is there a way to set HTTP_MAX_HEADER_SIZE via the usual |
@zauberpony I thought so, but after some investigation, it seem we need to add an option to |
I think you could set the |
OK, for now I simply patched http_parser.gyp, which works (so no need to rush), but of course a flag or env-variable (I did not yet test @richardlau's suggestion) would be cleaner in the long run. |
Document that the limit was changed from 80KB to 8KB in 1860352. Fixes: nodejs#24693
Document that the limit was changed from 80KB to 8KB in 1860352. Fixes: nodejs#24693 PR-URL: nodejs#24700 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Fixed in 063e8fb |
The maximum size of headers introduced in the security release of 2018/11/27 is not configurable. This change adds a --http-max-header-size option to ./configure. See: nodejs#24693
Document that the limit was changed from 80KB to 8KB in 1860352. Fixes: nodejs#24693 PR-URL: nodejs#24700 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Updates to the HTTP module that landed in recent Node.js releases in an attempt to mitigate several CVEs may cause braking changes on users and teams which make use of large headers due to reasons such as bulk APIs that will employ a large string in the query param.
It seems that:
At the very least, I'm proposing to update the HTTP API docs with the above details to convey this information.
I will follow-up with a PR to the docs.
--
Reference: https://www.nearform.com/blog/protecting-node-js-from-uncontrolled-resource-consumption-headers-attacks
Related issues: #24692
The text was updated successfully, but these errors were encountered: