Skip to content

🐛 Ignore 1xx interim responses in http2_adapter - #2601

Open
chiliec wants to merge 1 commit into
cfug:mainfrom
chiliec:fix/2600
Open

🐛 Ignore 1xx interim responses in http2_adapter#2601
chiliec wants to merge 1 commit into
cfug:mainfrom
chiliec:fix/2600

Conversation

@chiliec

@chiliec chiliec commented Sep 4, 2026

Copy link
Copy Markdown

New Pull Request Checklist

  • I have read the Documentation
  • I have read the Agent Contribution Guidelines (required if any part of the change was produced with AI assistance)
  • I have searched for a similar pull request in the project and found none
  • I have updated this branch with the latest main branch to avoid conflicts (via merge from master or rebase)
  • I have added the required tests to prove the fix/feature I'm adding
  • I have updated the documentation (if necessary)
  • I have run the tests without failures
  • I have updated the CHANGELOG.md in the corresponding package

Additional context and info (if any)

Closes #2600.

Problem. When an HTTP/2 server sends a 103 Early Hints (or any 1xx)
HEADERS frame before the final response, Http2Adapter._fetch treated the
interim frame as final: it parsed the 103 :status, completed the response
Completer, and then completed the same Completer again when the real
200 HEADERS frame arrived on the same stream, crashing the isolate with
Bad state: Future already completed. As a secondary effect, responseHeaders
was never reset between HEADERS frames, so interim headers (e.g. link) leaked
into the final response.

Fix. In the HEADERS-frame handler, 1xx responses are now skipped (the
handler returns without completing), and the accumulated response headers are
reset per final HEADERS frame so interim headers don't carry over. Single-frame
responses (the common case) are unaffected.

Note (sensitive area — header handling, §3). This touches header handling,
so I kept the change minimal and additive: no public API change, no signature
change, default behavior for normal single-HEADERS-frame responses is
identical.

Verification. Added a regression test (plugins/http2_adapter/test/early_hints_test.dart)
that drives a local h2c server emitting 103 then 200 on the same stream and
asserts the request resolves with 200, body hello, and that the interim
link header does not leak. Confirmed genuine RED→GREEN: reverting only the
source change makes the test fail with the exact Bad state: Future already completed crash; with the fix it passes. redirect_test.dart (14 tests, same
HEADERS-frame path) and headers_test.dart still pass; dart analyze and
dart format are clean on both changed files.

AI disclosure (§8.3): implementation and tests were produced with Claude; a human owns and has reviewed the change.

A `103 Early Hints` (or any 1xx) HEADERS frame was treated as the final
response: the adapter parsed its `:status`, completed the response
`Completer`, and then completed the same `Completer` again when the real
final HEADERS frame arrived, crashing the isolate with
`Bad state: Future already completed`. Interim headers also leaked into
the final response because `responseHeaders` was never reset between
frames.

Skip 1xx responses instead of completing on them, and reset the
accumulated headers per HEADERS frame so interim headers do not carry
over into the final response.

Closes cfug#2600

Co-Authored-By: Claude <noreply@anthropic.com>
@chiliec
chiliec requested a review from a team as a code owner September 4, 2026 11:17
@AlexV525

AlexV525 commented Sep 4, 2026

Copy link
Copy Markdown
Member

Review notes — both non-blocking, verified locally (the regression test reproduces the exact Bad state: Future already completed crash against current main and passes with the fix; dart test, dart analyze, dart format all clean):

1. Trailers are now deterministically dropped — please disclose it and add a hardening test

A trailer section arrives as a HEADERS frame without :status and with END_STREAM (RFC 9113 §8.4, §8.8.5). The new code collects frameHeaders but discards them whenever status == null. Before this PR, trailers were merged into responseHeaders — but only racily (observable only when they landed before the response object was built, since Headers.map exposes the live internal map), so a deterministic drop is defensible. Still, it silently discards server data (e.g. gRPC trailing metadata — the only channel for grpc-status, required by the gRPC/HTTP2 spec). Please:

  • add one sentence to the CHANGELOG entry / PR description noting that trailers are now discarded;
  • add a test where the server sends a trailer HEADERS frame after DATA and asserts the request still completes normally (pins the behavior, guards regressions).

Proper trailer support is an API addition — opened as #2602.

2. Malformed 1xx + END_STREAM should fail fast; and badResponse would be the wrong type

HeadersStreamMessage exposes endStream (package:http2, transport.dart). A 1xx HEADERS frame with END_STREAM terminates the stream without a final response — malformed per RFC 9110 §15.2 ("1xx … prior to a final response"; "terminated by the end of the header section") together with RFC 9113 §8.4 (interim responses carry no trailers and precede a final response). Go's x/net/http2 errors out on this ("1xx informational response with END_STREAM flag"). With receiveTimeout unset (the default), the current code waits forever while the stream holds a pooled connection. Suggested shape:

if (code >= 100 && code < 200) {
  if (message.endStream && !responseCompleter.isCompleted) {
    responseCompleter.completeError(
      DioException.connectionError(
        requestOptions: options,
        reason: 'Received an interim 1xx response with END_STREAM; '
            'the stream ended without a final response.',
      ),
    );
  }
  return;
}

connectionError matches the existing precedent in this same method ("Received redirect without location header." already uses DioException.connectionError); badResponse would mean "a final response failed validateStatus" — which is exactly the pre-fix symptom this PR removes.

Both points are the same concern (HEADERS-frame handling robustness), so folding them into this PR seems reasonable if you agree.

Comment drafted with AI assistance (GLM), reviewed and submitted by a human.

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.

http2_adapter: 1xx informational responses (e.g. 103 Early Hints) crash the request

2 participants