Repository navigation
nghttp2: a response's fields are no longer capped at 64, so one with more no longer ends its connection, and one nghttp2 will not send resets its stream - #515
Open
MDA2AV wants to merge 1 commit into
Conversation
…more no longer ends its connection, and one nghttp2 will not send resets its stream The shim packed fields into a fixed array and returned INVALID_ARGUMENT past it, which the managed side reads as a fault in the session: every other stream on the connection ended, and the response's own got nothing. The array is the connection's now and grows as needed. Without the cap, a header block past nghttp2's own 64 KiB limit reaches on_frame_not_send instead, which on main left its stream open for good, so that one is reset with INTERNAL_ERROR. Fixes #505.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
nghttp2_nv nv[64], andih2_unpack_headersreturned INVALID_ARGUMENT past it.SubmitResponseandSendStreamedHeadersread any error as a fault in the session and set_failed.:status, ended the connection and every other stream on it. Its own stream got nothing, neither a response nor a reset. nghttp2 has no such limit.Details are in #505.
The fix
ih2_unpack_headersfills it in one pass, nghttp2 copies out of it during the submit, andih2_freefrees it. A response within the first 64 fields costs nothing new but onemallocper connection.on_frame_not_sendcallback resets a stream whose HEADERS nghttp2 would not send (FRAME_SIZE_ERROR) with INTERNAL_ERROR. Only that error: on a stream already gone, a reset would answer the peer's own (RFC 9113 5.4.2).Native libraries
runtimes/linux-x64andruntimes/linux-arm64are rebuilt withscripts/build-nghttp2-native.shat its pinned nghttp2 ref: x64 on the host, arm64 with theaarch64-linux-gnucross toolchain in an Ubuntu 24.04 container. Both still need nothing past GLIBC 2.17, and neither build's log has a warning.#495 and #501 rebuild the same libraries. Whichever merges later has to merge main and rerun the build script, rather than take either side's binaries. The shim's text merges cleanly with both.
Behaviour
IsCompletedat its flush. Before, the peer waited on that stream for good.API
None.
Tests
New in the E2E suite (
Nghttp2ManyHeadersTests.cs), throughHttpClientover h2c:nghttp2: a buffered response with 63 headers of its own reaches the peer whole, and with 64 and 200nghttp2: a streamed response with 63 headers of its own reaches the peer whole, and with 64 and 200New in the Unit suite (
Nghttp2OversizedResponseTests.cs), over the strict test transport:nghttp2: a buffered response with a header block past nghttp2's limit resets its own stream, and the connection serves onnghttp2: a streamed response with a header block past nghttp2's limit hears the peer gone at its flushAgainst main's library (the tests only):
With the fix:
Bench
Measured with the same setup as #486:
h2load -c 32 -m 16 -t 1pinned to CPU 2.Http2/Nghttp2BufferedHttp2/Nghttp2Response(8 chunks of 1 KiB)Both are inside the noise. A connection gains one
mallocof 2.5 KiB, at its first response.Fixes #505.