-
-
Notifications
You must be signed in to change notification settings - Fork 294
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
Support very large headers #555
Conversation
984bcae
to
7a87f24
Compare
Where does it check that total header size doesn't exceed SETTINGS_MAX_HEADER_LIST_SIZE? |
Pretty sure it doesn't check that when encoding, note that it didn't beforehand either. |
7a87f24
to
90ba520
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks nice and clean to me! Perhaps a test in h2-tests/codec_write
about sending a header that would have been too big before?
There's a risk with enforcing SETTINGS_MAX_HEADER_LIST_SIZE when sending. The input may come from some other component or system that doesn't have such limits, and/or may not be able to change they header list to fit that size. An H2 library that enforces the rule can cause problems, so if it's ever implemented it probably needs to be optional. |
This completely refactors how headers are hpack-encoded. Instead of trying to be clever, constructing frames on the go while hpack-encoding, we just make a blob of all the hpack-encoded headers first, and then we split that blob in as many frames as necessary.
90ba520
to
dc2f331
Compare
I added a 32KiB header in the existing test. |
This completely refactors how headers are hpack-encoded.
Instead of trying to be clever, constructing frames on the go
while hpack-encoding, we just make a blob of all the
hpack-encoded headers first, and then we split that blob
in as many frames as necessary.