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 31eb68f6fb Shut the client read side down per transaction, not per
connection (#13523)
31eb68f6fb is described below
commit 31eb68f6fb07f105ee90ae062524e8b8f364b91d
Author: Brian Neradt <[email protected]>
AuthorDate: Mon Aug 10 11:35:32 2026 -0500
Shut the client read side down per transaction, not per connection (#13523)
HttpSM::state_watch_for_client_abort reached past the transaction to
_ua.get_txn()->get_netvc() to half close the client read side on an early
EOS. For HTTP/2 and HTTP/3 that NetVConnection is shared by every stream
on the connection, so a single aborted stream stopped the session from
reading frames for all of the others. Route the shutdown through the
transaction instead: Http2Stream and HQTransaction already implement
do_io_shutdown() as a deliberate no-op for exactly this reason, and
HTTP/1.x is unaffected because ProxyTransaction forwards to the session's
NetVConnection.
This is the remaining half of #12529. That change was written to address
two regressions from #12502, the second being "HTTP/2 connection is
closed if a stream is reset even if other streams are alive", with the
stated approach of calling ProxyTransaction::do_io_shutdown() instead of
NetVConnection::do_io_shutdown(). It converted the two branches it added
but left the pre-existing IO_SHUTDOWN_READ branch calling the
NetVConnection directly, so the connection-wide shutdown survived for the
case where the tunnel still has a consumer besides the client. A response
transform reaches that case readily: the transform stage runs the whole
body before anything is written back, so the stream has no write of its
own and a client reset arrives as an EOS on the stream's read VIO.
Clearing the session's read VIO buffer is not inert, because
Http2CommonSession still holds that VIO and re-enables it every 128
frames through HTTP2_SESSION_EVENT_REENABLE. A release build then takes
the ntodo() <= 0 path in net_read_io and the connection stalls silently,
failing every in-flight stream; a debug build aborts on
SSLNetVConnection's `ink_assert(buf.writer())`, the assertion reported
in #9448.
Co-authored-by: Claude Opus 5 <[email protected]>
---
src/proxy/http/HttpSM.cc | 11 +-
...ttp2_client_reset_keeps_session_reading.test.py | 23 ++++
..._client_reset_keeps_session_reading.replay.yaml | 149 +++++++++++++++++++++
3 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc
index 1ffa2d8b5b..7a8f59086a 100644
--- a/src/proxy/http/HttpSM.cc
+++ b/src/proxy/http/HttpSM.cc
@@ -905,10 +905,19 @@ HttpSM::state_watch_for_client_abort(int event, void
*data)
case VC_EVENT_EOS: {
// We got an early EOS.
if (!terminate_sm) { // Not done already
+ // ProxySession::do_io_shutdown dereferences its NetVConnection
+ // unconditionally, so only shut down while the peer is still attached.
NetVConnection *netvc = _ua.get_txn()->get_netvc();
+
if (_ua.get_txn()->allow_half_open() ||
tunnel.has_consumer_besides_client()) {
if (netvc) {
- netvc->do_io_shutdown(IO_SHUTDOWN_READ);
+ // Shut the read side down through the transaction rather than
through
+ // the NetVConnection. For multiplexed protocols the NetVConnection
is
+ // shared by every stream on the connection, so shutting its read
side
+ // down here would stop the session from reading frames for all of
the
+ // other streams. HTTP/2 and HTTP/3 therefore implement
+ // do_io_shutdown() as a no-op.
+ _ua.get_txn()->do_io_shutdown(IO_SHUTDOWN_READ);
}
} else if (t_state.txn_conf->cache_http &&
(server_entry != nullptr && server_entry->vc_read_handler ==
&HttpSM::state_read_server_response_header)) {
diff --git
a/tests/gold_tests/h2/http2_client_reset_keeps_session_reading.test.py
b/tests/gold_tests/h2/http2_client_reset_keeps_session_reading.test.py
new file mode 100644
index 0000000000..1e20b5aae6
--- /dev/null
+++ b/tests/gold_tests/h2/http2_client_reset_keeps_session_reading.test.py
@@ -0,0 +1,23 @@
+'''Verify an HTTP/2 stream reset does not stop the session from reading.'''
+# 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 = __doc__
+
+Test.SkipUnless(
+ Condition.HasOpenSSLVersion('1.1.1'),
Condition.HasProxyVerifierVersion('2.8.0'),
Condition.PluginExists('null_transform.so'))
+
+Test.ATSReplayTest(replay_file="replay/http2_client_reset_keeps_session_reading.replay.yaml")
diff --git
a/tests/gold_tests/h2/replay/http2_client_reset_keeps_session_reading.replay.yaml
b/tests/gold_tests/h2/replay/http2_client_reset_keeps_session_reading.replay.yaml
new file mode 100644
index 0000000000..52974a8c0b
--- /dev/null
+++
b/tests/gold_tests/h2/replay/http2_client_reset_keeps_session_reading.replay.yaml
@@ -0,0 +1,149 @@
+# 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.
+
+#
+# A response transform makes the tunnel run the whole body through the
transform
+# before anything is written back to the client. While that is happening the
+# client stream has no write of its own, so a client stream reset is delivered
as
+# a VC_EVENT_EOS on the stream's read VIO and lands in
+# HttpSM::state_watch_for_client_abort. The transform write consumer is still
+# alive there, so the SM half closes the client read side to let the rest of
the
+# transaction finish.
+#
+# That shutdown has to apply to the stream, not to the underlying connection:
for
+# HTTP/2 the NetVConnection is shared by every stream, so shutting its read
side
+# down stops the session from reading frames for all of the other streams. The
+# second request below is sent after the reset and only gets a response if ATS
is
+# still reading the connection.
+#
+
+meta:
+ version: '1.0'
+
+autest:
+ description: 'Verify an HTTP/2 stream reset does not stop the session from
reading'
+
+ server:
+ name: 'server'
+
+ client:
+ name: 'client'
+
+ ats:
+ name: 'ts'
+
+ process_config:
+ enable_tls: true
+ enable_cache: true
+
+ plugin_config:
+ # A response transform so the tunnel has a consumer besides the client
+ # while the body is being read from the origin.
+ - 'null_transform.so'
+
+ records_config:
+ proxy.config.diags.debug.enabled: 1
+ proxy.config.diags.debug.tags: 'http|http_tunnel'
+ # Let the rest of the transaction run for as long as the origin takes.
+ proxy.config.http.background_fill_active_timeout: 0
+ proxy.config.http.background_fill_completed_threshold: 0.0
+ # The origin speaks HTTP/2 so that it can delay the response body without
+ # delaying the response header.
+ proxy.config.ssl.client.alpn_protocols: 'h2,http/1.1'
+ proxy.config.ssl.client.verify.server.policy: 'PERMISSIVE'
+
+ remap_config:
+ - 'map / https://127.0.0.1:{SERVER_HTTPS_PORT}'
+
+ log_validation:
+ traffic_out:
+ contains:
+ - expression: 'adding consumer .transform write.'
+ description: 'Verify the tunnel had a consumer besides the client'
+ - expression: 'state_watch_for_client_abort, VC_EVENT_EOS'
+ description: 'Verify the stream reset was handled as a client
abort'
+
+sessions:
+- protocol:
+ stack: http2
+ tls:
+ sni: test_sni
+
+ transactions:
+
+ # Stream 1: the origin sends the response header right away and then stalls
+ # before the body, so the transform stage of the tunnel is still running when
+ # the client resets the stream.
+ - client-request:
+ frames:
+ - HEADERS:
+ headers:
+ fields:
+ - [":method", GET]
+ - [":scheme", https]
+ - [":authority", example.data.com]
+ - [":path", /reset-mid-transform]
+ - [uuid, reset-mid-transform]
+ - RST_STREAM:
+ delay: 1s
+ error-code: CANCEL
+
+ server-response:
+ frames:
+ - HEADERS:
+ headers:
+ fields:
+ - [":status", 200]
+ - [Content-Type, text/html]
+ - [Content-Length, '11']
+ - [Cache-Control, 'max-age=300']
+ - DATA:
+ delay: 3s
+ content:
+ encoding: plain
+ data: server_test
+ size: 11
+
+ # Stream 3: sent after the reset above. ATS only sees this request if it is
+ # still reading the connection, so a missing response here means one stream's
+ # abort took the whole connection down with it.
+ - client-request:
+ delay: 2s
+ headers:
+ fields:
+ - [":method", GET]
+ - [":scheme", https]
+ - [":authority", example.data.com]
+ - [":path", /after-the-reset]
+ - [uuid, after-the-reset]
+
+ server-response:
+ headers:
+ fields:
+ - [":status", 200]
+ - [Content-Type, text/html]
+ - [Content-Length, '16']
+ - [X-Response, after-the-reset]
+ content:
+ encoding: plain
+ data: after_the_reset
+ size: 16
+
+ proxy-response:
+ status: 200
+ headers:
+ fields:
+ - [X-Response, {value: 'after-the-reset', as: equal}]
