From a45cc0d96dac2b46e7d381e06c5fd347f3136e1f Mon Sep 17 00:00:00 2001 From: bneradt Date: Mon, 10 Aug 2026 09:38:37 -0500 Subject: [PATCH] Shut the client read side down per transaction, not per connection 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 --- src/proxy/http/HttpSM.cc | 11 +- ...client_reset_keeps_session_reading.test.py | 23 +++ ...nt_reset_keeps_session_reading.replay.yaml | 149 ++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 tests/gold_tests/h2/http2_client_reset_keeps_session_reading.test.py create mode 100644 tests/gold_tests/h2/replay/http2_client_reset_keeps_session_reading.replay.yaml diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 1ffa2d8b5b1..7a8f59086a7 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 00000000000..1e20b5aae6f --- /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 00000000000..52974a8c0b9 --- /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}]