Reduce H2 header allocations

include/proxy/hdrs/HdrHeap.h

@@ -175,6 +175,7 @@ struct StrHeapDesc {
 class HdrHeap
 {
 public:
+  // NOTE: also sizes an on-stack HEADERS encode buffer (2*this) in HTTP/2; a static_assert bounds the growth.
   static constexpr int DEFAULT_SIZE = 2048;
 
   void init();

include/proxy/hdrs/URL.h

@@ -215,6 +215,9 @@ ParseResult url_parse_http(HdrHeap *heap, URLImpl *url, const char **start, cons
                            bool verify_host_characters);
 ParseResult url_parse_http_regex(HdrHeap *heap, URLImpl *url, const char **start, const char *end, bool copy_strings);
 
+// strict_uri_parsing 0 = no check; 1/2 apply the same test url_parse() does.
+bool url_is_uri_compliant(int strict_uri_parsing, std::string_view value);
+
 char *url_unescapify(Arena *arena, const char *str, int length);
 
 void unescape_str(char *&buf, char *buf_e, const char *&str, const char *str_e, int &state);

include/proxy/http/HttpSM.h

@@ -199,6 +199,20 @@ class HttpSM : public Continuation, public PluginUserArgs<TS_USER_ARGS_TXN>
 
   void attach_client_session(ProxyTransaction *txn);
 
+  // Borrowed: must outlive the copy in state_read_client_request_header. Lets HTTP/2 skip serialize+reparse.
+  void
+  set_pre_parsed_ua_request(HTTPHdr *hdr)
+  {
+    _pre_parsed_ua_request = hdr;
+  }
+
+  // Non-null until state_read_client_request_header copies the borrow; lets send_headers assert synchronous delivery.
+  bool
+  has_pending_pre_parsed_ua_request() const
+  {
+    return _pre_parsed_ua_request != nullptr;
+  }
+
   // Called after the network connection has been completed
   //  to set the session timeouts and initiate a read while
   //  holding the lock for the server session
@@ -627,6 +641,8 @@ class HttpSM : public Continuation, public PluginUserArgs<TS_USER_ARGS_TXN>
 
 private:
   void cancel_pending_server_connection();
+
+  HTTPHdr *_pre_parsed_ua_request = nullptr;
 };
 
 ////

include/proxy/http2/Http2Stream.h

@@ -82,6 +82,8 @@ class Http2Stream : public ProxyTransaction
   void set_expect_receive_trailer() override;
 
   Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size);
+  Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, const uint8_t *block,
+                                      uint32_t block_len);
   void           send_headers(Http2ConnectionState &cstate);
   void           initiating_close();
   bool           is_outbound_connection() const;

src/proxy/hdrs/URL.cc

@@ -1188,6 +1188,22 @@ url_is_mostly_compliant(const char *start, const char *end)
 } // namespace UrlImpl
 using namespace UrlImpl;
 
+bool
+url_is_uri_compliant(int strict_uri_parsing, std::string_view value)
+{
+  const char *start = value.data();
+  const char *end   = start + value.length();
+
+  switch (strict_uri_parsing) {
+  case 1:
+    return url_is_strictly_compliant(start, end);
+  case 2:
+    return url_is_mostly_compliant(start, end);
+  default:
+    return true;
+  }
+}
+
 ParseResult
 url_parse(HdrHeap *heap, URLImpl *url, const char **start, const char *end, bool copy_strings_p, int strict_uri_parsing,
           bool verify_host_characters)

src/proxy/hdrs/VersionConverter.cc

@@ -201,7 +201,16 @@ VersionConverter::_convert_req_from_2_to_1(HTTPHdr &header) const
   if (MIMEField *field = header.field_find(PSEUDO_HEADER_AUTHORITY);
       field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
     auto authority{field->value_get()};
-    header.m_http->u.req.m_url_impl->set_host(header.m_heap, authority, true);
+
+    // Match the reparse so url_print() stays byte-identical (legacy path
+    // unchanged); require full consumption, else a stray '/', '?' or '#' is dropped.
+    const char *astart = authority.data();
+    const char *aend   = authority.data() + authority.length();
+
+    if (url_parse_internet(header.m_heap, header.m_http->u.req.m_url_impl, &astart, aend, true, true) != ParseResult::DONE ||
+        astart != aend) {
+      return ParseResult::ERROR;
+    }
 
     if (!is_connect_method) {
       MIMEField *host = header.field_find(static_cast<std::string_view>(MIME_FIELD_HOST));
@@ -229,14 +238,27 @@ VersionConverter::_convert_req_from_2_to_1(HTTPHdr &header) const
     // :path
     if (MIMEField *field = header.field_find(PSEUDO_HEADER_PATH);
         field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
-      auto path{field->value_get()};
+      auto  path{field->value_get()};
+      auto *url = header.m_http->u.req.m_url_impl;
+
+      // Match the reparse's m_query/m_fragment split so cache/remap see the same
+      // fields; set on delimiter presence even when the value is empty.
+      if (auto hpos = path.find('#'); hpos != std::string_view::npos) {
+        url->set_fragment(header.m_heap, path.substr(hpos + 1), true);
+        path = path.substr(0, hpos);
+      }
+
+      if (auto qpos = path.find('?'); qpos != std::string_view::npos) {
+        url->set_query(header.m_heap, path.substr(qpos + 1), true);
+        path = path.substr(0, qpos);
+      }
 
-      // cut first '/' if there, because `url_print()` add '/' before printing path
-      if (path.starts_with("/"sv)) {
+      // Match the reparse; url_print() re-adds a single leading '/'.
+      while (path.starts_with("/"sv)) {
         path.remove_prefix(1);
       }
 
-      header.m_http->u.req.m_url_impl->set_path(header.m_heap, path, true);
+      url->set_path(header.m_heap, path, true);
 
       header.field_delete(field);
     } else {

src/proxy/http/HttpSM.cc

@@ -607,6 +607,7 @@ HttpSM::state_read_client_request_header(int event, void *data)
   case VC_EVENT_ACTIVE_TIMEOUT:
     // The user agent is hosed.  Close it &
     //   bail on the state machine
+    _pre_parsed_ua_request = nullptr; // the stream is going away; the borrow would dangle
     vc_table.cleanup_entry(_ua.get_entry());
     _ua.set_entry(nullptr);
     set_ua_abort(HttpTransact::ABORTED, event);
@@ -627,10 +628,24 @@ HttpSM::state_read_client_request_header(int event, void *data)
   // tokenize header //
   /////////////////////
 
-  ParseResult state = t_state.hdr_info.client_request.parse_req(&http_parser, _ua.get_txn()->get_remote_reader(), &bytes_used,
-                                                                _ua.get_entry()->eos, t_state.http_config_param->strict_uri_parsing,
-                                                                t_state.http_config_param->http_request_line_max_size,
-                                                                t_state.http_config_param->http_hdr_field_max_size);
+  ParseResult state;
+
+  if (_pre_parsed_ua_request != nullptr) {
+    // The UA_FIRST_READ gate above never fires on this path: the read buffer stays empty.
+    if (milestones[TS_MILESTONE_UA_FIRST_READ] == 0) {
+      ATS_PROBE1(milestone_ua_first_read, sm_id);
+      milestones[TS_MILESTONE_UA_FIRST_READ] = ink_get_hrtime();
+    }
+    t_state.hdr_info.client_request.copy(_pre_parsed_ua_request);
+    _pre_parsed_ua_request = nullptr;
+    bytes_used             = t_state.hdr_info.client_request.length_get();
+    state                  = ParseResult::DONE;
+  } else {
+    state = t_state.hdr_info.client_request.parse_req(&http_parser, _ua.get_txn()->get_remote_reader(), &bytes_used,
+                                                      _ua.get_entry()->eos, t_state.http_config_param->strict_uri_parsing,
+                                                      t_state.http_config_param->http_request_line_max_size,
+                                                      t_state.http_config_param->http_hdr_field_max_size);
+  }
 
   client_request_hdr_bytes += bytes_used;
 

src/proxy/http2/Http2ConnectionState.cc

@@ -452,11 +452,22 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame)
   if (stream->trailing_header_is_possible()) {
     // Don't leak the header_blocks from the initial, non-trailing headers.
     ats_free(stream->header_blocks);
+    stream->header_blocks = nullptr;
   }
-  stream->header_blocks = static_cast<uint8_t *>(ats_malloc(header_block_fragment_length));
-  frame.reader()->memcpy(stream->header_blocks, header_block_fragment_length, header_block_fragment_offset);
 
-  if (frame.header().flags & HTTP2_FLAGS_HEADERS_END_HEADERS) {
+  // Avoids the per-request malloc+memcpy+free of the header block.
+  bool const     end_headers   = frame.header().flags & HTTP2_FLAGS_HEADERS_END_HEADERS;
+  uint8_t const *inplace_block = nullptr;
+
+  if (end_headers && frame.reader()->block_read_avail() >=
+                       static_cast<int64_t>(header_block_fragment_offset) + static_cast<int64_t>(header_block_fragment_length)) {
+    inplace_block = reinterpret_cast<uint8_t const *>(frame.reader()->start()) + header_block_fragment_offset;
+  } else {
+    stream->header_blocks = static_cast<uint8_t *>(ats_malloc(header_block_fragment_length));
+    frame.reader()->memcpy(stream->header_blocks, header_block_fragment_length, header_block_fragment_offset);
+  }
+
+  if (end_headers) {
     // NOTE: If there are END_HEADERS flag, decode stored Header Blocks.
     if (!stream->change_state(HTTP2_FRAME_TYPE_HEADERS, frame.header().flags)) {
       return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
@@ -475,8 +486,12 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame)
     } else {
       stream->mark_milestone(Http2StreamMilestone::START_DECODE_HEADERS);
     }
-    Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle,
-                                                         this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE));
+    Http2ErrorCode result = inplace_block ?
+                              stream->decode_header_blocks(*this->local_hpack_handle,
+                                                           this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE),
+                                                           inplace_block, header_block_fragment_length) :
+                              stream->decode_header_blocks(*this->local_hpack_handle,
+                                                           this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE));
 
     // If this was an outbound connection and the state was already closed, just clear the
     // headers after processing.  We just processed the header blocks to keep the dynamic table in
@@ -2472,9 +2487,11 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
     http2_convert_header_from_1_1_to_2(send_hdr);
   }
 
-  uint32_t        buf_len = send_hdr->length_get() * 2; // Make it double just in case
-  ts::LocalBuffer local_buffer(buf_len);
-  uint8_t        *buf = local_buffer.data();
+  uint32_t buf_len = send_hdr->length_get() * 2; // Make it double just in case
+  // Keep typical header sets off the heap; oversized ones spill to it.
+  static_assert(HdrHeap::DEFAULT_SIZE * 2 <= 8192, "keep HEADERS encode stack buffer within the event-thread stack budget");
+  ts::LocalBuffer<uint8_t, HdrHeap::DEFAULT_SIZE * 2> local_buffer(buf_len);
+  uint8_t                                            *buf = local_buffer.data();
 
   stream->mark_milestone(Http2StreamMilestone::START_ENCODE_HEADERS);
   Http2ErrorCode result = http2_encode_header_blocks(send_hdr, buf, buf_len, &header_blocks_size, *(this->peer_hpack_handle),

src/proxy/http2/Http2Stream.cc

@@ -283,10 +283,16 @@ Http2Stream::main_event_handler(int event, void *edata)
 
 Http2ErrorCode
 Http2Stream::decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size)
+{
+  return this->decode_header_blocks(hpack_handle, maximum_table_size, header_blocks, header_blocks_length);
+}
+
+Http2ErrorCode
+Http2Stream::decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, const uint8_t *block, uint32_t block_len)
 {
   Http2ErrorCode error =
-    http2_decode_header_blocks(&_receive_header, (const uint8_t *)header_blocks, header_blocks_length, nullptr, hpack_handle,
-                               _trailing_header_is_possible, maximum_table_size, this->is_outbound_connection());
+    http2_decode_header_blocks(&_receive_header, block, block_len, nullptr, hpack_handle, _trailing_header_is_possible,
+                               maximum_table_size, this->is_outbound_connection());
   if (error != Http2ErrorCode::HTTP2_ERROR_NO_ERROR) {
     Http2StreamDebug("Error decoding header blocks: %u", static_cast<uint32_t>(error));
   }
@@ -303,10 +309,13 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate ATS_UNUSED */)
 
   // Convert header to HTTP/1.1 format. Trailing headers need no conversion
   // because they, by definition, do not contain pseudo headers.
+  bool conversion_ok = true;
+
   if (this->trailing_header_is_possible()) {
     Http2StreamDebug("trailing header: Skipping send_headers initialization.");
   } else {
     if (http2_convert_header_from_2_to_1_1(&_receive_header) == ParseResult::ERROR) {
+      conversion_ok = false;
       Http2StreamDebug("Error converting HTTP/2 headers to HTTP/1.1.");
       if (_receive_header.type_get() == HTTPType::REQUEST) {
         // There's no way to cause Bad Request directly at this time.
@@ -326,6 +335,37 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate ATS_UNUSED */)
     this->_http_sm_id = this->_sm->sm_id;
   }
 
+  // The fast path skips parse_req, so re-apply strict_uri_parsing on the target.
+  // Evaluated last so path_get() (asserts REQUEST polarity) only sees a REQUEST.
+  auto uri_ok = [&]() {
+    int const level = this->_sm->t_state.http_config_param->strict_uri_parsing;
+
+    return level == 0 ||
+           (url_is_uri_compliant(level, _receive_header.path_get()) && url_is_uri_compliant(level, _receive_header.query_get()) &&
+            url_is_uri_compliant(level, _receive_header.fragment_get()));
+  };
+
+  // The conversion_ok gate keeps a \xffVOID method on the serialize path, where parse_req 400s it.
+  if (conversion_ok && !this->trailing_header_is_possible() && !this->is_outbound_connection() &&
+      _receive_header.type_get() == HTTPType::REQUEST && this->_sm != nullptr && this->read_vio.nbytes > 0 && uri_ok()) {
+    this->_sm->set_pre_parsed_ua_request(&_receive_header);
+    if (this->receive_end_stream) {
+      this->read_vio.nbytes = this->data_length;
+      this->read_vio.ndone  = this->read_vio.nbytes;
+      this->signal_read_event(VC_EVENT_READ_COMPLETE);
+    } else {
+      this->has_body = true;
+      this->signal_read_event(VC_EVENT_READ_READY);
+    }
+    // Delivery is synchronous (stream mutex): the borrow is copied, or the txn
+    // finished and _sm is null. A still-pending borrow is unreachable; recover anyway.
+    if (this->_sm == nullptr || !this->_sm->has_pending_pre_parsed_ua_request()) {
+      return;
+    }
+    ink_assert(!"pre-parsed handoff was deferred");
+    this->_sm->set_pre_parsed_ua_request(nullptr);
+  }
+
   // Write header to a buffer.  Borrowing logic from HttpSM::write_header_into_buffer.
   // Seems like a function like this ought to be in HTTPHdr directly
   int bufindex;

tests/gold_tests/h2/h2_request_handling.test.py

@@ -0,0 +1,25 @@
+'''
+Verify HTTP/2 client request handling through the fast-path handoff to HttpSM.
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+Test.Summary = '''
+Verify HTTP/2 client request handling through the fast-path handoff to HttpSM:
+URL normalization (explicit-port and IPv6 :authority) and query-string caching.
+'''
+
+Test.ATSReplayTest(replay_file="replay/h2_request_handling.replay.yaml")