Support http2 upstream phase2

[mwfj]

HTTP/2 upstream Phase-2: 9 correctness gates + 30 new tests

Summary

Closes 9 correctness gaps in the HTTP/2 upstream path that were deferred from PR #27 (Phase-1 MVP). Touches the codec / per-stream / sink layer only — no checkout-flow refactor (that's PR-3 / wait-queue work).

• CONNECT method rejected on H2 with a self-identifying 502 + X-H2-Limitation: connect-not-supported header. Primary gate in ProxyTransaction::DispatchH2, secondary defense-in-depth gate in UpstreamH2Connection::SubmitRequest.
• Truncation detection at clean close — Content-Length short-read and NO_BODY (204/304/HEAD) violations now surface as RESULT_TRUNCATED_RESPONSE (terminal — no retry; held-fallback machinery is deferred to PR-3).
• te: trailers capture-and-re-emit for gRPC clients — header rewriter strips te per RFC 7230 hop-by-hop rules; the H2 nv-array re-emits the synthetic token from a per-transaction flag set by an inline locale-safe ASCII tokenizer.
• Send-stall + response-timeout handoff — adds OnRequestSubmitted() to the sink interface (default no-op virtual), wired through nghttp2's on_frame_send_callback filtered on HEADERS/DATA + END_STREAM. Per-stream send-stall closure is killed by generation bump when END_STREAM clears the wire; response-timeout arms via an arm-once flag coordinated between OnHeaders and OnRequestSubmitted (whichever fires first wins).
• 30 new tests (TestN1–N19 + TestB15–B19) bringing the suite from 1227 → 1257.

Why

Phase-1 (PR #27) shipped the H2 codec and connection table but left correctness gaps that operators would only notice as silent misbehavior:

• Sending CONNECT to an H2 upstream emitted a malformed request (H2 codec always emits :scheme/:path, which RFC 9113 §8.5 forbids on CONNECT pseudo-headers).
• Backends that lied about Content-Length or sent body bytes on a NO_BODY response had their corrupted bytes streamed to the client.
• gRPC clients sending te: trailers had it stripped without re-emit, breaking trailer negotiation.
• The H2 path had no equivalent of the H1 send-loop's stall protection — a wedged peer that stopped reading our DATA frames pinned the stream until the PING timeout (or forever, if disabled).
• The H2 path had no per-transaction response timeout — a session-level deadline tears down sibling streams; the H1 transport-level deadline is meaningless once the lease is donated to the multiplexed session.

Scope

.github/workflows/weekly-valgrind.yml     |    1 +
docs/http2_upstream.md                    |    2 +
include/upstream/proxy_transaction.h      |   50 +
include/upstream/upstream_h2_connection.h |    7 +-
include/upstream/upstream_h2_stream.h     |   14 +
include/upstream/upstream_response_sink.h |   17 +
server/proxy_transaction.cc               |  235 ++++-
server/upstream_h2_connection.cc          |  132 ++-
test/h2_upstream_test.h                   | 1579 +++++++++++++++++++++++++++++
9 files changed, 2010 insertions(+), 27 deletions(-)

What ships

New result codes (include/upstream/proxy_transaction.h)

• RESULT_TRUNCATED_RESPONSE = -10 — terminal in this PR (held-fallback retry deferred to PR-3). Maps to 502 BadGateway in MakeErrorResponse.
• RESULT_H2_METHOD_NOT_SUPPORTED = -11 — terminal. Dedicated MakeErrorResponse branch emits X-H2-Limitation: connect-not-supported. Pre-routing hook in OnError calls ReleaseBreakerAdmissionNeutral so the rejection isn't counted as an upstream failure.
• // -12 reserved for PR-3's RESULT_GOAWAY_UNPROCESSED.

SEND_STALL_FALLBACK_MS = 30000 lifted from a function-local in the H1 send loop to a class-level static constexpr so both H1 and H2 paths share the same response_timeout_ms == 0 zero-disable semantic.

CONNECT rejection (criterion 20)

• Primary gate at the top of ProxyTransaction::DispatchH2 — early-return with RESULT_H2_METHOD_NOT_SUPPORTED before any pool / lease / breaker work.
• Secondary gate at the top of UpstreamH2Connection::SubmitRequest — explicit return -1 after warn log + sink->OnError dispatch (defense-in-depth for any future caller bypassing DispatchH2).

Truncation + NO_BODY validation (criteria 47, 55, 78)

• New request_method and body_bytes_received fields on UpstreamH2Stream.
• OnHeadersComplete extends NO_BODY framing to include status 204/304 + HEAD requests.
• OnDataChunkRecvCallback Step 1.5 validates pre-dispatch: rejects body bytes on NO_BODY framing; rejects DATA whose length exceeds expected_length - body_bytes_received. Rejection routes through self->ResetStream(stream_id) so the existing in_receive_data_ guard defers the inline FlushSend.
• OnStreamClose(NO_ERROR) branch surfaces CL short-reads as RESULT_TRUNCATED_RESPONSE instead of dispatching OnComplete.

Defense-in-depth caveat (called out in docs/http2_upstream.md and the internal reference doc): nghttp2's HTTP messaging enforcement (default-on) intercepts CL/NO_BODY violations BEFORE invoking user callbacks, routing them through OnStreamClose's non-NO_ERROR branch as RESULT_UPSTREAM_DISCONNECT. The application-level Step 1.5 and CL-short-read-on-NO_ERROR checks are unreachable on the standard nghttp2_session_client_new path; they are retained as a backstop for future code paths that opt out of enforcement (e.g. via nghttp2_option_set_no_http_messaging for streaming protocols). Operator dashboards see truncation in the disconnect bucket today, not a dedicated truncation bucket. Comments at the two callsites flag this clearly so future contributors know which layer they're working under.

te: trailers capture (criteria 69, 72)

ProxyTransaction constructor body adds an inline tokenizer that runs BEFORE HeaderRewriter::RewriteRequest strips all te values:

• Locale-safe lowercase via explicit ASCII branch (if (c >= 'A' && c <= 'Z') c |= 0x20) — std::tolower would corrupt Turkish locale ('I' → 'ı').
• Comma-separated token split with RFC 7230 OWS trim (SP + HTAB only).
• Sets client_te_trailers_ bool when any token equals "trailers".

The H2 outbound nv-array build re-emits a synthetic te: trailers from this flag AFTER the rewriter strip pass:

push_nv("te", 2, "trailers", 8);  // RFC 9113 §8.2.2 permits this exact value

UpstreamH2Connection::SubmitRequest signature gains bool client_te_trailers = false as the last positional param (defaulted to keep test callers compiling).

Send-stall + response-timeout handoff (criteria 23, 70)

New OnRequestSubmitted() virtual on UpstreamResponseSink — default no-op so non-H2 sinks ignore. Fires from OnFrameSendCallback in upstream_h2_connection.cc (registered via nghttp2_session_callbacks_set_on_frame_send_callback) on HEADERS/DATA frames carrying END_STREAM.

ProxyTransaction::DispatchH2 initializes ALL H2 state BEFORE calling h2->SubmitRequest:

h2_path_ = true;
h2_conn_weak_ = h2;
state_ = State::SENDING_REQUEST;
h2_response_timeout_armed_ = false;
const uint64_t send_stall_gen = ++h2_send_stall_generation_;
dispatcher_->EnQueueDelayed([weak_self, send_stall_gen]() { ... },
                             stall_budget_ms);
// Now SubmitRequest may inline-fire OnFrameSendCallback for bodyless requests.
int32_t stream_id = h2->SubmitRequest(...);

This resolves the synchronous-fire race for bodyless requests where nghttp2 inline-flushes HEADERS+END_STREAM during SubmitRequest's internal FlushSend. If state init were deferred to after submit, OnRequestSubmitted's if (!h2_path_) return; guard would silently drop the kill of the send-stall closure — spurious OnError(UPSTREAM_DISCONNECT) ~30s later.

OnRequestSubmitted itself:

• Bumps h2_send_stall_generation_ (kills the closure queued in DispatchH2).
• Transitions SENDING_REQUEST → AWAITING_RESPONSE if state hasn't already advanced (early peer reply).
• Arms response-timeout via ArmResponseTimeout() if !h2_response_timeout_armed_.

OnHeaders (H2 branch — distinct from H1) does NOT poison the connection on early-final-headers (per-stream isolation) and arms response-timeout via the same arm-once flag.

Cleanup H2 branch enforces strict ordering:

1. Bump h2_send_stall_generation_ (invalidates queued closure).
2. ClearResponseTimeout() — keys on h2_path_ to bump h2_response_timeout_generation_; MUST run with h2_path_ still true.
3. Reset h2_response_timeout_armed_ = false.
4. Reset h2_path_ = false (last).

Submit-failure rollback

If SubmitRequest returns failure AFTER the inline FlushSend has already fired OnRequestSubmitted synchronously (and armed the response-timeout closure), the rollback bumps BOTH generation counters and resets the arm-once flag:

++h2_send_stall_generation_;
++h2_response_timeout_generation_;  // closure may have been queued sync
h2_response_timeout_armed_ = false;
h2_path_ = false;
h2_conn_weak_.reset();

State stays untouched — AttemptCheckout (called by MaybeRetry's deferred-retry timer and immediate-retry branch) resets state_ before the next attempt.

Tests (commits 8 from the implementation sequence)

test/h2_upstream_test.h grows from 63 to 93 tests:

Series	Count	Coverage
TestN1–N19	25	Negative / correctness — CONNECT rejection (primary + secondary), te:trailers re-emit (with/without flag, locale variants, multi-stream isolation), CL exact / overflow / short-read, NO_BODY for HEAD/204/304 (with and without END_STREAM-on-HEADERS), OnRequestSubmitted bodyless / bodied / once-per-stream / not-on-CONNECT, RST mid-body maps to disconnect (NOT truncation), no spurious RST on natural close, ResetStream-after-complete is no-op, sibling-stream isolation across truncation, FailAllStreams cleanup.
TestB15–B19	5	Wire-level — trailers after DATA+END_STREAM, DATA padding stripped, GOAWAY with active stream, RST_STREAM mid-body wire frame, multi-stream RST one completes other.

All tests follow the dtor-defends-FailAllStreams lifetime pitfall: RecordingSink declared BEFORE UpstreamH2Connection so reverse-destruction order is correct.

CI updates

• .github/workflows/weekly-valgrind.yml — h2_upstream added to the suite list (it's a protocol/lifecycle suite — sink lifecycle, generation counters, nghttp2 heap; weekly valgrind catches reads-of-uninitialized that ASan misses).
• ci.yml::build-linux-tsan-rest and build-macos already had h2_upstream (TSan and OS-sensitivity coverage — no edit needed).

Public docs (docs/http2_upstream.md)

Two new entries under ## Caveats:

• CONNECT rejected — operator-facing explanation of the rejection + guidance to use H1 upstreams for CONNECT tunnelling.
• Truncation observability — operators see truncation in the RESULT_UPSTREAM_DISCONNECT bucket (nghttp2 messaging enforcement intercepts before our checks); guidance to correlate with upstream-side response logs if they need to distinguish "peer reset / TCP drop" from "Content-Length short read".

Test plan

• make -j4 builds clean.
• ./test_runner — 1257/1257 passing.
• ./test_runner h2_upstream — 93/93 passing (was 63/63).
• Run the full suite twice to catch race-condition flakes — clean.
• CI dimensions to verify on push: build-linux-gcc, build-linux-clang, build-linux-asan, build-linux-tsan-heavy, build-linux-tsan-rest, build-macos. (The weekly valgrind job will exercise h2_upstream on its next Sunday run.)

What's deferred (PR-3)

• Held-fallback retry on truncation — currently RESULT_TRUNCATED_RESPONSE is terminal. Retry requires the partial-response replay buffer (paused_buffer_ + live-mirror pending_retryable_5xx_body_ + drain backpressure + deferred terminal dispatch + OnBodyChunk == CLOSED terminal). Coupled to PR-3's wait-queue / replacement-connect machinery.
• Wait-queue + replacement-connect + ALPN-h1 fallback — the checkout/lifecycle overhaul touching partition / lease / wait-queue.
• GOAWAY-unprocessed proactive RST + RESULT_GOAWAY_UNPROCESSED retry within one tick (criterion 54) — reserved result code -12.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58ac29be44

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[gemini-code-assist]

Code Review

This pull request introduces several H2 upstream enhancements, including deterministic rejection of the CONNECT method, detection of response truncation due to Content-Length mismatches, and support for te: trailers negotiation. It also implements a more granular timeout model for H2 streams, distinguishing between send stalls and response wait times. A comprehensive suite of tests has been added to verify these protocol behaviors. Review feedback identifies an opportunity to improve the te header tokenizer to handle parameters like weights, ensuring more robust detection of trailer support.

[mwfj]

LGTM