From 3b98437e3cda1581072ce4745edba3ea8cf2f470 Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Fri, 10 Jul 2026 22:07:46 -0600 Subject: [PATCH 1/5] Reduce H2 header allocations Decode HPACK header blocks in place when the whole block is present and contiguous in the frame reader, avoiding a per-request malloc + memcpy + free. Encode HEADERS frames into a stack-backed LocalBuffer (up to 2 * HdrHeap::DEFAULT_SIZE) so typical response headers skip a heap allocation, with a static_assert guarding the stack budget. --- include/proxy/hdrs/HdrHeap.h | 1 + include/proxy/http2/Http2Stream.h | 2 ++ src/proxy/http2/Http2ConnectionState.cc | 33 +++++++++++++++++++------ src/proxy/http2/Http2Stream.cc | 13 +++++++--- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/include/proxy/hdrs/HdrHeap.h b/include/proxy/hdrs/HdrHeap.h index 75a3620a070..4f8dff673d4 100644 --- a/include/proxy/hdrs/HdrHeap.h +++ b/include/proxy/hdrs/HdrHeap.h @@ -175,6 +175,7 @@ struct StrHeapDesc { class HdrHeap { public: + // Also sizes HTTP/2's on-stack HEADERS encode buffer (2*this). static constexpr int DEFAULT_SIZE = 2048; void init(); diff --git a/include/proxy/http2/Http2Stream.h b/include/proxy/http2/Http2Stream.h index 3fb388670c6..ecf2ea17861 100644 --- a/include/proxy/http2/Http2Stream.h +++ b/include/proxy/http2/Http2Stream.h @@ -83,6 +83,8 @@ class Http2Stream : public ProxyTransaction void set_expect_receive_trailer() override; Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, uint32_t header_field_max_size); + Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, uint32_t header_field_max_size, + const uint8_t *block, uint32_t block_len); void send_headers(Http2ConnectionState &cstate); void initiating_close(); bool is_outbound_connection() const; diff --git a/src/proxy/http2/Http2ConnectionState.cc b/src/proxy/http2/Http2ConnectionState.cc index 446dfc4dbdd..89758424660 100644 --- a/src/proxy/http2/Http2ConnectionState.cc +++ b/src/proxy/http2/Http2ConnectionState.cc @@ -455,11 +455,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(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) { + // In-place decode avoids the per-request malloc+memcpy+free; needs one contiguous 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(header_block_fragment_offset) + static_cast(header_block_fragment_length)) { + inplace_block = reinterpret_cast(frame.reader()->start()) + header_block_fragment_offset; + } else { + stream->header_blocks = static_cast(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, @@ -478,8 +489,13 @@ 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), _header_field_max_size); + Http2ErrorCode result = inplace_block ? + stream->decode_header_blocks(*this->local_hpack_handle, + this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE), + _header_field_max_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), + _header_field_max_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 @@ -2514,9 +2530,10 @@ 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 + static_assert(HdrHeap::DEFAULT_SIZE * 2 <= 8192, "keep HEADERS encode stack buffer within the event-thread stack budget"); + ts::LocalBuffer 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), diff --git a/src/proxy/http2/Http2Stream.cc b/src/proxy/http2/Http2Stream.cc index 6be67db03b9..e083c510911 100644 --- a/src/proxy/http2/Http2Stream.cc +++ b/src/proxy/http2/Http2Stream.cc @@ -286,9 +286,16 @@ Http2Stream::main_event_handler(int event, void *edata) Http2ErrorCode Http2Stream::decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, uint32_t header_field_max_size) { - 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, - header_field_max_size, this->is_outbound_connection()); + return this->decode_header_blocks(hpack_handle, maximum_table_size, header_field_max_size, header_blocks, header_blocks_length); +} + +Http2ErrorCode +Http2Stream::decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, uint32_t header_field_max_size, + const uint8_t *block, uint32_t block_len) +{ + Http2ErrorCode error = + http2_decode_header_blocks(&_receive_header, block, block_len, nullptr, hpack_handle, _trailing_header_is_possible, + maximum_table_size, header_field_max_size, this->is_outbound_connection()); if (error != Http2ErrorCode::HTTP2_ERROR_NO_ERROR) { Http2StreamDebug("Error decoding header blocks: %u", static_cast(error)); } From 2c71ebe2e8a1c96ee22d1dc12ee681b0a41126a7 Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Wed, 22 Jul 2026 11:28:21 -0600 Subject: [PATCH 2/5] Hand HTTP/2 request headers to HttpSM without a reparse The H2 read path serialized each decoded request header so HttpSM could reparse it. Instead, normalize the URL in the 2->1.1 converter (split host:port and path?query, as a reparse would) and hand the decoded header to HttpSM via a refcounted copy(), skipping serialize+reparse. Roughly doubles small-request throughput. The fast path requires a successful 2->1.1 conversion (so a malformed request still gets its 400) and re-checks strict_uri_parsing on the target; non-compliant requests fall back to the serialize path. Header size stays bounded by the aggregate limits still in force here (SETTINGS_MAX_HEADER_LIST_SIZE and request_header_max_size); parse_req's per-field and request-line sub-limits are not separately applied. --- include/proxy/hdrs/URL.h | 3 +++ include/proxy/http/HttpSM.h | 16 ++++++++++++++ src/proxy/hdrs/URL.cc | 16 ++++++++++++++ src/proxy/hdrs/VersionConverter.cc | 30 ++++++++++++++++++++----- src/proxy/http/HttpSM.cc | 23 ++++++++++++++++---- src/proxy/http2/Http2Stream.cc | 35 ++++++++++++++++++++++++++++++ 6 files changed, 114 insertions(+), 9 deletions(-) diff --git a/include/proxy/hdrs/URL.h b/include/proxy/hdrs/URL.h index e8aeee0978d..8bc9603e632 100644 --- a/include/proxy/hdrs/URL.h +++ b/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); diff --git a/include/proxy/http/HttpSM.h b/include/proxy/http/HttpSM.h index c2128eeca20..1bce6dd7cce 100644 --- a/include/proxy/http/HttpSM.h +++ b/include/proxy/http/HttpSM.h @@ -205,6 +205,20 @@ class HttpSM : public Continuation, public PluginUserArgs 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 @@ -633,6 +647,8 @@ class HttpSM : public Continuation, public PluginUserArgs private: void cancel_pending_server_connection(); + + HTTPHdr *_pre_parsed_ua_request = nullptr; }; //// diff --git a/src/proxy/hdrs/URL.cc b/src/proxy/hdrs/URL.cc index b0d0e5329b3..d7eb2ebc77e 100644 --- a/src/proxy/hdrs/URL.cc +++ b/src/proxy/hdrs/URL.cc @@ -1212,6 +1212,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) diff --git a/src/proxy/hdrs/VersionConverter.cc b/src/proxy/hdrs/VersionConverter.cc index 02196ddc279..c64b488cb2a 100644 --- a/src/proxy/hdrs/VersionConverter.cc +++ b/src/proxy/hdrs/VersionConverter.cc @@ -201,7 +201,15 @@ 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 url_parse_http(): 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(MIME_FIELD_HOST)); @@ -229,14 +237,26 @@ 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; + + // Split as url_parse_http() would, so cache keys and remap see the same fields. + 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)) { + // url_parse_http() strips every leading '/'; url_print() re-adds exactly one. + 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 { diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 76727252536..c0cda412ae1 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -600,6 +600,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); @@ -620,10 +621,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) { + // UA_FIRST_READ never fires here: 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; diff --git a/src/proxy/http2/Http2Stream.cc b/src/proxy/http2/Http2Stream.cc index e083c510911..a435262e9f3 100644 --- a/src/proxy/http2/Http2Stream.cc +++ b/src/proxy/http2/Http2Stream.cc @@ -312,10 +312,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. @@ -335,6 +338,38 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate ATS_UNUSED */) this->_http_sm_id = this->_sm->sm_id; } + // parse_req is skipped here; re-apply strict_uri_parsing. + // 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())); + }; + + // A failed conversion leaves a \xffVOID method that only parse_req can turn into a 400. + 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) { + // nbytes == 0 reads as "paused" to the VIO layer, which swallows the signal. + this->read_vio.nbytes = this->data_length + _receive_header.length_get(); + 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; From f92948c83f423f6783124668c33be0070173755d Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Wed, 22 Jul 2026 15:56:42 -0600 Subject: [PATCH 3/5] Add autest for HTTP/2 header handling Proxy Verifier replay coverage for the HTTP/2 <-> HttpSM fast-path handoff in all three directions: inbound request URL normalization (explicit-port and IPv6 :authority) and query-string / cross-protocol cache-key parity; client response emission (bodyless 204/304/HEAD and header preservation); and outbound server-request handling to an HTTP/2 origin (GET with query, POST with body). Extend http2_txn_start_read_gate with a bodyless GET. Its only transaction was a POST, so a dropped read event still had DATA frames to recover on, leaving the END_STREAM-on-HEADERS case uncovered. Also fix http2 test case 8, which piped curl's stderr with the bash 4 "|&" operator. Autest runs commands through /bin/sh, which is bash 3.2 on macOS, so that run died with a syntax error before reaching ATS. --- .../h2/h2_outbound_request_handling.test.py | 27 ++ .../gold_tests/h2/h2_request_handling.test.py | 26 ++ tests/gold_tests/h2/http2.test.py | 5 +- tests/gold_tests/h2/http2_fc_iso.test.py | 348 ++++++++++++++++++ .../h2/http2_txn_start_read_gate.test.py | 3 + .../h2_outbound_request_handling.replay.yaml | 141 +++++++ .../h2/replay/h2_request_handling.replay.yaml | 322 ++++++++++++++++ .../http2_txn_start_read_gate.replay.yaml | 32 ++ 8 files changed, 902 insertions(+), 2 deletions(-) create mode 100644 tests/gold_tests/h2/h2_outbound_request_handling.test.py create mode 100644 tests/gold_tests/h2/h2_request_handling.test.py create mode 100644 tests/gold_tests/h2/http2_fc_iso.test.py create mode 100644 tests/gold_tests/h2/replay/h2_outbound_request_handling.replay.yaml create mode 100644 tests/gold_tests/h2/replay/h2_request_handling.replay.yaml diff --git a/tests/gold_tests/h2/h2_outbound_request_handling.test.py b/tests/gold_tests/h2/h2_outbound_request_handling.test.py new file mode 100644 index 00000000000..2d544a8e372 --- /dev/null +++ b/tests/gold_tests/h2/h2_outbound_request_handling.test.py @@ -0,0 +1,27 @@ +''' +Verify HTTP/2 outbound (SM -> origin) server-request handling: the request +header is handed to the outbound stream without a serialize+reparse. +''' +# 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 outbound server-request handling: the server request header is +handed to the outbound Http2Stream without a serialize+reparse, and reaches an +HTTP/2 origin byte-correct (GET with query, POST with body). +''' + +Test.ATSReplayTest(replay_file="replay/h2_outbound_request_handling.replay.yaml") diff --git a/tests/gold_tests/h2/h2_request_handling.test.py b/tests/gold_tests/h2/h2_request_handling.test.py new file mode 100644 index 00000000000..a945117264d --- /dev/null +++ b/tests/gold_tests/h2/h2_request_handling.test.py @@ -0,0 +1,26 @@ +''' +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 header handling through the fast-path handoff to HttpSM: request +URL normalization (explicit-port and IPv6 :authority), query-string caching, +and response emission (bodyless 204/304/HEAD and response-header preservation). +''' + +Test.ATSReplayTest(replay_file="replay/h2_request_handling.replay.yaml") diff --git a/tests/gold_tests/h2/http2.test.py b/tests/gold_tests/h2/http2.test.py index 7907344cbda..edae0ad48db 100644 --- a/tests/gold_tests/h2/http2.test.py +++ b/tests/gold_tests/h2/http2.test.py @@ -252,8 +252,9 @@ tr = Test.AddTestRun("huge response header") # Different versions of curl have "bytes data" at various places in the output. # Normalize them by simply filtering out those lines since they are not -# important to this test. -tr.MakeCurlCommand(f'-vs -k --http2 https://127.0.0.1:{ts.Variables.ssl_port}/huge_resp_hdrs |& grep -v "bytes data"', ts=ts) +# important to this test. Use "2>&1 |" rather than "|&": autest runs commands +# through /bin/sh, which is bash 3.2 on macOS and predates "|&". +tr.MakeCurlCommand(f'-vs -k --http2 https://127.0.0.1:{ts.Variables.ssl_port}/huge_resp_hdrs 2>&1 | grep -v "bytes data"', ts=ts) tr.Processes.Default.ReturnCode = 0 # Different versions of curl will have different cases for HTTP/2 field names. tr.Processes.Default.Streams.stdout = Testers.GoldFile("gold/http2_8_stdout.gold", case_insensitive=True) diff --git a/tests/gold_tests/h2/http2_fc_iso.test.py b/tests/gold_tests/h2/http2_fc_iso.test.py new file mode 100644 index 00000000000..d70c7fa228d --- /dev/null +++ b/tests/gold_tests/h2/http2_fc_iso.test.py @@ -0,0 +1,348 @@ +"""Isolation repro: HTTP/2 flow control, policy 0, 500-byte window.""" + +# 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. + +import re +from enum import Enum +from typing import List, Optional + +Test.Summary = __doc__ + + +class Http2FlowControlTest: + """Define an object to test HTTP/2 flow control behavior.""" + + _replay_file: str = 'http2_flow_control.replay.yaml' + _replay_chunked_file: str = 'http2_flow_control_chunked.replay.yaml' + _valid_policy_values: List[int] = list(range(0, 3)) + _flow_control_policy: Optional[int] = None + _flow_control_policy_is_malformed: bool = False + + _default_initial_window_size: int = 65535 + _default_max_concurrent_streams: int = 100 + _default_flow_control_policy: int = 0 + + _dns_counter: int = 0 + _server_counter: int = 0 + _ts_counter: int = 0 + _client_counter: int = 0 + + IS_OUTBOUND = True + IS_INBOUND = False + + IS_HTTP2_TO_ORIGIN = True + IS_HTTP1_TO_ORIGIN = False + + class ServerType(Enum): + """Define the type of server to use in a TestRun.""" + + HTTP1_CONTENT_LENGTH = 0 + HTTP1_CHUNKED = 1 + HTTP2 = 2 + + def __init__( + self, + description: str, + initial_window_size: Optional[int] = None, + max_concurrent_streams: Optional[int] = None, + flow_control_policy: Optional[int] = None): + """Declare the various test Processes. + + :param description: A description of the test. + + :param initial_window_size: The value with which to configure the + proxy.config.http2.initial_window_size_(in|out) ATS parameter in the + records.yaml file. If the paramenter is None, then no window size + will be explicitly set and ATS will use the default value. + + :param max_concurrent_streams: The value with which to configure the + proxy.config.http2.max_concurrent_streams_(in|out) ATS parameter in the + records.yaml file. If the paramenter is None, then no window size + will be explicitly set and ATS will use the default value. + + :param flow_control_policy: The value with which to configure the + proxy.config.http2.flow_control.policy_(in|out) ATS parameter the + records.yaml file. If the paramenter is None, then no policy + configuration will be explicitly set and ATS will use the default + value. + """ + self._description = description + + self._initial_window_size = initial_window_size + self._expected_initial_stream_window_size = ( + initial_window_size if initial_window_size is not None else self._default_initial_window_size) + + self._max_concurrent_streams = max_concurrent_streams + self._expected_max_concurrent_streams = ( + max_concurrent_streams if max_concurrent_streams is not None else self._default_max_concurrent_streams) + + self._flow_control_policy = flow_control_policy + self._expected_flow_control_policy = ( + flow_control_policy if flow_control_policy is not None else self._default_flow_control_policy) + + self._flow_control_policy_is_malformed = ( + self._flow_control_policy is not None and self._flow_control_policy not in self._valid_policy_values) + + def _configure_dns(self, tr: 'TestRun') -> 'Process': + """Configure the DNS.""" + dns = tr.MakeDNServer(f'dns-{Http2FlowControlTest._dns_counter}') + Http2FlowControlTest._dns_counter += 1 + return dns + + def _configure_server(self, tr: 'TestRun', server_type: ServerType) -> Optional['Process']: + """Configure the test server.""" + if self._flow_control_policy_is_malformed: + return None + if server_type == self.ServerType.HTTP1_CHUNKED: + replay_file = self._replay_chunked_file + else: + replay_file = self._replay_file + + server = tr.AddVerifierServerProcess(f'server-{Http2FlowControlTest._server_counter}', replay_file) + Http2FlowControlTest._server_counter += 1 + return server + + def _configure_trafficserver(self, tr: 'TestRun', is_outbound: bool, server_type: ServerType) -> 'Process': + """Configure a Traffic Server process.""" + ts = tr.MakeATSProcess(f'ts-{Http2FlowControlTest._ts_counter}', enable_tls=True) + Http2FlowControlTest._ts_counter += 1 + + ts.addDefaultSSLFiles() + ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + 'proxy.config.dns.nameservers': '127.0.0.1:{0}'.format(self._dns.Variables.Port), + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.http.insert_age_in_response': 0, + 'proxy.config.diags.debug.enabled': 3, + 'proxy.config.diags.debug.tags': 'http', + }) + + if server_type == self.ServerType.HTTP2: + ts.Disk.records_config.update({ + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + }) + + if self._initial_window_size is not None: + if is_outbound: + configuration = 'proxy.config.http2.initial_window_size_out' + else: + configuration = 'proxy.config.http2.initial_window_size_in' + ts.Disk.records_config.update({ + configuration: self._initial_window_size, + }) + + if self._flow_control_policy is not None: + if is_outbound: + configuration = 'proxy.config.http2.flow_control.policy_out' + else: + configuration = 'proxy.config.http2.flow_control.policy_in' + ts.Disk.records_config.update({ + configuration: self._flow_control_policy, + }) + + if self._max_concurrent_streams is not None: + if is_outbound: + configuration = 'proxy.config.http2.max_concurrent_streams_out' + else: + configuration = 'proxy.config.http2.max_concurrent_streams_in' + ts.Disk.records_config.update({ + configuration: self._max_concurrent_streams, + }) + + ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + + if self._server is not None: + ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{self._server.Variables.https_port}') + + if self._flow_control_policy_is_malformed: + if is_outbound: + configuration = 'proxy.config.http2.flow_control.policy_out' + else: + configuration = 'proxy.config.http2.flow_control.policy_in' + ts.Disk.diags_log.Content = Testers.ContainsExpression( + f"ERROR.*{configuration}", "Expected an error about an invalid flow control policy.") + + return ts + + def _configure_client( + self, + tr, + ): + """Configure a client process. + + :param tr: The TestRun to associate the client with. + """ + tr.AddVerifierClientProcess( + f'client-{Http2FlowControlTest._client_counter}', self._replay_file, https_ports=[self._ts.Variables.ssl_port]) + Http2FlowControlTest._client_counter += 1 + + def _configure_log_expectations(self, host): + """Configure the log expectations for the client or server.""" + hostname = "server" if host == self._server else "client" + if self._flow_control_policy_is_malformed: + # Since we're just testing ATS configuration errors, there's no + # need to set up client expectations. + return + + # ATS currently always sends a MAX_CONCURRENT_STREAMS setting. + host.Streams.stdout += Testers.ContainsExpression( + f'MAX_CONCURRENT_STREAMS:{self._expected_max_concurrent_streams}', + f"{hostname} should receive a MAX_CONCURRENT_STREAMS setting.") + + if self._initial_window_size is not None: + host.Streams.stdout += Testers.ContainsExpression( + f'INITIAL_WINDOW_SIZE:{self._expected_initial_stream_window_size}', + f"{hostname} should receive an INITIAL_WINDOW_SIZE setting.") + + if self._expected_flow_control_policy == 0: + update_window_size = (self._expected_initial_stream_window_size - self._default_initial_window_size) + if update_window_size > 0: + host.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id 0: {update_window_size}', f"{hostname} should receive a session WINDOW_UPDATE.") + + if self._expected_flow_control_policy in (1, 2): + # Verify the larger window size. + + session_window_size = (self._expected_initial_stream_window_size * self._expected_max_concurrent_streams) + + # ATS will send a WINDOW_UPDATE frame to the client to increase + # the session window size to the configured value from the default + # value. + update_window_size = (session_window_size - self._expected_initial_stream_window_size) + + # A WINDOW_UPDATE can only increase the window size. So make sure that + # the new window size is greater than the default window size. + if update_window_size > Http2FlowControlTest._default_initial_window_size: + host.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id 0: {update_window_size}', f"{hostname} should receive an initial session WINDOW_UPDATE.") + else: + # Our test traffic is large enough that eventually we should + # send a session WINDOW_UPDATE frame for the smaller window. + # It's not clear what it will be in advance though. A 100 byte + # session window may not receive a 100 byte WINDOW_UPDATE frame + # if the client is sending DATA frames in 10 byte chunks due to + # a smaller stream window. + host.Streams.stdout += Testers.ContainsExpression( + 'WINDOW_UPDATE.*id 0: ', f"{hostname} should receive a session WINDOW_UPDATE.") + + if self._expected_flow_control_policy == 2: + # Verify the streams window sizes get updated. + stream_window_1 = session_window_size + stream_window_2 = int(session_window_size / 2) + stream_window_3 = int(session_window_size / 3) + if self._server: + # Toward the server, there is a potential race condition + # between sending of first-request and the sending of the + # SETTINGS frame which reduces the stream window size. + # Allow for either scenario. + host.Streams.stdout += Testers.ContainsExpression( + (f'INITIAL_WINDOW_SIZE:{stream_window_1}.*' + f'INITIAL_WINDOW_SIZE:{stream_window_2}.*'), + f"{hostname} should stream receive window updates", + reflags=re.DOTALL | re.MULTILINE) + else: + host.Streams.stdout += Testers.ContainsExpression( + ( + f'INITIAL_WINDOW_SIZE:{stream_window_1}.*' + f'INITIAL_WINDOW_SIZE:{stream_window_2}.*' + f'INITIAL_WINDOW_SIZE:{stream_window_3}'), + f"{hostname} should stream receive window updates", + reflags=re.DOTALL | re.MULTILINE) + + if self._expected_initial_stream_window_size < 1000: + first_id = 5 if self._server else 3 + + if self._server and self._expected_flow_control_policy == 2: + # Toward the server, there is a potential race condition + # between sending of first-request and the sending of the + # SETTINGS frame which reduces the stream window size. Allow + # for either scenario. + window_update_size = f'33|{self._expected_initial_stream_window_size}' + else: + window_update_size = f'{self._expected_initial_stream_window_size}' + # For the smaller session window sizes, we expect WINDOW_UPDATE frames. + host.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id {first_id}: {window_update_size}', f"{hostname} should receive a stream WINDOW_UPDATE.") + + host.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id {first_id + 2}: {window_update_size}', f"{hostname} should receive a stream WINDOW_UPDATE.") + + host.Streams.stdout += Testers.ContainsExpression( + f'WINDOW_UPDATE.*id {first_id + 4}: {window_update_size}', f"{hostname} should receive a stream WINDOW_UPDATE.") + + def _configure_test_run_common(self, tr, is_outbound: bool, server_type: ServerType) -> None: + """Perform the common Process configuration.""" + self._dns = self._configure_dns(tr) + self._server = self._configure_server(tr, server_type) + self._ts = self._configure_trafficserver(tr, is_outbound, server_type) + if not self._flow_control_policy_is_malformed: + self._configure_client(tr) + tr.Processes.Default.StartBefore(self._dns) + tr.Processes.Default.StartBefore(self._server) + else: + tr.Processes.Default.Command = "true" + tr.Processes.Default.StartBefore(self._ts) + tr.TimeOut = 20 + + def _configure_inbound_http1_to_origin_test_run(self) -> None: + """Configure the TestRun for inbound stream configuration.""" + tr = Test.AddTestRun(f'{self._description} - inbound, ' + 'HTTP/1 Content-Length origin') + self._configure_test_run_common(tr, self.IS_INBOUND, self.ServerType.HTTP1_CONTENT_LENGTH) + self._configure_log_expectations(tr.Processes.Default) + + tr = Test.AddTestRun(f'{self._description} - inbound, ' + 'HTTP/1 chunked origin') + self._configure_test_run_common(tr, self.IS_INBOUND, self.ServerType.HTTP1_CHUNKED) + self._configure_log_expectations(tr.Processes.Default) + + def _configure_inbound_http2_to_origin_test_run(self) -> None: + """Configure the TestRun for inbound stream configuration.""" + tr = Test.AddTestRun(f'{self._description} - inbound, HTTP/2 origin') + self._configure_test_run_common(tr, self.IS_INBOUND, self.ServerType.HTTP2) + self._configure_log_expectations(tr.Processes.Default) + + def _configure_outbound_test_run(self) -> None: + """Configure the TestRun outbound stream configuration.""" + tr = Test.AddTestRun(f'{self._description} - outbound, HTTP/2 origin') + self._configure_test_run_common(tr, self.IS_OUTBOUND, self.ServerType.HTTP2) + self._configure_log_expectations(self._server) + + def run(self) -> None: + """Configure the test run for various origin side configurations.""" + self._configure_inbound_http1_to_origin_test_run() + self._configure_inbound_http2_to_origin_test_run() + self._configure_outbound_test_run() + + +# +# ISOLATION REPRO: only the "policy 0, 500-byte window" configuration. +# +test = Http2FlowControlTest( + description="Flow control policy 0 (default): small initial_window_size", + initial_window_size=500, # The default is 65 KB. + flow_control_policy=0) +test.run() diff --git a/tests/gold_tests/h2/http2_txn_start_read_gate.test.py b/tests/gold_tests/h2/http2_txn_start_read_gate.test.py index 8ad8aea333b..d4ed3697a37 100644 --- a/tests/gold_tests/h2/http2_txn_start_read_gate.test.py +++ b/tests/gold_tests/h2/http2_txn_start_read_gate.test.py @@ -53,6 +53,9 @@ tr.Processes.Default.Streams.All += Testers.ExcludesExpression(r'\[ERROR\]', 'Proxy Verifier should not report errors.') tr.Processes.Default.Streams.All += Testers.ContainsExpression( 'Equals Success: Key: "1", Content Data: "body", Value: "response-body"', 'Client should receive the response body.') +tr.Processes.Default.Streams.All += Testers.ContainsExpression( + 'Equals Success: Key: "2", Content Data: "body", Value: "bodyless-response"', + 'Client should receive the response to a bodyless request.') server.Streams.All += Testers.ContainsExpression( 'Equals Success: Key: "1", Content Data: "body", Value: "request-body"', 'Origin should receive the request body.') diff --git a/tests/gold_tests/h2/replay/h2_outbound_request_handling.replay.yaml b/tests/gold_tests/h2/replay/h2_outbound_request_handling.replay.yaml new file mode 100644 index 00000000000..b8340aede9b --- /dev/null +++ b/tests/gold_tests/h2/replay/h2_outbound_request_handling.replay.yaml @@ -0,0 +1,141 @@ +# 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. + +# Exercises the outbound (SM -> origin over HTTP/2) header handoff: the server +# request header is handed to the outbound Http2Stream without a serialize + +# reparse. proxy-request verification confirms the request ATS built by copying +# (method + URL, pseudos preserved, fields) reaches the origin byte-correct. + +meta: + version: "1.0" + +autest: + description: 'HTTP/2 outbound: server-request direct handoff to an H2 origin' + + server: + name: 'server' + + client: + name: 'client' + + ats: + name: 'ts' + + process_config: + enable_tls: true + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'http|http2' + proxy.config.ssl.client.alpn_protocols: 'h2,http/1.1' + proxy.config.http.server_session_sharing.pool: 'thread' + proxy.config.ssl.client.verify.server.policy: 'PERMISSIVE' + + remap_config: + - from: "https://example.com/" + to: "https://127.0.0.1:{SERVER_HTTPS_PORT}/" + +sessions: +- protocol: + stack: http2 + tls: + sni: example.com + transactions: + + # 1. GET with a query string and a custom header. The outbound copy must set + # method + URL (splitting :path into path + query) and forward the field, so + # the origin's proxy-request sees them exactly. + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", example.com] + - [":path", "/outbound/item?a=1&b=2"] + - ["x-req-custom", "req-alpha"] + - [uuid, ob-get] + proxy-request: + protocol: + stack: http2 + tls: + sni: example.com + proxy-verify-mode: 1 + proxy-provided-cert: false + method: GET + url: + - [ path, { value: /outbound/item, as: equal } ] + - [ query, { value: "a=1&b=2", as: equal } ] + headers: + fields: + - [ "x-req-custom", { value: "req-alpha", as: equal } ] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "8"] + content: + encoding: plain + data: OB-GET-1 + proxy-response: + status: 200 + content: + verify: {value: OB-GET-1, as: equal} + + # 2. POST with a body. The header is direct-passed (0 header bytes on the write), + # then the body streams to the origin over the same H2 stream. + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", POST] + - [":scheme", https] + - [":authority", example.com] + - [":path", /outbound/post] + - [Content-Length, "5"] + - [uuid, ob-post] + - DATA: + content: + encoding: plain + data: HELLO + proxy-request: + protocol: + stack: http2 + tls: + sni: example.com + proxy-verify-mode: 1 + proxy-provided-cert: false + method: POST + url: + - [ path, { value: /outbound/post, as: equal } ] + content: + verify: {value: HELLO, as: equal} + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "9"] + content: + encoding: plain + data: OB-POST-1 + proxy-response: + status: 200 + content: + verify: {value: OB-POST-1, as: equal} diff --git a/tests/gold_tests/h2/replay/h2_request_handling.replay.yaml b/tests/gold_tests/h2/replay/h2_request_handling.replay.yaml new file mode 100644 index 00000000000..94405b7e881 --- /dev/null +++ b/tests/gold_tests/h2/replay/h2_request_handling.replay.yaml @@ -0,0 +1,322 @@ +# 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. + +# Exercises the HTTP/2 <-> HttpSM header handoff (the "fast path"): the decoded +# request header is normalized in the 2->1.1 converter and handed to the SM +# without a serialize+reparse, and the response header is handed back to the +# stream the same way. Covers request URL normalization (explicit-port and IPv6 +# :authority), query-string caching, and response emission (bodyless 204/304/HEAD +# and header preservation). + +meta: + version: "1.0" + +autest: + description: 'HTTP/2 request handling: URL normalization and caching' + + server: + name: 'server' + + client: + name: 'client' + + ats: + name: 'ts' + + process_config: + enable_tls: true + enable_cache: true + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'http|http2|cache' + + remap_config: + - from: "https://example.com/" + to: "http://127.0.0.1:{SERVER_HTTP_PORT}/" + - from: "https://example.com:8443/" + to: "http://127.0.0.1:{SERVER_HTTP_PORT}/" + - from: "https://[::1]:8443/" + to: "http://127.0.0.1:{SERVER_HTTP_PORT}/" + +sessions: +- protocol: + stack: http2 + tls: + sni: example.com + transactions: + + # 1. Query string: cache a query-bearing resource, then re-request it. The + # fast path must split :path into m_path + m_query so the response caches + # and the key is stable. The re-request's origin response is marked unused; + # a cache miss would fetch it and fail the body check. + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", example.com] + - [":path", "/cache/item?a=1&b=2"] + - [uuid, query-fill] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "9"] + - [Cache-Control, max-age=300] + content: + encoding: plain + data: CACHED-Q1 + proxy-response: + status: 200 + + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", example.com] + - [":path", "/cache/item?a=1&b=2"] + - [uuid, query-hit] + server-response: + status: 500 + reason: NOTUSED + proxy-response: + status: 200 + content: + verify: {value: CACHED-Q1, as: equal} + + # 2. Explicit-port :authority. The converter must split host:port so remap + # matches on host example.com, port 8443. If the port stayed in the host + # string the map would not match and this would not be a 200. + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", "example.com:8443"] + - [":path", /port] + - [uuid, explicit-port] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "7"] + content: + encoding: plain + data: PORT-OK + proxy-response: + status: 200 + content: + verify: {value: PORT-OK, as: equal} + + # 3. IPv6 literal :authority. url_parse_internet keeps the brackets; remap + # matches host [::1], port 8443. + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", "[::1]:8443"] + - [":path", /v6] + - [uuid, ipv6-authority] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "5"] + content: + encoding: plain + data: V6-OK + proxy-response: + status: 200 + content: + verify: {value: V6-OK, as: equal} + +# 4. Cross-protocol cache-key parity: fill over HTTP/1.1-over-TLS, then read the +# same https URL over HTTP/2. The keys must match, so the H2 fast path has to +# hash host/path/query exactly as the HTTP/1.1 reparse does. A divergent H2 +# key would miss and fetch the unused 500, failing the body check. +- protocol: + stack: https + tls: + sni: example.com + transactions: + - client-request: + method: GET + version: "1.1" + url: "/parity?x=1&y=2" + headers: + fields: + - [Host, example.com] + - [uuid, parity-fill-h1] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "11"] + - [Cache-Control, max-age=300] + content: + encoding: plain + data: PARITY-BODY + proxy-response: + status: 200 + +- protocol: + stack: http2 + tls: + sni: example.com + transactions: + - client-request: + # Run after the HTTP/1.1 fill above has populated the cache. + delay: 2s + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", example.com] + - [":path", "/parity?x=1&y=2"] + - [uuid, parity-read-h2] + server-response: + status: 500 + reason: NOTUSED + proxy-response: + status: 200 + content: + verify: {value: PARITY-BODY, as: equal} + +# 5. Response direction. The response header is direct-passed to the stream +# (no serialize+reparse) and copied into _send_header. +- protocol: + stack: http2 + tls: + sni: example.com + transactions: + + # 5a. Bodyless response (204). With zero body bytes, do_io_write must still + # emit the HEADERS frame; otherwise the client hangs waiting for a response. + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", example.com] + - [":path", /no-content] + - [uuid, resp-204] + server-response: + status: 204 + reason: No Content + proxy-response: + status: 204 + + # 5b. Response header set survives the direct copy (values and a multi-token + # value must match what a serialize+reparse would have produced). + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", example.com] + - [":path", /resp-headers] + - [uuid, resp-headers] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "5"] + - [X-Custom-One, alpha] + - [X-Custom-Two, "bravo charlie"] + - [Cache-Control, no-store] + content: + encoding: plain + data: HDROK + proxy-response: + status: 200 + headers: + fields: + - [X-Custom-One, {value: alpha, as: equal}] + - [X-Custom-Two, {value: "bravo charlie", as: equal}] + - [Cache-Control, {value: no-store, as: equal}] + content: + verify: {value: HDROK, as: equal} + + # 5c. HEAD: the body is precluded by method, not status. The header still has to + # be emitted with no body bytes (another zero-body do_io_write trigger path). + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", HEAD] + - [":scheme", https] + - [":authority", example.com] + - [":path", /head] + - [uuid, resp-head] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "20"] + - [X-Head-Marker, present] + proxy-response: + status: 200 + headers: + fields: + - [X-Head-Marker, {value: present, as: equal}] + + # 5d. 304 relay: a conditional request the origin answers with a bodyless 304, + # which ATS relays to the client (body precluded by status). + - client-request: + frames: + - HEADERS: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", example.com] + - [":path", /conditional] + - ["if-none-match", '"v1"'] + - [uuid, resp-304] + server-response: + status: 304 + reason: Not Modified + headers: + fields: + - [ETag, '"v1"'] + proxy-response: + status: 304 + headers: + fields: + - [ETag, {value: '"v1"', as: equal}] + diff --git a/tests/gold_tests/h2/replay/http2_txn_start_read_gate.replay.yaml b/tests/gold_tests/h2/replay/http2_txn_start_read_gate.replay.yaml index 6f9bddd24b0..e34917919ef 100644 --- a/tests/gold_tests/h2/replay/http2_txn_start_read_gate.replay.yaml +++ b/tests/gold_tests/h2/replay/http2_txn_start_read_gate.replay.yaml @@ -60,3 +60,35 @@ sessions: encoding: plain data: response-body verify: {as: equal} + + # A bodyless GET arrives with END_STREAM on the HEADERS frame, so there are no + # DATA frames to fall back on if the read event is dropped while TXN_START is + # gated. Regression coverage for the direct-header-passing path. + - client-request: + frames: + - HEADERS: + headers: + fields: + - [:method, GET] + - [:scheme, https] + - [:authority, delay-txn-start.test] + - [:path, /read-gate-no-body] + - [uuid, '2'] + + proxy-request: + url: "/read-gate-no-body" + + server-response: + status: 200 + reason: OK + content: + encoding: plain + data: bodyless-response + size: 17 + + proxy-response: + status: 200 + content: + encoding: plain + data: bodyless-response + verify: {as: equal} From 73871800c3d0277332446b281332c97bbade11d1 Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Thu, 23 Jul 2026 10:53:56 -0600 Subject: [PATCH 4/5] Unify H2 header handoff onto a ProxyTransaction interface Replace the HttpSM::_pre_parsed_ua_request borrow pointer with virtual supports_direct_header_passing(), is_parsed_receive_header_ready() and parsed_receive_header() on ProxyTransaction, and use the same seam for the response and outbound-request directions. All three skip the serialize+reparse round-trip between HttpSM and the HTTP/2 stream. Request in: HttpSM pulls the decoded header from the transaction in state_read_client_request_header rather than the stream pushing a raw pointer. The stream owns _receive_header and is torn down with the SM, so the copy cannot see a dangling borrow; that drops the timeout null-out and the synchronous-delivery assert. Response out / request out: write_response_header_into_buffer and setup_server_send_request hand client_response / server_request to the stream instead of serializing them. update_write_request copies the fields onto _send_header, preserving the pseudo-headers that create(HTTP_2_0) installed and the 1.1->2 conversion fills -- a plain copy() would wipe them. A bodyless message (204/304/HEAD, or a GET to an H2 origin) has no body bytes to drive the write, so it is flushed via has_pending_send_header(). The ready flag re-arms per header so 1xx interim responses and retried requests each deliver. Because the header no longer passes through a buffer, everything that inferred it from one had to be corrected. client_response_hdr_bytes stays 0 on this path, so reported_client_response_hdr_bytes() serves the readers that mean "bytes the client received": logging, TSHttpTxnClientRespHdrBytesGet(), the size stats, and the tunnel_handler_post_ua guard that decides whether a response header has already gone out -- left raw, an early origin response followed by a post-body timeout would synthesize an error over it. Readers doing tunnel byte arithmetic keep the raw counter. The tunnel also identifies the UA consumer by comparing against get_ua_txn() rather than casting on vc_type, which response plugin agents share while carrying an INKVConnInternal. The interface design is adopted from Masakazu Kitajo's no-header-marshaling patch. The VersionConverter URL parity work and the H2 allocation reductions on this branch are unchanged. --- include/proxy/ProxyTransaction.h | 30 +++++++++++ include/proxy/http/HttpSM.h | 58 ++++++++++++++------ include/proxy/http2/Http2Stream.h | 6 +++ src/api/InkAPI.cc | 2 +- src/proxy/http/HttpSM.cc | 36 +++++++------ src/proxy/http/HttpTunnel.cc | 10 ++-- src/proxy/http2/Http2Stream.cc | 72 ++++++++++++++++++++----- src/proxy/logging/TransactionLogData.cc | 2 +- 8 files changed, 165 insertions(+), 51 deletions(-) diff --git a/include/proxy/ProxyTransaction.h b/include/proxy/ProxyTransaction.h index 7665392ec50..48808902de1 100644 --- a/include/proxy/ProxyTransaction.h +++ b/include/proxy/ProxyTransaction.h @@ -61,6 +61,12 @@ class ProxyTransaction : public VConnection virtual bool expect_receive_trailer() const; virtual void set_expect_receive_trailer(); + virtual bool supports_direct_header_passing() const; + virtual bool is_parsed_receive_header_ready() const; + virtual const HTTPHdr *parsed_receive_header() const; + + virtual bool has_pending_send_header() const; + // Implement VConnection interface. VIO *do_io_read(Continuation *c, int64_t nbytes = INT64_MAX, MIOBuffer *buf = nullptr) override; VIO *do_io_write(Continuation *c = nullptr, int64_t nbytes = INT64_MAX, IOBufferReader *buf = nullptr, @@ -320,6 +326,30 @@ ProxyTransaction::cancel_active_timeout() } } +inline bool +ProxyTransaction::supports_direct_header_passing() const +{ + return false; +} + +inline bool +ProxyTransaction::is_parsed_receive_header_ready() const +{ + return false; +} + +inline const HTTPHdr * +ProxyTransaction::parsed_receive_header() const +{ + return nullptr; +} + +inline bool +ProxyTransaction::has_pending_send_header() const +{ + return false; +} + // See if we need to schedule on the primary thread for the transaction or change the thread that is associated with the VC. // If we reschedule, the scheduled action is returned. Otherwise, NULL is returned inline Action * diff --git a/include/proxy/http/HttpSM.h b/include/proxy/http/HttpSM.h index 1bce6dd7cce..cb8770cda23 100644 --- a/include/proxy/http/HttpSM.h +++ b/include/proxy/http/HttpSM.h @@ -205,20 +205,6 @@ class HttpSM : public Continuation, public PluginUserArgs 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 @@ -624,6 +610,12 @@ class HttpSM : public Continuation, public PluginUserArgs IOBufferReader *_netvc_reader = nullptr; MIOBuffer *_netvc_read_buffer = nullptr; + bool _client_response_header_is_ready = false; + bool _server_request_header_is_ready = false; + + // Direct-passed headers bypass the tunnel, so client_response_hdr_bytes stays 0. + int _direct_response_hdr_bytes = 0; + void kill_this(); void update_stats(); void transform_cleanup(TSHttpHookID hook, HttpTransformInfo *info); @@ -640,6 +632,23 @@ class HttpSM : public Continuation, public PluginUserArgs int client_transaction_priority_weight() const; int client_transaction_priority_dependence() const; + HTTPHdr *get_client_response_header(); + HTTPHdr *get_server_request_header(); + + // For logging/SDK: client_response_hdr_bytes counts only what the tunnel wrote. + int + reported_client_response_hdr_bytes() const + { + return client_response_hdr_bytes > 0 ? client_response_hdr_bytes : _direct_response_hdr_bytes; + } + + void + clear_pending_send_header() + { + _client_response_header_is_ready = false; + _server_request_header_is_ready = false; + } + ink_hrtime get_server_inactivity_timeout(); ink_hrtime get_server_active_timeout(); ink_hrtime get_server_connect_timeout(); @@ -647,8 +656,6 @@ class HttpSM : public Continuation, public PluginUserArgs private: void cancel_pending_server_connection(); - - HTTPHdr *_pre_parsed_ua_request = nullptr; }; //// @@ -729,13 +736,30 @@ HttpSM::get_cache_sm() inline int HttpSM::write_response_header_into_buffer(HTTPHdr *h, MIOBuffer *b) { - if (t_state.client_info.http_version == HTTPVersion(0, 9)) { + if (_ua.get_txn()->supports_direct_header_passing()) { + // Nothing lands in the buffer, so 0 keeps the tunnel's byte math honest. + _client_response_header_is_ready = true; + _direct_response_hdr_bytes = h->length_get(); + return 0; + } else if (t_state.client_info.http_version == HTTPVersion(0, 9)) { return 0; } else { return write_header_into_buffer(h, b); } } +inline HTTPHdr * +HttpSM::get_client_response_header() +{ + return _client_response_header_is_ready ? &t_state.hdr_info.client_response : nullptr; +} + +inline HTTPHdr * +HttpSM::get_server_request_header() +{ + return _server_request_header_is_ready ? &t_state.hdr_info.server_request : nullptr; +} + inline int HttpSM::find_server_buffer_size() { diff --git a/include/proxy/http2/Http2Stream.h b/include/proxy/http2/Http2Stream.h index ecf2ea17861..c942a15de2e 100644 --- a/include/proxy/http2/Http2Stream.h +++ b/include/proxy/http2/Http2Stream.h @@ -82,6 +82,11 @@ class Http2Stream : public ProxyTransaction bool expect_receive_trailer() const override; void set_expect_receive_trailer() override; + bool supports_direct_header_passing() const override; + bool is_parsed_receive_header_ready() const override; + const HTTPHdr *parsed_receive_header() const override; + bool has_pending_send_header() const override; + Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, uint32_t header_field_max_size); Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, uint32_t header_field_max_size, const uint8_t *block, uint32_t block_len); @@ -222,6 +227,7 @@ class Http2Stream : public ProxyTransaction int _sent_request_method{-1}; HTTPHdr _receive_header; + bool _is_parsed_receive_header_ready = false; #if TS_USE_MALLOC_ALLOCATOR MIOBuffer _receive_buffer{BUFFER_SIZE_INDEX_FOR_XMALLOC_SIZE(4096)}; #else diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 6d085befbb4..33cfd8c7f0b 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -5565,7 +5565,7 @@ TSHttpTxnClientRespHdrBytesGet(TSHttpTxn txnp) sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS); HttpSM *sm = reinterpret_cast(txnp); - return sm->client_response_hdr_bytes; + return sm->reported_client_response_hdr_bytes(); } int64_t diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index c0cda412ae1..53ea127526b 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -600,7 +600,6 @@ 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); @@ -621,23 +620,22 @@ HttpSM::state_read_client_request_header(int event, void *data) // tokenize header // ///////////////////// - ParseResult state; + ParseResult state; + ProxyTransaction *ua_txn = _ua.get_txn(); - if (_pre_parsed_ua_request != nullptr) { + if (ua_txn->supports_direct_header_passing() && ua_txn->is_parsed_receive_header_ready()) { // UA_FIRST_READ never fires here: 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; + t_state.hdr_info.client_request.copy(ua_txn->parsed_receive_header()); + 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); + state = t_state.hdr_info.client_request.parse_req( + &http_parser, ua_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; @@ -4029,7 +4027,9 @@ HttpSM::tunnel_handler_post_ua(int event, HttpTunnelProducer *p) case VC_EVENT_INACTIVITY_TIMEOUT: case VC_EVENT_ACTIVE_TIMEOUT: case HTTP_TUNNEL_EVENT_PARSE_ERROR: - if (client_response_hdr_bytes == 0) { + // Not the raw counter: it stays 0 when the header bypassed the tunnel, which would + // synthesize an error over a response the client already got. + if (reported_client_response_hdr_bytes() == 0) { p->handler_state = static_cast(HttpSmPost_t::UA_FAIL); set_ua_abort(HttpTransact::ABORTED, event); @@ -7026,7 +7026,13 @@ HttpSM::setup_server_send_request() // We need a reader so bytes don't fall off the end of // the buffer IOBufferReader *buf_start = server_entry->write_buffer->alloc_reader(); - server_request_hdr_bytes = hdr_length = write_header_into_buffer(&t_state.hdr_info.server_request, server_entry->write_buffer); + + if (server_txn->supports_direct_header_passing()) { + _server_request_header_is_ready = true; + server_request_hdr_bytes = hdr_length = 0; + } else { + server_request_hdr_bytes = hdr_length = write_header_into_buffer(&t_state.hdr_info.server_request, server_entry->write_buffer); + } // the plugin decided to append a message to the request if (t_state.api_server_request_body_set) { @@ -7113,7 +7119,7 @@ HttpSM::setup_cache_read_transfer() // Now dump the header into the buffer ink_assert(t_state.hdr_info.client_response.status_get() != HTTPStatus::NOT_MODIFIED); client_response_hdr_bytes = hdr_size = write_response_header_into_buffer(&t_state.hdr_info.client_response, buf); - cache_response_hdr_bytes = client_response_hdr_bytes; + cache_response_hdr_bytes = reported_client_response_hdr_bytes(); HTTP_SM_SET_DEFAULT_HANDLER(&HttpSM::tunnel_handler); @@ -8016,7 +8022,7 @@ HttpSM::update_stats() HttpTransact::update_size_and_time_stats( &t_state, total_time, ua_write_time, os_read_time, client_request_hdr_bytes, client_request_body_bytes, - client_response_hdr_bytes, client_response_body_bytes, server_request_hdr_bytes, server_request_body_bytes, + reported_client_response_hdr_bytes(), client_response_body_bytes, server_request_hdr_bytes, server_request_body_bytes, server_response_hdr_bytes, server_response_body_bytes, pushed_response_hdr_bytes, pushed_response_body_bytes, milestones); /* if (is_action_tag_set("http_handler_times")) { diff --git a/src/proxy/http/HttpTunnel.cc b/src/proxy/http/HttpTunnel.cc index 2f0b0afcc0f..9c247582093 100644 --- a/src/proxy/http/HttpTunnel.cc +++ b/src/proxy/http/HttpTunnel.cc @@ -1211,7 +1211,12 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) } } - if (c_write == 0) { + // A bodyless HTTP/2 response (204/304/HEAD) still owes a HEADERS frame, and the stream + // self-signals WRITE_COMPLETE. Match by identity: plugin agents are HTTP_CLIENT too. + ProxyTransaction *ua_txn = sm->get_ua_txn(); + bool const flush_header = c_write == 0 && ua_txn != nullptr && c->vc == ua_txn && ua_txn->has_pending_send_header(); + + if (c_write == 0 && !flush_header) { // Nothing to do, call back the cleanup handlers c->write_vio = nullptr; consumer_handler(VC_EVENT_WRITE_COMPLETE, c); @@ -1234,10 +1239,9 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) Dbg(dbg_ctl_http_tunnel, "Start write vio %" PRId64 " bytes", c_write); // Start the writes now that we know we will consume all the initial data c->write_vio = c->vc->do_io_write(this, c_write, c->buffer_reader); - ink_assert(c_write > 0); if (c->write_vio == nullptr) { consumer_handler(VC_EVENT_ERROR, c); - } else if (c->write_vio->ntodo() == 0 && c->alive) { + } else if (!flush_header && c->write_vio->ntodo() == 0 && c->alive) { consumer_handler(VC_EVENT_WRITE_COMPLETE, c); } } diff --git a/src/proxy/http2/Http2Stream.cc b/src/proxy/http2/Http2Stream.cc index a435262e9f3..800cc8d4d8b 100644 --- a/src/proxy/http2/Http2Stream.cc +++ b/src/proxy/http2/Http2Stream.cc @@ -302,6 +302,34 @@ Http2Stream::decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_ta return error; } +bool +Http2Stream::supports_direct_header_passing() const +{ + return true; +} + +bool +Http2Stream::is_parsed_receive_header_ready() const +{ + return this->_is_parsed_receive_header_ready; +} + +const HTTPHdr * +Http2Stream::parsed_receive_header() const +{ + return &this->_receive_header; +} + +bool +Http2Stream::has_pending_send_header() const +{ + if (this->parsing_header_done || this->_sm == nullptr) { + return false; + } + return this->is_outbound_connection() ? this->_sm->get_server_request_header() != nullptr : + this->_sm->get_client_response_header() != nullptr; +} + void Http2Stream::send_headers(Http2ConnectionState & /* cstate ATS_UNUSED */) { @@ -351,7 +379,8 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate ATS_UNUSED */) // A failed conversion leaves a \xffVOID method that only parse_req can turn into a 400. 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); + // The stream owns _receive_header and outlives the handoff, so the pulled pointer cannot dangle. + this->_is_parsed_receive_header_ready = true; if (this->receive_end_stream) { // nbytes == 0 reads as "paused" to the VIO layer, which swallows the signal. this->read_vio.nbytes = this->data_length + _receive_header.length_get(); @@ -361,13 +390,7 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate ATS_UNUSED */) 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); + return; } // Write header to a buffer. Borrowing logic from HttpSM::write_header_into_buffer. @@ -603,7 +626,8 @@ Http2Stream::do_io_write(Continuation *c, int64_t nbytes, IOBufferReader *abuffe write_vio.op = VIO::WRITE; _send_reader = abuffer; - if (c != nullptr && nbytes > 0 && this->is_state_writeable()) { + // A bodyless message (nbytes == 0) still owes its HEADERS frame. + if (c != nullptr && (nbytes > 0 || this->has_pending_send_header()) && this->is_state_writeable()) { update_write_request(false); } else if (!this->is_state_writeable()) { // Cannot start a write on a closed stream @@ -883,7 +907,8 @@ Http2Stream::update_write_request(bool call_update) IOBufferReader *vio_reader = write_vio.get_reader(); - if (write_vio.ntodo() > 0 && (!vio_reader->is_read_avail_more_than(0))) { + // A direct-passed header is work to do even with an empty send buffer. + if (write_vio.ntodo() > 0 && !vio_reader->is_read_avail_more_than(0) && !this->has_pending_send_header()) { Http2StreamDebug("update_write_request give up without doing anything ntodo=%" PRId64 " is_read_avail=%d client_window=%zd" " session_window=%zd", write_vio.ntodo(), vio_reader->is_read_avail_more_than(0), _peer_rwnd, @@ -893,15 +918,34 @@ Http2Stream::update_write_request(bool call_update) // Process the new data if (!this->parsing_header_done) { - // Still parsing the request or response header int bytes_used = 0; ParseResult state; - if (this->is_outbound_connection()) { + HTTPHdr *send_hdr = this->_sm == nullptr ? nullptr : + this->is_outbound_connection() ? this->_sm->get_server_request_header() : + this->_sm->get_client_response_header(); + + if (send_hdr != nullptr) { + // Field-by-field, not copy(): copy() would wipe the create(HTTP_2_0) pseudos that the + // 1.1->2 conversion fills. The ready flag re-arms per header (1xx interim, retries). + if (this->is_outbound_connection()) { + this->_send_header.method_set(send_hdr->method_get()); + this->_send_header.url_set(send_hdr->url_get()); + } else { + this->_send_header.status_set(send_hdr->status_get()); + } + for (auto &field : *send_hdr) { + MIMEField *f = this->_send_header.field_create(field.name_get()); + + f->value_set(this->_send_header.m_heap, this->_send_header.m_mime, field.value_get()); + this->_send_header.field_attach(f); + } + this->_sm->clear_pending_send_header(); + state = ParseResult::DONE; + } else if (this->is_outbound_connection()) { state = this->_send_header.parse_req(&http_parser, this->_send_reader, &bytes_used, false); } else { - state = this->_send_header.parse_resp(&http_parser, this->_send_reader, &bytes_used, false); + state = ParseResult::CONT; } - // HTTPHdr::parse_resp() consumed the send_reader in above write_vio.ndone += bytes_used; switch (state) { diff --git a/src/proxy/logging/TransactionLogData.cc b/src/proxy/logging/TransactionLogData.cc index b226fc8349f..ea286c75ba2 100644 --- a/src/proxy/logging/TransactionLogData.cc +++ b/src/proxy/logging/TransactionLogData.cc @@ -525,7 +525,7 @@ int64_t TransactionLogData::get_client_response_hdr_bytes() const { if (likely(m_http_sm != nullptr)) { - return m_http_sm->client_response_hdr_bytes; + return m_http_sm->reported_client_response_hdr_bytes(); } return 0; } From 86afd774d9fbf0710147a329a2f4c4cd283a3a25 Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Fri, 24 Jul 2026 23:53:18 -0600 Subject: [PATCH 5/5] Fix H2 interim-response drop and request validation parity Restore the inbound parse_resp() fallback in update_write_request(): the direct-header fast path only covers final responses, so serialized 1xx responses (100-continue, 103 early-hints) written by setup_100_continue_transfer() were dropped instead of being emitted as HEADERS frames. Gate the request fast path on the parse_req checks it would otherwise skip, falling back to serialize+parse when any of them would reject: the method must be all-token; a Content-Length must be a single 1*DIGIT with no differing duplicate and no Transfer-Encoding; and the Host built from :authority must satisfy http_parse_host_header(), the same test validate_hdr_host() applies. url_parse_internet() accepts the userinfo that RFC 9113 8.3.1 bans, so ":authority: user@host" would otherwise reach the origin instead of being rejected. Keep the fast path from mis-mapping an oversized header set to 414 (REQUEST_URI_TOO_LONG); it now returns a generic 400. --- src/proxy/http/HttpSM.cc | 11 +++++++---- src/proxy/http2/Http2Stream.cc | 35 ++++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 53ea127526b..8ce1a539f87 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -621,7 +621,8 @@ HttpSM::state_read_client_request_header(int event, void *data) ///////////////////// ParseResult state; - ProxyTransaction *ua_txn = _ua.get_txn(); + ProxyTransaction *ua_txn = _ua.get_txn(); + bool direct_header_passed = false; if (ua_txn->supports_direct_header_passing() && ua_txn->is_parsed_receive_header_ready()) { // UA_FIRST_READ never fires here: the read buffer stays empty. @@ -630,8 +631,9 @@ HttpSM::state_read_client_request_header(int event, void *data) milestones[TS_MILESTONE_UA_FIRST_READ] = ink_get_hrtime(); } t_state.hdr_info.client_request.copy(ua_txn->parsed_receive_header()); - bytes_used = t_state.hdr_info.client_request.length_get(); - state = ParseResult::DONE; + bytes_used = t_state.hdr_info.client_request.length_get(); + state = ParseResult::DONE; + direct_header_passed = true; } else { state = t_state.hdr_info.client_request.parse_req( &http_parser, ua_txn->get_remote_reader(), &bytes_used, _ua.get_entry()->eos, t_state.http_config_param->strict_uri_parsing, @@ -718,7 +720,8 @@ HttpSM::state_read_client_request_header(int event, void *data) // Disable further I/O on the client _ua.get_entry()->read_vio->nbytes = _ua.get_entry()->read_vio->ndone; - (bytes_used > t_state.http_config_param->http_request_line_max_size) ? + // bytes_used is the whole header set on the direct path, not a request-line length. + (!direct_header_passed && bytes_used > t_state.http_config_param->http_request_line_max_size) ? t_state.http_return_code = HTTPStatus::REQUEST_URI_TOO_LONG : t_state.http_return_code = HTTPStatus::NONE; diff --git a/src/proxy/http2/Http2Stream.cc b/src/proxy/http2/Http2Stream.cc index 800cc8d4d8b..942886a8d19 100644 --- a/src/proxy/http2/Http2Stream.cc +++ b/src/proxy/http2/Http2Stream.cc @@ -31,8 +31,10 @@ #include "tscore/Diags.h" #include "tscore/HTTPVersion.h" #include "tscore/ink_assert.h" +#include "tscore/ParseRules.h" #include "tsutil/DbgCtl.h" +#include #include #define REMEMBER(e, r) \ @@ -376,9 +378,37 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate ATS_UNUSED */) url_is_uri_compliant(level, _receive_header.fragment_get())); }; + // parse_req also enforces token methods, Host and Content-Length framing (RFC 9110 8.6). + auto parse_req_would_accept = [&]() { + auto method{_receive_header.method_get()}; + if (method.empty() || std::any_of(method.begin(), method.end(), [](char c) { return !ParseRules::is_token(c); })) { + return false; + } + // url_parse_internet() accepts the userinfo that RFC 9113 8.3.1 bans from :authority, + // and the Host built from it never sees validate_hdr_host(). Mirror that check here. + if (MIMEField *host = _receive_header.field_find(static_cast(MIME_FIELD_HOST)); host != nullptr) { + std::string_view parsed_host; + int port = 0; + bool has_port = false; + + if (host->has_dups() || !http_parse_host_header(host->value_get(), parsed_host, port, has_port)) { + return false; + } + } + if (MIMEField *cl = _receive_header.field_find(static_cast(MIME_FIELD_CONTENT_LENGTH)); cl != nullptr) { + auto value{cl->value_get()}; + if (cl->has_dups() || value.empty() || std::any_of(value.begin(), value.end(), [](char c) { return c < '0' || c > '9'; }) || + _receive_header.field_find(static_cast(MIME_FIELD_TRANSFER_ENCODING)) != nullptr) { + return false; + } + } + return true; + }; + // A failed conversion leaves a \xffVOID method that only parse_req can turn into a 400. 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()) { + _receive_header.type_get() == HTTPType::REQUEST && this->_sm != nullptr && this->read_vio.nbytes > 0 && uri_ok() && + parse_req_would_accept()) { // The stream owns _receive_header and outlives the handoff, so the pulled pointer cannot dangle. this->_is_parsed_receive_header_ready = true; if (this->receive_end_stream) { @@ -944,7 +974,8 @@ Http2Stream::update_write_request(bool call_update) } else if (this->is_outbound_connection()) { state = this->_send_header.parse_req(&http_parser, this->_send_reader, &bytes_used, false); } else { - state = ParseResult::CONT; + // Interim 1xx responses (setup_100_continue_transfer()) are still serialized. + state = this->_send_header.parse_resp(&http_parser, this->_send_reader, &bytes_used, false); } write_vio.ndone += bytes_used;