Skip to content
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

introduce headerSize limit #64

Merged
merged 7 commits into from
Dec 4, 2021

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Dec 4, 2021

According to the comments, the maxHeaderSize should be actually be corresponding to the node http modules maxHeaderSize value. But it is actually wrong, as the default maxHeaderSize is and was always 8 kb and not 80 kb. On the other hand, the header of a request is different than the header of a multipart.

With this PR we expose headerSize as configurable limit. So if somebody wants to reduce the allowed size of a header, it is now possible to set the limit.

This PR is non-breaking, as the original 80 kbytes are still enforced. Maybe we should inform in a security part of the readme.md that we recommend to reduce the header size to something smaller.

Checklist

@Uzlopak Uzlopak requested a review from kibertoad December 4, 2021 13:28
@kibertoad
Copy link
Member

We probably should mention these params in readme

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Dec 4, 2021

@kibertoad
They are in the section of limits. Or do you mean something else?

@kibertoad
Copy link
Member

missed it, sorry

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Dec 4, 2021

@kibertoad
💯 % test coverage of headerparser.js

@kibertoad
Copy link
Member

Woohoo!

@Uzlopak Uzlopak merged commit 158b6f8 into fastify:master Dec 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants