This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new dcb18509fa Fix H2 origin payload handling (#13363)
dcb18509fa is described below
commit dcb18509fa299a159d9ac511a06ac53bacc17254
Author: Brian Neradt <[email protected]>
AuthorDate: Wed Jul 29 17:09:38 2026 -0500
Fix H2 origin payload handling (#13363)
HTTP/2 origin responses can legally carry a non-zero Content-Length
when no payload is sent, such as responses to HEAD requests. ATS
discarded the outbound request method after encoding the H2 HEADERS
frame and could therefore reject a valid no-body response as a
payload-length error.
An H2 DATA sender also treated every byte visible through its
IOBufferReader as eligible for the current write. Reader availability is
independent of the finite VIO operation: VIO::ntodo() is the
authoritative boundary, and the ordinary network VConnection already
caps writes to it. Without that cap, the regression sent 327,675 bytes
for a 300,000-byte PUT and the H2 origin returned GOAWAY with
PROTOCOL_ERROR.
This retains the outbound request method on the H2 stream for response
validation. It also caps DATA payloads to the remaining write VIO bytes,
still setting END_STREAM when the final authorized bytes are sent, and
extends the H2 origin replay coverage with HEAD and large PUT cases.
---
include/proxy/http2/Http2Stream.h | 21 +++-
src/proxy/http2/Http2ConnectionState.cc | 15 ++-
.../h2/gold/http-request-method-metrics.gold | 2 +-
tests/gold_tests/h2/h2origin.test.py | 6 +-
tests/gold_tests/h2/replay_h2origin/h2-origin.yaml | 123 +++++++++++++++++++++
5 files changed, 160 insertions(+), 7 deletions(-)
diff --git a/include/proxy/http2/Http2Stream.h
b/include/proxy/http2/Http2Stream.h
index dbf64edb4b..3fb388670c 100644
--- a/include/proxy/http2/Http2Stream.h
+++ b/include/proxy/http2/Http2Stream.h
@@ -164,6 +164,7 @@ public:
void increment_data_length(uint64_t length);
bool payload_length_is_valid() const;
bool is_write_vio_done() const;
+ int64_t write_vio_ntodo() const;
void update_sent_count(unsigned num_bytes);
Http2StreamId get_id() const;
Http2StreamState get_state() const;
@@ -175,6 +176,7 @@ public:
void set_receive_headers(HTTPHdr &h2_headers);
void reset_receive_headers();
void reset_send_headers();
+ void set_sent_request_method(int method);
MIOBuffer *read_vio_writer() const;
int64_t read_vio_read_avail();
bool is_read_enabled() const;
@@ -215,6 +217,7 @@ private:
Http2StreamId _id = -1;
Http2StreamState _state = Http2StreamState::HTTP2_STREAM_STATE_IDLE;
int64_t _http_sm_id = -1;
+ int _sent_request_method{-1};
HTTPHdr _receive_header;
#if TS_USE_MALLOC_ALLOCATOR
@@ -316,6 +319,12 @@ Http2Stream::is_write_vio_done() const
return this->write_vio.ntodo() == 0;
}
+inline int64_t
+Http2Stream::write_vio_ntodo() const
+{
+ return this->write_vio.ntodo();
+}
+
inline void
Http2Stream::update_sent_count(unsigned num_bytes)
{
@@ -391,6 +400,12 @@ Http2Stream::reset_send_headers()
this->_send_header.create(HTTPType::RESPONSE);
}
+inline void
+Http2Stream::set_sent_request_method(int method)
+{
+ _sent_request_method = method;
+}
+
// Check entire DATA payload length if content-length: header exists
inline void
Http2Stream::increment_data_length(uint64_t length)
@@ -407,9 +422,9 @@ Http2Stream::payload_length_is_valid() const
// Skip Content-Length check on [RFC 7230] 3.3.2 conditions
bool is_payload_precluded =
- this->is_outbound_connection() && (_send_header.method_get_wksidx() ==
HTTP_WKSIDX_HEAD ||
- (_send_header.method_get_wksidx() ==
HTTP_WKSIDX_GET && _send_header.presence(mask) &&
- _receive_header.status_get() ==
HTTPStatus::NOT_MODIFIED));
+ this->is_outbound_connection() &&
+ (_sent_request_method == HTTP_WKSIDX_HEAD || (_sent_request_method ==
HTTP_WKSIDX_GET && _send_header.presence(mask) &&
+ _receive_header.status_get()
== HTTPStatus::NOT_MODIFIED));
if (content_length != 0 && !is_payload_precluded && content_length !=
data_length) {
Warning("Bad payload length content_length=%d data_legnth=%d session_id=%"
PRId64, content_length,
diff --git a/src/proxy/http2/Http2ConnectionState.cc
b/src/proxy/http2/Http2ConnectionState.cc
index cb44800014..446dfc4dbd 100644
--- a/src/proxy/http2/Http2ConnectionState.cc
+++ b/src/proxy/http2/Http2ConnectionState.cc
@@ -2332,6 +2332,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream
*stream, size_t &payload_len
uint8_t flags = 0x00;
IOBufferReader *resp_reader = stream->get_data_reader_for_send();
+ bool last_write_vio_payload{false};
SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());
@@ -2366,6 +2367,16 @@ Http2ConnectionState::send_a_data_frame(Http2Stream
*stream, size_t &payload_len
} else {
payload_length = resp_reader->read_avail();
}
+ const int64_t remaining_write = stream->write_vio_ntodo();
+ if (remaining_write != INT64_MAX) {
+ if (remaining_write > 0) {
+ last_write_vio_payload = payload_length >=
static_cast<size_t>(remaining_write);
+ payload_length = std::min(payload_length,
static_cast<size_t>(remaining_write));
+ } else {
+ last_write_vio_payload = true;
+ payload_length = 0;
+ }
+ }
} else {
payload_length = 0;
}
@@ -2395,7 +2406,8 @@ Http2ConnectionState::send_a_data_frame(Http2Stream
*stream, size_t &payload_len
return Http2SendDataFrameResult::NO_PAYLOAD;
}
- if (stream->is_write_vio_done() &&
!resp_reader->is_read_avail_more_than(payload_length) &&
!stream->expect_send_trailer()) {
+ if (stream->is_write_vio_done() && (last_write_vio_payload ||
!resp_reader->is_read_avail_more_than(payload_length)) &&
+ !stream->expect_send_trailer()) {
Http2StreamDebug(this->session, stream->get_id(), "End of Data Frame");
flags |= HTTP2_FLAGS_DATA_END_STREAM;
}
@@ -2524,6 +2536,7 @@ Http2ConnectionState::send_headers_frame(Http2Stream
*stream)
flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
if (stream->is_outbound_connection()) { // Will be sending a request_header
int method = send_hdr->method_get_wksidx();
+ stream->set_sent_request_method(method);
// Set END_STREAM on request headers for POST, etc. methods combined with
// an explicit length 0. Some origins RST on request headers with
diff --git a/tests/gold_tests/h2/gold/http-request-method-metrics.gold
b/tests/gold_tests/h2/gold/http-request-method-metrics.gold
index f949dc4270..6418e16a56 100644
--- a/tests/gold_tests/h2/gold/http-request-method-metrics.gold
+++ b/tests/gold_tests/h2/gold/http-request-method-metrics.gold
@@ -1,3 +1,3 @@
proxy.process.http.get_requests 4
proxy.process.http.post_requests 11
-proxy.process.http.put_requests 0
+proxy.process.http.put_requests 1
diff --git a/tests/gold_tests/h2/h2origin.test.py
b/tests/gold_tests/h2/h2origin.test.py
index 6b0211a8d0..41c9c5bae6 100644
--- a/tests/gold_tests/h2/h2origin.test.py
+++ b/tests/gold_tests/h2/h2origin.test.py
@@ -89,7 +89,7 @@ tr = Test.AddTestRun("Wait for the squid.log to be written")
timeout = 30
watcher = tr.Processes.Process("watcher")
watcher.Command = f"sleep {timeout}"
-watcher.Ready = When.FileContains(ts.Disk.squid_log.Name, r'14 http/1.1
http/2')
+watcher.Ready = When.FileContains(ts.Disk.squid_log.Name, r'16 http/2 http/2')
watcher.TimeOut = timeout
tr.StillRunningAfter = ts
tr.StillRunningAfter = server
@@ -99,7 +99,7 @@ tr.Processes.Default.Command = 'echo await_squid_log'
tr.Processes.Default.ReturnCode = 0
# UUIDs 1-4 should be http/1.1 clients and H2 origin
-# UUIDs 5-9 should be http/2 clients and H2 origins
+# UUIDs 5-11 and 15-16 should be http/2 clients and H2 origins
ts.Disk.squid_log.Content = Testers.ContainsExpression(" [1-4] http/1.1
http/2", "cases 1-4 request http/1.1")
ts.Disk.squid_log.Content += Testers.ExcludesExpression(" [1-4] http/2
http/2", "cases 1-4 request http/1.1")
ts.Disk.squid_log.Content += Testers.ContainsExpression(" 1[1-4] http/1.1
http/2", "cases 12-14 request http/1.1")
@@ -108,6 +108,8 @@ ts.Disk.squid_log.Content += Testers.ContainsExpression("
[5-9] http/2 http/2",
ts.Disk.squid_log.Content += Testers.ExcludesExpression(" [5-9] http/1.1
http/2", "cases 5-11 request http/2")
ts.Disk.squid_log.Content += Testers.ContainsExpression(" 1[0-1] http/2
http/2", "cases 5-11 request http/2")
ts.Disk.squid_log.Content += Testers.ExcludesExpression(" 1[0-1] http/1.1
http/2", "cases 5-11 request http/2")
+ts.Disk.squid_log.Content += Testers.ContainsExpression(" 1[5-6] http/2
http/2", "cases 15-16 request http/2")
+ts.Disk.squid_log.Content += Testers.ExcludesExpression(" 1[5-6] http/1.1
http/2", "cases 15-16 request http/2")
tr = Test.AddTestRun("Test HTTP method Metrics")
tr.Processes.Default.Command = (
diff --git a/tests/gold_tests/h2/replay_h2origin/h2-origin.yaml
b/tests/gold_tests/h2/replay_h2origin/h2-origin.yaml
index acf7035fcd..0310eacb34 100644
--- a/tests/gold_tests/h2/replay_h2origin/h2-origin.yaml
+++ b/tests/gold_tests/h2/replay_h2origin/h2-origin.yaml
@@ -477,3 +477,126 @@ sessions:
content:
encoding: plain
size: 3200
+
+ #
+ # Test 8: HEAD response with a non-zero Content-Length and no body.
+ #
+ - all: { headers: { fields: [[ uuid, 15 ]]}}
+
+ client-request:
+ version: '2'
+ scheme: https
+ method: HEAD
+ url: /some/head
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Host, data.brian.example.com ]
+ content:
+ encoding: plain
+ size: 0
+
+ proxy-request:
+ protocol:
+ stack: http2
+ tls:
+ version: TLSv1.2
+ sni: data.brian.example.com
+ proxy-verify-mode: 1
+ proxy-provided-cert: false
+ version: '2'
+ scheme: https
+ method: HEAD
+ url: /some/head
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Host, data.brian.example.com ]
+ - [ Content-Length, 0 ]
+ content:
+ encoding: plain
+ size: 0
+
+ server-response:
+ version: '2'
+ status: 200
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Content-Length, 100 ]
+ content:
+ encoding: plain
+ size: 0
+
+ proxy-response:
+ version: '2'
+ status: 200
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Content-Length, 100 ]
+ content:
+ encoding: plain
+ size: 0
+
+ #
+ # Test 9: large PUT body with a large response.
+ #
+ - all: { headers: { fields: [[ uuid, 16 ]]}}
+
+ client-request:
+ version: '2'
+ scheme: https
+ method: PUT
+ url: /some/large-put
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Host, data.brian.example.com ]
+ - [ Content-Length, 300000 ]
+ content:
+ encoding: plain
+ size: 300000
+
+ proxy-request:
+ protocol:
+ stack: http2
+ tls:
+ version: TLSv1.2
+ sni: data.brian.example.com
+ proxy-verify-mode: 1
+ proxy-provided-cert: false
+ version: '2'
+ scheme: https
+ method: PUT
+ url: /some/large-put
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Host, data.brian.example.com ]
+ - [ Content-Length, 300000 ]
+ content:
+ encoding: plain
+ size: 300000
+
+ server-response:
+ version: '2'
+ status: 200
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Content-Length, 300000 ]
+ content:
+ encoding: plain
+ size: 300000
+
+ proxy-response:
+ version: '2'
+ status: 200
+ headers:
+ encoding: esc_json
+ fields:
+ - [ Content-Length, 300000 ]
+ content:
+ encoding: plain
+ size: 300000
