(trafficserver) branch master updated: HTTP/3: bound the DATA payload copy to the frame (#13629)

[dm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

brbzull0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 902a2432f3 HTTP/3: bound the DATA payload copy to the frame (#13629)
902a2432f3 is described below

commit 902a2432f394a09c17c49cb3adb908fc81e9a771
Author: Damian Meden <dm...@apache.org>
AuthorDate: Fri Sep 11 10:01:14 2026 +0200

    HTTP/3: bound the DATA payload copy to the frame (#13629)
    
    Http3FrameDispatcher hands each frame a cloned reader whose size_limit
    is that frame's length, but handle_frame() called
    MIOBuffer::write(reader) with no length. That defaults to INT64_MAX and
    walks the raw block chain, so anything already buffered behind the DATA
    frame -- a following frame header and its payload -- was copied into the
    body too. Pass read_avail(), which honours size_limit.
---
 src/proxy/http3/Http3StreamDataVIOAdaptor.cc      |  9 ++-
 src/proxy/http3/test/test_Http3FrameDispatcher.cc | 72 +++++++++++++++++++++++
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/src/proxy/http3/Http3StreamDataVIOAdaptor.cc b/src/proxy/http3/Http3StreamDataVIOAdaptor.cc
index cdfd1ee374..f59e303e31 100644
--- a/src/proxy/http3/Http3StreamDataVIOAdaptor.cc
+++ b/src/proxy/http3/Http3StreamDataVIOAdaptor.cc
@@ -48,8 +48,13 @@ Http3StreamDataVIOAdaptor::handle_frame(std::shared_ptr<const Http3Frame> frame,
   ink_assert(frame->type() == Http3FrameType::DATA);
   const Http3DataFrame *dframe = dynamic_cast<const Http3DataFrame *>(frame.get());
 
-  // Need to wait for headers to be written
-  int64_t written           = this->_buffer->write(dframe->data());
+  // The frame's reader is bounded to this frame by size_limit, but the default
+  // length for MIOBuffer::write is INT64_MAX, which walks the raw block chain
+  // and copies whatever follows the frame in the stream read buffer. Pass
+  // read_avail(), which honours size_limit, to bound the copy to the payload.
+  IOBufferReader *reader  = dframe->data();
+  int64_t         written = this->_buffer->write(reader, reader->read_avail());
+
   this->_total_data_length += written;
 
   return Http3ErrorUPtr(nullptr);
diff --git a/src/proxy/http3/test/test_Http3FrameDispatcher.cc b/src/proxy/http3/test/test_Http3FrameDispatcher.cc
index e4466ea175..c1cb5920d2 100644
--- a/src/proxy/http3/test/test_Http3FrameDispatcher.cc
+++ b/src/proxy/http3/test/test_Http3FrameDispatcher.cc
@@ -23,8 +23,13 @@
 
 #include <catch2/catch_test_macros.hpp>
 
+#include <array>
+#include <string_view>
+
+#include "iocore/eventsystem/VIO.h"
 #include "proxy/http3/Http3FrameDispatcher.h"
 #include "proxy/http3/Http3ProtocolEnforcer.h"
+#include "proxy/http3/Http3StreamDataVIOAdaptor.h"
 #include "Mock.h"
 
 namespace
@@ -385,6 +390,73 @@ TEST_CASE("ignore unknown frames", "[http3]")
   }
 }
 
+TEST_CASE("Http3StreamDataVIOAdaptor buffers only the current DATA frame payload", "[http3]")
+{
+  // The dispatcher hands each handler a reader whose size_limit covers just
+  // one frame. The adaptor must respect that limit, so the body it collects
+  // is the concatenated payloads and nothing else, no matter how the frames
+  // are split across reads.
+  MIOBuffer      *sink_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_512);
+  IOBufferReader *sink_reader = sink_buffer->alloc_reader();
+  VIO             sink_vio;
+
+  sink_vio.mutex = new_ProxyMutex();
+  sink_vio.set_writer(sink_buffer);
+
+  Http3FrameDispatcher      http3FrameDispatcher;
+  Http3StreamDataVIOAdaptor adaptor(&sink_vio);
+  http3FrameDispatcher.add_handler(&adaptor);
+
+  MIOBuffer      *buf    = new_MIOBuffer(BUFFER_SIZE_INDEX_512);
+  IOBufferReader *reader = buf->alloc_reader();
+  uint64_t        nread  = 0;
+
+  uint8_t input[] = {// 1st frame (DATA)
+                     0x00, 0x04, 'A', 'A', 'A', 'A',
+                     // 2nd frame (DATA)
+                     0x00, 0x04, 'B', 'B', 'B', 'B'};
+
+  constexpr std::string_view expected_body = "AAAABBBB";
+
+  SECTION("Both frames arrive in a single read")
+  {
+    // The first frame's payload is followed in the same buffer by the second
+    // frame, so an unbounded copy would pull the trailing header and payload
+    // into the body as well.
+    buf->write(input, sizeof(input));
+
+    Http3ErrorUPtr error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::UNKNOWN, *reader, nread);
+    CHECK(!error);
+    CHECK(nread == sizeof(input));
+  }
+
+  SECTION("Frames arrive one byte at a time")
+  {
+    // on_read_ready resets nread on every call, so total it up as we go.
+    uint64_t consumed = 0;
+
+    for (uint8_t *it{input}; it < input + sizeof(input); ++it) {
+      buf->write(it, 1);
+
+      Http3ErrorUPtr error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::UNKNOWN, *reader, nread);
+      CHECK(!error);
+      consumed += nread;
+    }
+    CHECK(consumed == sizeof(input));
+  }
+
+  adaptor.finalize();
+  REQUIRE(sink_reader->read_avail() == static_cast<int64_t>(expected_body.size()));
+
+  std::array<char, expected_body.size()> body;
+
+  sink_reader->memcpy(body.data(), body.size());
+  CHECK(std::string_view(body.data(), body.size()) == expected_body);
+
+  free_MIOBuffer(buf);
+  free_MIOBuffer(sink_buffer);
+}
+
 TEST_CASE("HTTP/2 reserved frame type not allowed", "[http3]")
 {
   SECTION("Reject reserved frame type in non control stream")