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

cmcfarlen pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 58f8c6ff2fdb23351c0c1d7df79faf5a77d4c5c4
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Fri Jul 5 09:18:17 2024 -0600

    Enable receiving large H3 frames (#11497)
    
    * http3: Support reading large H3 frames
    
    * Update tests
    
    * Fix buffer-overflow
    
    (cherry picked from commit bd5f3eded5a9d534fca0b3324f9ff011a9f62494)
---
 include/proxy/http3/Http3Frame.h             |  60 ++++--
 include/proxy/http3/Http3FrameDispatcher.h   |   2 +
 src/proxy/http3/Http3Frame.cc                | 292 +++++++++++++++++----------
 src/proxy/http3/Http3FrameDispatcher.cc      |  68 ++++---
 src/proxy/http3/Http3StreamDataVIOAdaptor.cc |   4 +-
 src/proxy/http3/test/test_Http3Frame.cc      |  85 +++++---
 6 files changed, 323 insertions(+), 188 deletions(-)

diff --git a/include/proxy/http3/Http3Frame.h b/include/proxy/http3/Http3Frame.h
index 2b13be1c53..08ac54115a 100644
--- a/include/proxy/http3/Http3Frame.h
+++ b/include/proxy/http3/Http3Frame.h
@@ -35,29 +35,39 @@ public:
   constexpr static size_t MAX_FRAM_HEADER_OVERHEAD = 128; ///< Type (i) + 
Length (i)
 
   Http3Frame() {}
-  Http3Frame(const uint8_t *buf, size_t len);
+  Http3Frame(IOBufferReader &reader);
   Http3Frame(Http3FrameType type);
-  virtual ~Http3Frame() {}
+  virtual ~Http3Frame();
 
+  bool                       is_valid() const;
   uint64_t                   total_length() const;
   uint64_t                   length() const;
   Http3FrameType             type() const;
+  bool                       update();
   virtual Ptr<IOBufferBlock> to_io_buffer_block() const;
-  virtual void               reset(const uint8_t *buf, size_t len);
+  virtual void               reset(IOBufferReader &reader);
   static int                 length(const uint8_t *buf, size_t buf_len, 
uint64_t &length);
   static Http3FrameType      type(const uint8_t *buf, size_t buf_len);
 
 protected:
-  uint64_t       _length         = 0;
-  Http3FrameType _type           = Http3FrameType::UNKNOWN;
-  size_t         _payload_offset = 0;
+  IOBufferReader *_reader           = nullptr;
+  bool            _finished_reading = false;
+  uint64_t        _length           = 0;
+  Http3FrameType  _type             = Http3FrameType::UNKNOWN;
+  size_t          _payload_offset   = 0;
+  bool            _is_valid         = true;
+
+  virtual bool _parse();
+
+private:
+  bool _is_ready = false;
 };
 
 class Http3UnknownFrame : public Http3Frame
 {
 public:
   Http3UnknownFrame() : Http3Frame() {}
-  Http3UnknownFrame(const uint8_t *buf, size_t len);
+  Http3UnknownFrame(IOBufferReader &reader);
 
   Ptr<IOBufferBlock> to_io_buffer_block() const override;
 
@@ -74,15 +84,17 @@ class Http3DataFrame : public Http3Frame
 {
 public:
   Http3DataFrame() : Http3Frame() {}
-  Http3DataFrame(const uint8_t *buf, size_t len);
+  Http3DataFrame(IOBufferReader &reader);
   Http3DataFrame(ats_unique_buf payload, size_t payload_len);
 
   Ptr<IOBufferBlock> to_io_buffer_block() const override;
-  void               reset(const uint8_t *buf, size_t len) override;
+  void               reset(IOBufferReader &reader) override;
 
   const uint8_t *payload() const;
   uint64_t       payload_length() const;
 
+  IOBufferReader *data() const;
+
 private:
   const uint8_t *_payload      = nullptr;
   ats_unique_buf _payload_uptr = {nullptr};
@@ -97,17 +109,21 @@ class Http3HeadersFrame : public Http3Frame
 {
 public:
   Http3HeadersFrame() : Http3Frame() {}
-  Http3HeadersFrame(const uint8_t *buf, size_t len);
+  Http3HeadersFrame(IOBufferReader &reader);
   Http3HeadersFrame(ats_unique_buf header_block, size_t header_block_len);
+  ~Http3HeadersFrame();
 
   Ptr<IOBufferBlock> to_io_buffer_block() const override;
-  void               reset(const uint8_t *buf, size_t len) override;
+  void               reset(IOBufferReader &reader) override;
 
   const uint8_t *header_block() const;
   uint64_t       header_block_length() const;
 
+protected:
+  bool _parse() override;
+
 private:
-  const uint8_t *_header_block      = nullptr;
+  uint8_t       *_header_block      = nullptr;
   ats_unique_buf _header_block_uptr = {nullptr};
   size_t         _header_block_len  = 0;
 };
@@ -120,7 +136,7 @@ class Http3SettingsFrame : public Http3Frame
 {
 public:
   Http3SettingsFrame() : Http3Frame(Http3FrameType::SETTINGS) {}
-  Http3SettingsFrame(const uint8_t *buf, size_t len, uint32_t max_settings = 
0);
+  Http3SettingsFrame(IOBufferReader &reader, uint32_t max_settings = 0);
 
   static constexpr size_t                         MAX_PAYLOAD_SIZE = 60;
   static constexpr std::array<Http3SettingsId, 4> VALID_SETTINGS_IDS{
@@ -131,21 +147,22 @@ public:
   };
 
   Ptr<IOBufferBlock> to_io_buffer_block() const override;
-  void               reset(const uint8_t *buf, size_t len) override;
+  void               reset(IOBufferReader &reader) override;
 
-  bool           is_valid() const;
   Http3ErrorUPtr get_error() const;
 
   bool     contains(Http3SettingsId id) const;
   uint64_t get(Http3SettingsId id) const;
   void     set(Http3SettingsId id, uint64_t value);
 
+protected:
+  bool _parse() override;
+
 private:
+  uint32_t                            _max_settings = 0;
   std::map<Http3SettingsId, uint64_t> _settings;
-  // TODO: make connection error with HTTP_MALFORMED_FRAME
-  bool           _valid = false;
-  Http3ErrorCode _error_code;
-  const char    *_error_reason = nullptr;
+  Http3ErrorCode                      _error_code;
+  const char                         *_error_reason = nullptr;
 };
 
 using Http3FrameDeleterFunc  = void (*)(Http3Frame *p);
@@ -217,14 +234,13 @@ public:
   /*
    * This is used for creating a Http3Frame object based on received data.
    */
-  static Http3FrameUPtr create(const uint8_t *buf, size_t len);
+  static Http3FrameUPtr create(IOBufferReader &reader);
 
   /*
    * This works almost the same as create() but it reuses created objects for 
performance.
    * If you create a frame object which has the same frame type that you 
created before, the object will be reset by new data.
    */
-  std::shared_ptr<const Http3Frame> fast_create(IOBufferReader &reader, size_t 
frame_len);
-  std::shared_ptr<const Http3Frame> fast_create(const uint8_t *buf, size_t 
len);
+  std::shared_ptr<Http3Frame> fast_create(IOBufferReader &reader);
 
   /*
    * Creates a HEADERS frame.
diff --git a/include/proxy/http3/Http3FrameDispatcher.h 
b/include/proxy/http3/Http3FrameDispatcher.h
index f0b2360a0e..d904956c1d 100644
--- a/include/proxy/http3/Http3FrameDispatcher.h
+++ b/include/proxy/http3/Http3FrameDispatcher.h
@@ -47,6 +47,8 @@ private:
   int64_t                          _reading_frame_type_len;
   int64_t                          _reading_frame_length_len;
   uint64_t                         _reading_frame_payload_len;
+  uint64_t                         _bytes_to_skip;
   Http3FrameFactory                _frame_factory;
+  std::shared_ptr<Http3Frame>      _current_frame = nullptr;
   std::vector<Http3FrameHandler *> _handlers[256];
 };
diff --git a/src/proxy/http3/Http3Frame.cc b/src/proxy/http3/Http3Frame.cc
index 96c46f794d..f2e3b91ca2 100644
--- a/src/proxy/http3/Http3Frame.cc
+++ b/src/proxy/http3/Http3Frame.cc
@@ -68,28 +68,61 @@ Http3Frame::type(const uint8_t *buf, size_t buf_len)
 // Generic Frame
 //
 
-Http3Frame::Http3Frame(const uint8_t *buf, size_t buf_len)
+Http3Frame::Http3Frame(IOBufferReader &reader) : _reader(&reader)
 {
+  // Type field and Length field are variable-length integers, which can be 64 
bits for each.
+  // 16 bytes (128 bits) is large enough to contain the two fields.
+  constexpr int MAX_HEADER_SIZE = 16;
+  uint8_t       header[MAX_HEADER_SIZE];
+  uint8_t      *p          = reinterpret_cast<uint8_t *>(reader.memcpy(header, 
MAX_HEADER_SIZE));
+  uint64_t      header_len = p - header;
+
   // Type
   size_t   type_field_length = 0;
   uint64_t type              = 0;
   // Ideally we'd simply pass this->_type to decode, but arm compilers 
complain with:
   // error: dereferencing type-punned pointer will break strict-aliasing rules
-  int ret     = QUICVariableInt::decode(type, type_field_length, buf, buf_len);
+  int ret = QUICVariableInt::decode(type, type_field_length, header, 
header_len);
+  if (ret != 0) {
+    this->_is_valid = false;
+    return;
+  }
   this->_type = static_cast<Http3FrameType>(type);
-  ink_assert(ret != 1);
+
+  // Check if we can proceed to read Length field
+  if (header_len <= type_field_length) {
+    this->_is_valid = false;
+    return;
+  }
 
   // Length
   size_t length_field_length = 0;
-  ret = QUICVariableInt::decode(this->_length, length_field_length, buf + 
type_field_length, buf_len - type_field_length);
-  ink_assert(ret != 1);
+  ret = QUICVariableInt::decode(this->_length, length_field_length, header + 
type_field_length, header_len - type_field_length);
+  if (ret != 0) {
+    this->_is_valid = false;
+    return;
+  }
 
-  // Payload offset
+  // Rest of the data is Frame Payload
+  this->_reader->consume(type_field_length + length_field_length);
   this->_payload_offset = type_field_length + length_field_length;
 }
 
 Http3Frame::Http3Frame(Http3FrameType type) : _type(type) {}
 
+Http3Frame::~Http3Frame()
+{
+  if (this->_reader) {
+    this->_reader->dealloc();
+  }
+}
+
+bool
+Http3Frame::is_valid() const
+{
+  return this->_is_valid;
+}
+
 uint64_t
 Http3Frame::total_length() const
 {
@@ -112,6 +145,22 @@ Http3Frame::type() const
   }
 }
 
+bool
+Http3Frame::update()
+{
+  if (!this->_is_ready) {
+    this->_is_ready = this->_parse();
+  }
+
+  return this->_is_ready;
+}
+
+bool
+Http3Frame::_parse()
+{
+  return true;
+}
+
 Ptr<IOBufferBlock>
 Http3Frame::to_io_buffer_block() const
 {
@@ -120,16 +169,16 @@ Http3Frame::to_io_buffer_block() const
 }
 
 void
-Http3Frame::reset(const uint8_t *buf, size_t len)
+Http3Frame::reset(IOBufferReader &reader)
 {
   this->~Http3Frame();
-  new (this) Http3Frame(buf, len);
+  new (this) Http3Frame(reader);
 }
 
 //
 // UNKNOWN Frame
 //
-Http3UnknownFrame::Http3UnknownFrame(const uint8_t *buf, size_t buf_len) : 
Http3Frame(buf, buf_len), _buf(buf), _buf_len(buf_len) {}
+Http3UnknownFrame::Http3UnknownFrame(IOBufferReader &reader) : 
Http3Frame(reader) {}
 
 Ptr<IOBufferBlock>
 Http3UnknownFrame::to_io_buffer_block() const
@@ -150,10 +199,9 @@ Http3UnknownFrame::to_io_buffer_block() const
 //
 // DATA Frame
 //
-Http3DataFrame::Http3DataFrame(const uint8_t *buf, size_t buf_len) : 
Http3Frame(buf, buf_len)
+Http3DataFrame::Http3DataFrame(IOBufferReader &reader) : Http3Frame(reader)
 {
-  this->_payload     = buf + this->_payload_offset;
-  this->_payload_len = buf_len - this->_payload_offset;
+  this->_payload_len = this->_length;
 }
 
 Http3DataFrame::Http3DataFrame(ats_unique_buf payload, size_t payload_len)
@@ -186,10 +234,10 @@ Http3DataFrame::to_io_buffer_block() const
 }
 
 void
-Http3DataFrame::reset(const uint8_t *buf, size_t len)
+Http3DataFrame::reset(IOBufferReader &reader)
 {
   this->~Http3DataFrame();
-  new (this) Http3DataFrame(buf, len);
+  new (this) Http3DataFrame(reader);
 }
 
 const uint8_t *
@@ -204,14 +252,16 @@ Http3DataFrame::payload_length() const
   return this->_payload_len;
 }
 
+IOBufferReader *
+Http3DataFrame::data() const
+{
+  return _reader;
+}
+
 //
 // HEADERS Frame
 //
-Http3HeadersFrame::Http3HeadersFrame(const uint8_t *buf, size_t buf_len) : 
Http3Frame(buf, buf_len)
-{
-  this->_header_block     = buf + this->_payload_offset;
-  this->_header_block_len = buf_len - this->_payload_offset;
-}
+Http3HeadersFrame::Http3HeadersFrame(IOBufferReader &reader) : 
Http3Frame(reader) {}
 
 Http3HeadersFrame::Http3HeadersFrame(ats_unique_buf header_block, size_t 
header_block_len)
   : Http3Frame(Http3FrameType::HEADERS), 
_header_block_uptr(std::move(header_block)), _header_block_len(header_block_len)
@@ -220,6 +270,13 @@ Http3HeadersFrame::Http3HeadersFrame(ats_unique_buf 
header_block, size_t header_
   this->_header_block = this->_header_block_uptr.get();
 }
 
+Http3HeadersFrame::~Http3HeadersFrame()
+{
+  if (this->_header_block_uptr == nullptr) {
+    ats_free(this->_header_block);
+  }
+}
+
 Ptr<IOBufferBlock>
 Http3HeadersFrame::to_io_buffer_block() const
 {
@@ -243,10 +300,10 @@ Http3HeadersFrame::to_io_buffer_block() const
 }
 
 void
-Http3HeadersFrame::reset(const uint8_t *buf, size_t len)
+Http3HeadersFrame::reset(IOBufferReader &reader)
 {
   this->~Http3HeadersFrame();
-  new (this) Http3HeadersFrame(buf, len);
+  new (this) Http3HeadersFrame(reader);
 }
 
 const uint8_t *
@@ -261,61 +318,28 @@ Http3HeadersFrame::header_block_length() const
   return this->_header_block_len;
 }
 
-//
-// SETTINGS Frame
-//
-
-Http3SettingsFrame::Http3SettingsFrame(const uint8_t *buf, size_t buf_len, 
uint32_t max_settings) : Http3Frame(buf, buf_len)
+bool
+Http3HeadersFrame::_parse()
 {
-  size_t   len       = this->_payload_offset;
-  uint32_t nsettings = 0;
-
-  while (len < buf_len) {
-    if (nsettings >= max_settings) {
-      this->_error_code   = Http3ErrorCode::H3_EXCESSIVE_LOAD;
-      this->_error_reason = reinterpret_cast<const char *>("too many 
settings");
-      break;
-    }
-
-    size_t id_len = QUICVariableInt::size(buf + len);
-    if ((len + id_len) >=
-        buf_len) { // if the id is larger than the buffer or at the boundary 
of the buffer (i.e. no value), it is invalid
-      this->_error_code   = Http3ErrorCode::H3_SETTINGS_ERROR;
-      this->_error_reason = reinterpret_cast<const char *>("invalid SETTINGS 
frame");
-      break;
-    }
-    uint16_t id  = QUICIntUtil::read_QUICVariableInt(buf + len, buf_len - len);
-    len         += id_len;
-
-    size_t value_len = QUICVariableInt::size(buf + len);
-    if ((len + value_len) > buf_len) {
-      this->_error_code   = Http3ErrorCode::H3_SETTINGS_ERROR;
-      this->_error_reason = reinterpret_cast<const char *>("invalid SETTINGS 
frame");
-      break;
-    }
-    uint64_t value  = QUICIntUtil::read_QUICVariableInt(buf + len, buf_len - 
len);
-    len            += value_len;
+  if (this->_reader->read_avail() != static_cast<int64_t>(this->_length)) {
+    // Whole payload is not received yet
+    return false;
+  }
 
-    // Ignore any SETTINGS identifier it does not understand.
-    bool ignore = true;
-    for (const auto &known_id : Http3SettingsFrame::VALID_SETTINGS_IDS) {
-      if (id == static_cast<uint64_t>(known_id)) {
-        ignore = false;
-        break;
-      }
-    }
+  this->_header_block_len = this->_length;
+  this->_header_block     = static_cast<uint8_t 
*>(ats_malloc(this->_header_block_len));
+  this->_reader->memcpy(this->_header_block, this->_header_block_len);
 
-    if (ignore) {
-      continue;
-    }
+  return true;
+}
 
-    this->_settings.insert(std::make_pair(static_cast<Http3SettingsId>(id), 
value));
-    ++nsettings;
-  }
+//
+// SETTINGS Frame
+//
 
-  if (len == buf_len) {
-    this->_valid = true;
-  }
+Http3SettingsFrame::Http3SettingsFrame(IOBufferReader &reader, uint32_t 
max_settings)
+  : Http3Frame(reader), _max_settings(max_settings)
+{
 }
 
 Ptr<IOBufferBlock>
@@ -363,16 +387,10 @@ Http3SettingsFrame::to_io_buffer_block() const
 }
 
 void
-Http3SettingsFrame::reset(const uint8_t *buf, size_t len)
+Http3SettingsFrame::reset(IOBufferReader &reader)
 {
   this->~Http3SettingsFrame();
-  new (this) Http3SettingsFrame(buf, len);
-}
-
-bool
-Http3SettingsFrame::is_valid() const
-{
-  return this->_valid;
+  new (this) Http3SettingsFrame(reader);
 }
 
 Http3ErrorUPtr
@@ -405,6 +423,74 @@ Http3SettingsFrame::set(Http3SettingsId id, uint64_t value)
   this->_settings[id] = value;
 }
 
+bool
+Http3SettingsFrame::_parse()
+{
+  if (this->_reader->read_avail() != static_cast<int64_t>(this->_length)) {
+    // Whole payload is not received yet
+    return false;
+  }
+
+  uint8_t *buf = static_cast<uint8_t *>(ats_malloc(this->_length));
+  this->_reader->memcpy(buf, this->_length);
+
+  size_t   len       = 0;
+  uint32_t nsettings = 0;
+
+  while (len < this->_length) {
+    if (nsettings >= this->_max_settings) {
+      this->_error_code   = Http3ErrorCode::H3_EXCESSIVE_LOAD;
+      this->_error_reason = reinterpret_cast<const char *>("too many 
settings");
+      this->_is_valid     = false;
+      break;
+    }
+
+    size_t id_len = QUICVariableInt::size(buf + len);
+    if ((len + id_len) >=
+        this->_length) { // if the id is larger than the buffer or at the 
boundary of the buffer (i.e. no value), it is invalid
+      this->_error_code   = Http3ErrorCode::H3_SETTINGS_ERROR;
+      this->_error_reason = reinterpret_cast<const char *>("invalid SETTINGS 
frame");
+      break;
+    }
+    uint16_t id  = QUICIntUtil::read_QUICVariableInt(buf + len, this->_length 
- len);
+    len         += id_len;
+
+    size_t value_len = QUICVariableInt::size(buf + len);
+    if ((len + value_len) > this->_length) {
+      this->_error_code   = Http3ErrorCode::H3_SETTINGS_ERROR;
+      this->_error_reason = reinterpret_cast<const char *>("invalid SETTINGS 
frame");
+      break;
+    }
+    uint64_t value  = QUICIntUtil::read_QUICVariableInt(buf + len, 
this->_length - len);
+    len            += value_len;
+
+    // Ignore any SETTINGS identifier it does not understand.
+    bool ignore = true;
+    for (const auto &known_id : Http3SettingsFrame::VALID_SETTINGS_IDS) {
+      if (id == static_cast<uint64_t>(known_id)) {
+        ignore = false;
+        break;
+      }
+    }
+
+    if (ignore) {
+      continue;
+    }
+
+    this->_settings.insert(std::make_pair(static_cast<Http3SettingsId>(id), 
value));
+    ++nsettings;
+  }
+
+  if (len != this->_length) {
+    // TODO: make connection error with HTTP_MALFORMED_FRAME
+    this->_is_valid = false;
+  }
+
+  ats_free(buf);
+
+  return true;
+}
+
 //
 // Http3FrameFactory
 //
@@ -415,43 +501,50 @@ Http3FrameFactory::create_null_frame()
 }
 
 Http3FrameUPtr
-Http3FrameFactory::create(const uint8_t *buf, size_t len)
+Http3FrameFactory::create(IOBufferReader &reader)
 {
   ts::Http3Config::scoped_config params;
   Http3Frame                    *frame = nullptr;
-  Http3FrameType                 type  = Http3Frame::type(buf, len);
+
+  // FIXME Frame type can be longer than 1 byte
+  uint8_t type_buf[1];
+  reader.memcpy(type_buf, sizeof(type_buf));
+  Http3FrameType type = Http3Frame::type(type_buf, sizeof(type_buf));
 
   switch (type) {
   case Http3FrameType::HEADERS:
     frame = http3HeadersFrameAllocator.alloc();
-    new (frame) Http3HeadersFrame(buf, len);
+    new (frame) Http3HeadersFrame(reader);
     return Http3FrameUPtr(frame, &Http3FrameDeleter::delete_headers_frame);
   case Http3FrameType::DATA:
     frame = http3DataFrameAllocator.alloc();
-    new (frame) Http3DataFrame(buf, len);
+    new (frame) Http3DataFrame(reader);
     return Http3FrameUPtr(frame, &Http3FrameDeleter::delete_data_frame);
   case Http3FrameType::SETTINGS:
     frame = http3SettingsFrameAllocator.alloc();
-    new (frame) Http3SettingsFrame(buf, len, params->max_settings());
+    new (frame) Http3SettingsFrame(reader, params->max_settings());
     return Http3FrameUPtr(frame, &Http3FrameDeleter::delete_settings_frame);
   default:
     // Unknown frame
     Dbg(dbg_ctl_http3_frame_factory, "Unknown frame type %hhx", 
static_cast<uint8_t>(type));
     frame = http3FrameAllocator.alloc();
-    new (frame) Http3Frame(buf, len);
+    new (frame) Http3Frame(reader);
     return Http3FrameUPtr(frame, &Http3FrameDeleter::delete_frame);
   }
 }
 
-std::shared_ptr<const Http3Frame>
-Http3FrameFactory::fast_create(const uint8_t *buf, size_t len)
+std::shared_ptr<Http3Frame>
+Http3FrameFactory::fast_create(IOBufferReader &reader)
 {
-  Http3FrameType type = Http3Frame::type(buf, len);
+  // FIXME Frame type can be longer than 1 byte
+  uint8_t type_buf[1];
+  reader.memcpy(type_buf, sizeof(type_buf));
+  Http3FrameType type = Http3Frame::type(type_buf, sizeof(type_buf));
   if (type == Http3FrameType::UNKNOWN) {
     if (!this->_unknown_frame) {
-      this->_unknown_frame = Http3FrameFactory::create(buf, len);
+      this->_unknown_frame = Http3FrameFactory::create(reader);
     } else {
-      this->_unknown_frame->reset(buf, len);
+      this->_unknown_frame->reset(reader);
     }
     return this->_unknown_frame;
   }
@@ -459,32 +552,17 @@ Http3FrameFactory::fast_create(const uint8_t *buf, size_t 
len)
   std::shared_ptr<Http3Frame> frame = 
this->_reusable_frames[static_cast<uint8_t>(type)];
 
   if (frame == nullptr) {
-    frame = Http3FrameFactory::create(buf, len);
+    frame = Http3FrameFactory::create(reader);
     if (frame != nullptr) {
       this->_reusable_frames[static_cast<uint8_t>(type)] = frame;
     }
   } else {
-    frame->reset(buf, len);
+    frame->reset(reader);
   }
 
   return frame;
 }
 
-std::shared_ptr<const Http3Frame>
-Http3FrameFactory::fast_create(IOBufferReader &reader, size_t frame_len)
-{
-  uint8_t buf[65536];
-
-  // FIXME DATA frames can be giga bytes
-  ink_assert(sizeof(buf) > frame_len);
-
-  if (reader.read(buf, frame_len) < static_cast<int64_t>(frame_len)) {
-    // Return if whole frame data is not available
-    return nullptr;
-  }
-  return this->fast_create(buf, frame_len);
-}
-
 Http3HeadersFrameUPtr
 Http3FrameFactory::create_headers_frame(const uint8_t *header_block, size_t 
header_block_len)
 {
diff --git a/src/proxy/http3/Http3FrameDispatcher.cc 
b/src/proxy/http3/Http3FrameDispatcher.cc
index ae1a6e4771..1b6b3f32bb 100644
--- a/src/proxy/http3/Http3FrameDispatcher.cc
+++ b/src/proxy/http3/Http3FrameDispatcher.cc
@@ -50,10 +50,9 @@ Http3FrameDispatcher::add_handler(Http3FrameHandler *handler)
 Http3ErrorUPtr
 Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id, Http3StreamType 
stream_type, IOBufferReader &reader, uint64_t &nread)
 {
-  std::shared_ptr<const Http3Frame> frame(nullptr);
-  Http3ErrorUPtr                    error = Http3ErrorUPtr(nullptr);
-  nread                                   = 0;
-  uint32_t frame_count                    = 0;
+  Http3ErrorUPtr error = Http3ErrorUPtr(nullptr);
+  nread                = 0;
+  uint32_t frame_count = 0;
 
   while (true) {
     // Read a length of Type field and hopefully a length of Length field too
@@ -94,32 +93,49 @@ Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id, 
Http3StreamType stre
     }
 
     if (this->_reading_state == READING_PAYLOAD) {
-      // Create a frame
-      // Type field length + Length field length + Payload length
-      size_t frame_len     = this->_reading_frame_type_len + 
this->_reading_frame_length_len + this->_reading_frame_payload_len;
-      auto   cloned_reader = reader.clone();
-      frame                = this->_frame_factory.fast_create(*cloned_reader, 
frame_len);
-      cloned_reader->dealloc();
-      if (frame == nullptr) {
+      if (read_len == 0) {
         break;
       }
-      ++frame_count;
-
-      // Consume buffer if frame is created
-      nread += frame_len;
-      reader.consume(frame_len);
-
-      // Dispatch
-      Http3FrameType type = frame->type();
-      Dbg(dbg_ctl_http3, "[RX] [%" PRIu64 "] | %s size=%zu", stream_id, 
Http3DebugNames::frame_type(type), frame_len);
-      std::vector<Http3FrameHandler *> handlers = 
this->_handlers[static_cast<uint8_t>(type)];
-      for (auto h : handlers) {
-        error = h->handle_frame(frame, frame_count - 1, stream_type);
-        if (error && error->cls != Http3ErrorClass::UNDEFINED) {
-          return error;
+      if (this->_current_frame == nullptr) {
+        // Create a frame
+        // Type field length + Length field length + Payload length
+        size_t frame_len = this->_reading_frame_type_len + 
this->_reading_frame_length_len + this->_reading_frame_payload_len;
+
+        // Create a reader to read one frame. The reader will be deallocated 
by the frame object created
+        auto cloned_reader        = reader.clone();
+        cloned_reader->size_limit = frame_len;
+        this->_current_frame      = 
this->_frame_factory.fast_create(*cloned_reader);
+        if (this->_current_frame == nullptr) {
+          cloned_reader->dealloc();
+          break;
         }
+        this->_bytes_to_skip = this->_current_frame->total_length();
+        ++frame_count;
+      }
+
+      auto skip = std::min(static_cast<uint64_t>(reader.read_avail()), 
this->_bytes_to_skip);
+      reader.consume(skip);
+      this->_bytes_to_skip -= skip;
+      nread                += skip;
+
+      if (this->_current_frame->update()) {
+        // Dispatch
+        Http3FrameType type = this->_current_frame->type();
+        Debug("http3", "[RX] [%" PRIu64 "] | %s size=%" PRIu64 "/%" PRIu64, 
stream_id, Http3DebugNames::frame_type(type),
+              this->_current_frame->total_length() - _bytes_to_skip, 
this->_current_frame->total_length());
+        std::vector<Http3FrameHandler *> handlers = 
this->_handlers[static_cast<uint8_t>(type)];
+        for (auto h : handlers) {
+          error = h->handle_frame(this->_current_frame, frame_count - 1, 
stream_type);
+          if (error && error->cls != Http3ErrorClass::UNDEFINED) {
+            return error;
+          }
+        }
+      }
+
+      if (this->_bytes_to_skip == 0) {
+        this->_current_frame = nullptr;
+        this->_reading_state = READING_TYPE_LEN;
       }
-      this->_reading_state = READING_TYPE_LEN;
     }
   }
 
diff --git a/src/proxy/http3/Http3StreamDataVIOAdaptor.cc 
b/src/proxy/http3/Http3StreamDataVIOAdaptor.cc
index 0a9749bf6b..d4ffcb3a21 100644
--- a/src/proxy/http3/Http3StreamDataVIOAdaptor.cc
+++ b/src/proxy/http3/Http3StreamDataVIOAdaptor.cc
@@ -45,8 +45,8 @@ Http3StreamDataVIOAdaptor::handle_frame(std::shared_ptr<const 
Http3Frame> frame,
   const Http3DataFrame *dframe = dynamic_cast<const Http3DataFrame 
*>(frame.get());
 
   // Need to wait for headers to be written
-  this->_buffer->write(dframe->payload(), dframe->payload_length());
-  this->_total_data_length += dframe->payload_length();
+  int64_t written           = this->_buffer->write(dframe->data());
+  this->_total_data_length += written;
 
   return Http3ErrorUPtr(nullptr);
 }
diff --git a/src/proxy/http3/test/test_Http3Frame.cc 
b/src/proxy/http3/test/test_Http3Frame.cc
index 80010a2976..c28267f27d 100644
--- a/src/proxy/http3/test/test_Http3Frame.cc
+++ b/src/proxy/http3/test/test_Http3Frame.cc
@@ -37,38 +37,29 @@ TEST_CASE("Http3Frame Type", "[http3]")
 
 TEST_CASE("Load DATA Frame", "[http3]")
 {
-  SECTION("No flags")
+  SECTION("Normal")
   {
     uint8_t buf1[] = {
       0x00,                   // Type
       0x04,                   // Length
       0x11, 0x22, 0x33, 0x44, // Payload
     };
-    std::shared_ptr<const Http3Frame> frame1 = Http3FrameFactory::create(buf1, 
sizeof(buf1));
+    MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+    input->write(buf1, sizeof(buf1));
+    IOBufferReader             *input_reader = input->alloc_reader();
+    std::shared_ptr<Http3Frame> frame1       = 
Http3FrameFactory::create(*input_reader);
+    frame1->update();
     CHECK(frame1->type() == Http3FrameType::DATA);
     CHECK(frame1->length() == 4);
 
-    std::shared_ptr<const Http3DataFrame> data_frame = 
std::dynamic_pointer_cast<const Http3DataFrame>(frame1);
+    std::shared_ptr<Http3DataFrame> data_frame = 
std::dynamic_pointer_cast<Http3DataFrame>(frame1);
     CHECK(data_frame);
     CHECK(data_frame->payload_length() == 4);
-    CHECK(memcmp(data_frame->payload(), "\x11\x22\x33\x44", 4) == 0);
-  }
+    IOBufferReader *data_reader = data_frame->data();
+    CHECK(data_reader->read_avail() == 4);
+    CHECK(memcmp(data_reader->start(), "\x11\x22\x33\x44", 4) == 0);
 
-  SECTION("Have flags (invalid)")
-  {
-    uint8_t buf1[] = {
-      0x00,                   // Type
-      0x04,                   // Length
-      0x11, 0x22, 0x33, 0x44, // Payload
-    };
-    std::shared_ptr<const Http3Frame> frame1 = Http3FrameFactory::create(buf1, 
sizeof(buf1));
-    CHECK(frame1->type() == Http3FrameType::DATA);
-    CHECK(frame1->length() == 4);
-
-    std::shared_ptr<const Http3DataFrame> data_frame = 
std::dynamic_pointer_cast<const Http3DataFrame>(frame1);
-    CHECK(data_frame);
-    CHECK(data_frame->payload_length() == 4);
-    CHECK(memcmp(data_frame->payload(), "\x11\x22\x33\x44", 4) == 0);
+    free_MIOBuffer(input);
   }
 }
 
@@ -144,16 +135,22 @@ TEST_CASE("Load SETTINGS Frame", "[http3]")
       0x4a, 0xba, // Identifier
       0x00,       // Value
     };
+    MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+    input->write(buf, sizeof(buf));
+    IOBufferReader *input_reader = input->alloc_reader();
 
-    std::shared_ptr<const Http3Frame> frame = Http3FrameFactory::create(buf, 
sizeof(buf));
+    std::shared_ptr<Http3Frame> frame = 
Http3FrameFactory::create(*input_reader);
+    frame->update();
     CHECK(frame->type() == Http3FrameType::SETTINGS);
     CHECK(frame->length() == sizeof(buf) - 2);
 
-    std::shared_ptr<const Http3SettingsFrame> settings_frame = 
std::dynamic_pointer_cast<const Http3SettingsFrame>(frame);
+    std::shared_ptr<Http3SettingsFrame> settings_frame = 
std::dynamic_pointer_cast<Http3SettingsFrame>(frame);
     CHECK(settings_frame);
     CHECK(settings_frame->is_valid());
     CHECK(settings_frame->get(Http3SettingsId::MAX_FIELD_SECTION_SIZE) == 
0x0400);
     CHECK(settings_frame->get(Http3SettingsId::NUM_PLACEHOLDERS) == 0x0f);
+
+    free_MIOBuffer(input);
   }
 }
 
@@ -219,10 +216,14 @@ TEST_CASE("Http3FrameFactory Create Unknown Frame", 
"[http3]")
     0x0f, // Type
     0x00, // Length
   };
-  std::shared_ptr<const Http3Frame> frame1 = Http3FrameFactory::create(buf1, 
sizeof(buf1));
+  MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+  input->write(buf1, sizeof(buf1));
+  IOBufferReader                   *input_reader = input->alloc_reader();
+  std::shared_ptr<const Http3Frame> frame1       = 
Http3FrameFactory::create(*input_reader);
   CHECK(frame1);
   CHECK(frame1->type() == Http3FrameType::UNKNOWN);
   CHECK(frame1->length() == 0);
+  free_MIOBuffer(input);
 }
 
 TEST_CASE("Http3FrameFactory Fast Create Frame", "[http3]")
@@ -239,21 +240,31 @@ TEST_CASE("Http3FrameFactory Fast Create Frame", 
"[http3]")
     0x04,                   // Length
     0xaa, 0xbb, 0xcc, 0xdd, // Payload
   };
-  std::shared_ptr<const Http3Frame> frame1 = factory.fast_create(buf1, 
sizeof(buf1));
+  MIOBuffer *input1 = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+  input1->write(buf1, sizeof(buf1));
+  IOBufferReader *input_reader1 = input1->alloc_reader();
+  MIOBuffer      *input2        = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+  input2->write(buf2, sizeof(buf2));
+  IOBufferReader *input_reader2 = input2->alloc_reader();
+
+  std::shared_ptr<const Http3Frame> frame1 = 
factory.fast_create(*input_reader1);
   CHECK(frame1 != nullptr);
 
   std::shared_ptr<const Http3DataFrame> data_frame1 = 
std::dynamic_pointer_cast<const Http3DataFrame>(frame1);
   CHECK(data_frame1 != nullptr);
-  CHECK(memcmp(data_frame1->payload(), buf1 + 2, 4) == 0);
+  CHECK(memcmp(data_frame1->data()->start(), buf1 + 2, 4) == 0);
 
-  std::shared_ptr<const Http3Frame> frame2 = factory.fast_create(buf2, 
sizeof(buf2));
+  std::shared_ptr<const Http3Frame> frame2 = 
factory.fast_create(*input_reader2);
   CHECK(frame2 != nullptr);
 
   std::shared_ptr<const Http3DataFrame> data_frame2 = 
std::dynamic_pointer_cast<const Http3DataFrame>(frame2);
   CHECK(data_frame2 != nullptr);
-  CHECK(memcmp(data_frame2->payload(), buf2 + 2, 4) == 0);
+  CHECK(memcmp(data_frame2->data()->start(), buf2 + 2, 4) == 0);
 
   CHECK(frame1 == frame2);
+
+  free_MIOBuffer(input1);
+  free_MIOBuffer(input2);
 }
 
 TEST_CASE("Http3FrameFactory Fast Create Unknown Frame", "[http3]")
@@ -263,14 +274,18 @@ TEST_CASE("Http3FrameFactory Fast Create Unknown Frame", 
"[http3]")
   uint8_t buf1[] = {
     0x0f, // Type
   };
-  std::shared_ptr<const Http3Frame> frame1 = factory.fast_create(buf1, 
sizeof(buf1));
+  MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+  input->write(buf1, sizeof(buf1));
+  IOBufferReader                   *input_reader = input->alloc_reader();
+  std::shared_ptr<const Http3Frame> frame1       = 
factory.fast_create(*input_reader);
   CHECK(frame1);
   CHECK(frame1->type() == Http3FrameType::UNKNOWN);
+  free_MIOBuffer(input);
 }
 
 TEST_CASE("SETTINGS frame handler", "[http3]")
 {
-  uint8_t input[] = {
+  uint8_t buf1[] = {
     0x04,       // Type
     0x08,       // Length
     0x06,       // Identifier
@@ -280,11 +295,15 @@ TEST_CASE("SETTINGS frame handler", "[http3]")
     0x4a, 0x0a, // Identifier
     0x00,       // Value
   };
+  MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+  input->write(buf1, sizeof(buf1));
+  IOBufferReader *input_reader = input->alloc_reader();
 
   Http3SettingsHandler handler       = Http3SettingsHandler(nullptr);
-  Http3SettingsFrame   invalid_frame = Http3SettingsFrame(input, 
sizeof(input), 1);
+  Http3SettingsFrame   invalid_frame = Http3SettingsFrame(*input_reader, 1);
   Http3ErrorUPtr       error         = Http3ErrorUPtr(nullptr);
 
+  invalid_frame.update();
   CHECK(invalid_frame.is_valid() == false);
 
   std::shared_ptr<const Http3Frame> invalid_frame_ptr = std::make_shared<const 
Http3SettingsFrame>(invalid_frame);
@@ -293,11 +312,15 @@ TEST_CASE("SETTINGS frame handler", "[http3]")
   REQUIRE(error);
   CHECK(error->code == Http3ErrorCode::H3_EXCESSIVE_LOAD);
 
-  Http3SettingsFrame valid_frame = Http3SettingsFrame(input, sizeof(input), 3);
+  input->reset();
+  input->write(buf1, sizeof(buf1));
+  Http3SettingsFrame valid_frame = Http3SettingsFrame(*input_reader, 3);
 
   CHECK(valid_frame.is_valid() == true);
 
   std::shared_ptr<const Http3Frame> valid_frame_ptr = std::make_shared<const 
Http3SettingsFrame>(valid_frame);
   error                                             = 
handler.handle_frame(valid_frame_ptr);
   CHECK(error == nullptr);
+
+  free_MIOBuffer(input);
 }

Reply via email to