HTTP/3: bound the DATA payload copy to the frame

[brbzull0]

Http3FrameDispatcher::on_read_ready() clones the stream reader per frame and
bounds it to that frame:

auto cloned_reader        = reader.clone();
cloned_reader->size_limit = frame_len;

(src/proxy/http3/Http3FrameDispatcher.cc:112-113)

Http3StreamDataVIOAdaptor::handle_frame() then copied the payload with

int64_t written = this->_buffer->write(dframe->data());

MIOBuffer::write is declared write(IOBufferReader *r, int64_t len = INT64_MAX, int64_t offset = 0) (include/iocore/eventsystem/IOBuffer.h:997), so with no
length it walks the raw block chain and ignores the reader's size_limit
entirely. Whatever is already buffered behind the DATA frame -- typically the
next frame's header and payload -- was copied into the response body.

The fix passes reader->read_avail(), which clamps to size_limit
(src/iocore/eventsystem/IOBuffer.cc:497-499), so the copy stops at the frame
payload.

Test

Adds Http3StreamDataVIOAdaptor buffers only the current DATA frame payload to
test_Http3FrameDispatcher.cc, and adds Http3StreamDataVIOAdaptor.cc to the
test_http3 target so the adaptor is linked in.

The test writes two back-to-back DATA frames (AAAA then BBBB) into one
buffer and expects a body of exactly AAAABBBB, in two sections: both frames
arriving in a single read, and the same bytes arriving one at a time. It
asserts on the sink VIO, which is the body HttpSM goes on to forward.

Confirmed this is a regression test: restoring the unbounded
write(dframe->data()) fails
sink_reader->read_avail() == 8 (test_Http3FrameDispatcher.cc:449) with
14 == 8, because the second frame's 0x00 0x04 header and its payload are
pulled into the body. With the fix, test_http3 passes 153 assertions in 16
cases, up from 134 in 15 on master.

Also run: h3_flow_control, h3_stream_lifetime, h3_proxy_verifier autests
(3/3 pass; h3_range_cache self-skips, the local curl has no http3).

[brbzull0]

[approve ci autest 1]

[Copilot]

🟢 Approval recommended

The fix is narrowly scoped, addresses a concrete correctness issue, and is backed by a targeted regression test that exercises both coalesced and fragmented read scenarios.

Pull request overview

This PR fixes an HTTP/3 DATA-frame handling bug where payload copying could overrun the current frame boundary and incorrectly include bytes from subsequent frames already buffered on the stream. It tightens the Http3StreamDataVIOAdaptor DATA copy to the current frame’s bounded reader and adds a regression test to prevent reintroduction.

Changes:

• Bound DATA payload copying in Http3StreamDataVIOAdaptor::handle_frame() by passing reader->read_avail() to MIOBuffer::write(...).
• Added Http3StreamDataVIOAdaptor::total_data_length() to allow asserting the accumulated body size in tests.
• Added a regression test covering both “single read” and “byte-at-a-time” arrival patterns, and linked the adaptor into the test_http3 target.

File summaries

File	Description
src/proxy/http3/test/test_Http3FrameDispatcher.cc	Adds a regression test validating DATA payload copies are bounded per frame (no bleed from subsequent frames).
src/proxy/http3/Http3StreamDataVIOAdaptor.cc	Fixes DATA payload copying by explicitly bounding the write length to the frame reader’s read_avail().
src/proxy/http3/CMakeLists.txt	Links Http3StreamDataVIOAdaptor.cc into the test_http3 target for the new test.
include/proxy/http3/Http3StreamDataVIOAdaptor.h	Exposes total_data_length() for test assertions of accumulated DATA length.

Review details

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

---

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

[maskit]

Fix looks right — the bound is exact: consume() of the Type/Length fields drops the clone's size_limit to _length, and _parse() only goes true once read_avail() reaches it.

One request: drop total_data_length(). What matters is what reaches the sink VIO, since that's the body HttpSM forwards — sink_reader->read_avail() == 8 is the assertion that tests it, and it fails without the fix on its own. total_data_length() only exposes an internal counter that nothing in production reads (has_data() is just > 0). Dropping it takes the header hunk to zero.

[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 5 comments.