Skip to content

nghttp3: a connection that did not negotiate h3 is refused instead of served - #381

Open
MDA2AV wants to merge 1 commit into
mainfrom
fix/352-nghttp3-alpn
Open

MDA2AV wants to merge 1 commit into
mainfrom
fix/352-nghttp3-alpn

Conversation

@MDA2AV

@MDA2AV MDA2AV commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Fixes #352.

What was wrong. A QuicEngine with no ALPN allow list confirms whatever the client offers, including nothing. Http3Connection backstops that: once its uni streams can open, it refuses anything but h3 with H3_GENERAL_PROTOCOL_ERROR. Nghttp3Connection never read NegotiatedProtocol, so a connection that negotiated echo was set up and answered 200, on all four run loops.

What changed. The check now sits in TrySetup, the first-wake setup that all four run loops share (buffered, buffered async, streaming and streamed response). It runs once the uni streams are open, the same point at which the pure-C# stack checks. Anything but h3 is refused before nghttp3 is stood up, the same way the pure-C# stack refuses it:

  • the same log line ([ioxide.nghttp3] protocol error: negotiated 'echo', not h3);
  • the same close code, H3_GENERAL_PROTOCOL_ERROR (0x101).

The code goes through the existing _peerCode, and the caller's FailInternal keeps it, because the first failure wins. No state is added.

One deliberate difference from Http3Connection: the check is skipped once the connection is closed. A connection evicted before its handshake finished (by the idle sweep or a reactor stop) still wakes its handler. That wake arrives closed, with NegotiatedProtocol still null and the uni streams still openable, because QuicRemoveConnection wakes the handler inline before the engine is destroyed. Refusing it would log a protocol error and call Close on every abandoned handshake, where the transport means an eviction to be silent. The closed wake keeps today's behavior.

Proof. Tests:

  • E2E H3AlpnTests: h3/nghttp3: a connection that did not negotiate h3 is not served, promoted from Pending.
  • E2E H3AlpnTests, new control: h3/nghttp3: control - a permissive engine serves a connection that negotiated h3. It uses the same engine and handler with h3 offered, and gets a 200.
  • E2E H3ErrorCodeTests, new and at the code seam next to its pure-C# twin: h3/codes: nghttp3 - a non-h3 ALPN closes with H3_GENERAL_PROTOCOL_ERROR on every run loop, unless already closed.
    • Each of the four run loops gets an echo connection with a GET queued. It must serve nothing and close with 0x101.
    • A closed connection with no protocol must not be closed at all.
    • The GET carries a non-empty :authority. nghttp3 refuses WellFormedGet's empty one as H3_MESSAGE_ERROR, which would make "not served" pass for the wrong reason.

Run both ways:

  • Without the fix (tests kept, source reverted):
    • The promoted test fails with an h3 handler must not serve a non-h3 connection, got 200 'served-by-nghttp3'.
    • The code-seam test fails with buffered: served 1 request(s) on a connection that negotiated 'echo'. A throwaway probe on the reverted source showed all four loops serve the GET (served=1, no close).
    • The control passes.
  • With the fix minus the closed-connection skip, the code-seam test fails with an evicted connection was closed with 0x101.
  • With the fix, all three pass.

Suites (main 71aab91 vs. this branch):

  • E2E: 292 passed, 5 pending, vs. 295 passed, 0 failed, 0 skipped, 4 pending.
  • Http: 88 passed on both.
  • ioxide.nghttp3 builds for net10.0 and net11.0 with no warnings.

Overlap. #382 (issue #351) also changes H3ErrorCodeTests.cs.

All eight together. This batch's PRs (#376 to #383) were merged onto main 71aab91 in one scratch tree. The only conflict is the binary libioxide_ngtcp2.so between #376 and #377, rebuilt there from the merged shim with the script's defaults. Everything builds, and Unit 102, E2E 309, Http 88, Chaos 80, Tls 169 (14 kTLS cases skipped) and File 5 pass, with 0 failed and 0 pending. The repo's only pending test left is the Redis suite's (#336).

Noticed, not changed.

  • Http3Connection's backstop has no closed-connection skip. I swapped QuicSniHostileTests' handler for Http3Connection in a throwaway run: every half-open connection's eviction logs [ioxide.http3] protocol error: negotiated '(none)', not h3 and calls Close(0x101). The same one-condition skip (&& !IsClosed) would quiet it. That code is in ioxide.http3, so it is not in this PR.
  • TrySetup's comment "Runs on the first post-handshake wake" is not true for those evictions.

This branch has not been deployed

No deployments
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.

h3/nghttp3: a connection that did not negotiate h3 is served

1 participant