brbzull0 opened a new pull request, #13628:
URL: https://github.com/apache/trafficserver/pull/13628

   `QPACK::_update_largest_known_received_index_by_stream_id()` and
   `QPACK::_update_reference_counts()` both read `this->_references[stream_id]`
   (`src/proxy/http3/QPACK.cc:1020`, `:1033`). `std::map::operator[]` inserts a
   default-constructed element when the key is absent, so using it for a read
   inserts an entry for any stream id that is not already tracked.
   
   `stream_id` here comes from `_read_header_acknowledgement()` /
   `_read_stream_cancellation()`, i.e. straight off the peer's QPACK decoder
   stream, so the key is whatever the peer sent.
   
   **This is not a behaviour change.** Both call sites
   (`_on_decoder_stream_read_ready()`, `QPACK.cc:1120-1122` and `:1128-1129`) 
call
   `this->_references.erase(stream_id)` unconditionally two statements later,
   inside the same `if (... >= 0)` block with no early exit in between. And 
because
   `EntryReference` is an aggregate with no user-provided constructor,
   `operator[]` value-initializes it, so `smallest` and `largest` are both `0`:
   `largest > _largest_known_received_index` is never true for an unsigned `0`, 
and
   `if (smallest)` never fires. The inserted entry is therefore inert and then
   removed.
   
   What it does change is cost and clarity: each Header Acknowledgement or 
Stream
   Cancellation naming an untracked stream currently allocates and frees a
   `std::map` node for nothing, and `operator[]`-as-a-read is easy to misread as
   harmless when it is the reason the `erase()` calls are load-bearing. 
Replacing
   it with `find()` plus an early return removes both.
   
   ### Test
   
   No new test. The change is observationally equivalent by the reasoning 
above, so
   there is no behaviour for a regression test to pin — a test asserting the
   current outputs would pass with or without the patch.
   
   Run as a no-regression check: `h3_proxy_verifier`, `h3_python_client` and
   `h3_stream_lifetime` autests (3/3 pass), which exercise real QPACK header
   exchange including decoder-stream instructions.
   


-- 
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]

Reply via email to