brbzull0 opened a new pull request, #13635:
URL: https://github.com/apache/trafficserver/pull/13635
`HQSession::get_transaction()` (`src/proxy/http3/Http3Session.cc:100`)
matched
transactions with:
```c
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`):
```c
: 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()` is left as-is; nothing else compares or routes by it.
`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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
