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

HTTP/2 headers may not be split in CONTINUATION frames #12488

Closed
sbordet opened this issue Nov 6, 2024 · 2 comments · Fixed by #12489
Closed

HTTP/2 headers may not be split in CONTINUATION frames #12488

sbordet opened this issue Nov 6, 2024 · 2 comments · Fixed by #12489
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented Nov 6, 2024

Jetty version(s)
12.0.x

Description
HTTP/2 headers generation invokes the HpackEncoder with a ByteBuffer that is sized to the maxFrameSize.
This is incorrect for those cases where maxHeaderListSize > maxFrameSize.

@sbordet sbordet added the Bug For general bugs on Jetty side label Nov 6, 2024
@sbordet sbordet self-assigned this Nov 6, 2024
@sbordet sbordet moved this to 🏗 In progress in Jetty 12.0.16 FROZEN Nov 6, 2024
sbordet added a commit that referenced this issue Nov 6, 2024
* Removed unnecessary and wrong initialization of Parser and Generator properties, as they are initialized by SETTINGS frames (or they have default values already).
* Fixed headers generation, taking into account maxHeaderListSize and maxFrameSize correctly.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@wendigo
Copy link

wendigo commented Nov 7, 2024

How does it affect the server when this condition happens?

@sbordet
Copy link
Contributor Author

sbordet commented Nov 7, 2024

@wendigo the server would throw an error and close the connection.

However, this event is quite rare, because it really needs very large headers, typically way more than the 16 KiB max frame size thanks to Huffman compression (say 20 KiB or more).

Note that common response headers are about 300-500 bytes, so to trigger this problem the headers would need to be 40x bigger, which is very rare.
We have not seen this error ever reported.

sbordet added a commit that referenced this issue Nov 9, 2024
* Introduced HTTP2Client.maxRequestHeadersSize.
* Initialized HpackEncoder.maxHeaderListSize.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.16 FROZEN Nov 12, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.16 FROZEN Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants