HTTP/3: compare stream ids at full width in HQSession lookup

[brbzull0]

HQSession::get_transaction() (src/proxy/http3/Http3Session.cc:100) matched
transactions with:

if (t->get_transaction_id() == static_cast<int>(id)) {

HQTransaction::get_transaction_id() is declared int (it overrides
ProxyTransaction's, and exists mainly for compact log fields and probe
arguments), while QUICStreamId is a 62-bit value. So the left side narrows on
return and the right side narrows in the cast, and two live transactions whose
stream ids share the low 32 bits resolve to the same entry.

This adds HQTransaction::get_quic_stream_id() and compares against that
instead. It returns _stream_id, the QUICStreamId the constructor already
caches (src/proxy/http3/Http3Transaction.cc:68):

: super(session), _info(info), _stream_id(info.adapter.stream().id())

Reading the id back through _info.adapter.stream() at call time would
reintroduce the use-after-free that #13213 fixed, which is why the accessor uses
the cached member.

get_transaction_id() keeps its int return; nothing else compares or routes
by it. Its narrowing is now an explicit static_cast<int> with a comment
pointing at the new accessor, so the truncation reads as deliberate.
QUICStreamId is already visible in Http3Transaction.h via
iocore/net/quic/QUICStreamVCAdapter.h, so no new include is needed.

I checked for other sites narrowing a stream id the same way; this was the only
one in src/proxy/http3/ and include/proxy/http3/.

Test

No new test, and no existing test fails without this change.

The aliasing needs two concurrent transactions whose stream ids differ only
above bit 32. Client-initiated bidirectional stream ids advance by 4, and
initial_max_streams_bidi_in defaults to 100, so a stock server never issues an
id above roughly 400 -- reaching the collision requires a configuration no
default deployment uses. I would rather submit this as hardening than add a test
that only passes under a hand-tuned stream limit.

Run as a no-regression check, 8/8 pass: h3_active_timeout, h3_flow_control,
h3_go_client, h3_proxy_verifier, h3_python_client, h3_sni_check,
h3_stream_lifetime, quic_no_activity_timeout. test_http3 is unchanged at
134 assertions in 15 cases, as expected -- the lookup is not unit tested.

---

Updated 2026-09-08

• get_transaction_id() casts explicitly with static_cast<int> instead of
  narrowing implicitly on return, and carries a comment pointing at
  get_quic_stream_id() for the full-width value.
• get_quic_stream_id() is declared in the // HQTransaction block next to
  direction() rather than under // Implement ProxyClienTransaction interface, since it is not a ProxyTransaction override.

Both are cast explicitness and declaration placement; the lookup change itself
is unchanged from the original push.

[brbzull0]

[approve ci autest 2]

[brbzull0]

[approve ci freebsd]

[Copilot]

🟢 Approval recommended

The change is narrowly scoped, fixes a real ID-width correctness issue in the lookup, and the only remaining feedback is a minor clarity improvement (explicit cast) rather than a functional concern.

Pull request overview

This PR hardens HTTP/3 transaction lookup by comparing QUIC stream IDs at full width, preventing collisions caused by narrowing a 62-bit QUICStreamId to int during HQSession::get_transaction() matching.

Changes:

• Add HQTransaction::get_quic_stream_id() accessor returning the cached QUICStreamId.
• Update HQSession::get_transaction(QUICStreamId) to match transactions using the full-width stream ID.
• Document the intended distinction between get_transaction_id() (compact int) and the full QUIC stream identifier.

File summaries

File	Description
src/proxy/http3/Http3Transaction.cc	Implements get_quic_stream_id() returning the cached stream ID.
src/proxy/http3/Http3Session.cc	Updates transaction lookup comparison to use get_quic_stream_id() without truncation.
include/proxy/http3/Http3Transaction.h	Declares and documents the new full-width QUIC stream ID accessor.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The change is small, well-scoped, and corrects a concrete narrowing bug by switching lookup comparisons to the full-width cached QUICStreamId.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[maskit]

Fix looks right, and it matches how HTTP/2 already splits this — Http2Stream::get_id() for routing, get_transaction_id() for logging, with find_stream() comparing the full-width one.

One request: move get_quic_stream_id() down to the // HQTransaction block, next to direction(). It's not a ProxyTransaction override, so it doesn't belong under // Implement ProxyClienTransaction interface. (stream_closed() and set_stream_cleanup() are already misfiled there — no need for a third.)

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[maskit]

[approve ci]