From 43896bfea6ecd12606c36d1b53998e24ee77a11c Mon Sep 17 00:00:00 2001 From: Katsutoshi Ikenoya Date: Wed, 4 Mar 2026 17:52:27 +0900 Subject: [PATCH 1/4] Fix 408 timeout caused by ignoring DATA frames after GOAWAY --- include/proxy/http2/Http2ConnectionState.h | 11 +++ include/proxy/http2/Http2Stream.h | 3 + src/proxy/http2/Http2CommonSession.cc | 19 +++-- src/proxy/http2/Http2ConnectionState.cc | 96 ++++++++++++++++++---- 4 files changed, 106 insertions(+), 23 deletions(-) diff --git a/include/proxy/http2/Http2ConnectionState.h b/include/proxy/http2/Http2ConnectionState.h index 53054416d9e..a506de7ab63 100644 --- a/include/proxy/http2/Http2ConnectionState.h +++ b/include/proxy/http2/Http2ConnectionState.h @@ -140,6 +140,7 @@ class Http2ConnectionState : public Continuation Http2StreamId get_latest_stream_id_in() const; Http2StreamId get_latest_stream_id_out() const; int get_stream_requests() const; + bool get_goaway_sent() const; void increment_stream_requests(); bool is_peer_concurrent_stream_ub() const; bool is_peer_concurrent_stream_lb() const; @@ -281,6 +282,10 @@ class Http2ConnectionState : public Continuation Http2StreamId latest_streamid_out = 0; std::atomic stream_requests = 0; + // The last stream identifier in the GOAWAY frame + Http2StreamId last_stream_id_tx = 0; + bool goaway_sent = false; + // Counter for current active streams which are started by the client. std::atomic peer_streams_count_in = 0; @@ -442,6 +447,12 @@ Http2ConnectionState::get_stream_requests() const return stream_requests; } +inline bool +Http2ConnectionState::get_goaway_sent() const +{ + return goaway_sent; +} + inline void Http2ConnectionState::increment_stream_requests() { diff --git a/include/proxy/http2/Http2Stream.h b/include/proxy/http2/Http2Stream.h index 5d9beac647e..650201c13b9 100644 --- a/include/proxy/http2/Http2Stream.h +++ b/include/proxy/http2/Http2Stream.h @@ -184,6 +184,9 @@ class Http2Stream : public ProxyTransaction bool parsing_header_done = false; bool is_first_transaction_flag = false; + bool reset_header_after_decoding = false; + bool free_stream_after_decoding = false; + HTTPHdr _send_header; IOBufferReader *_send_reader = nullptr; Http2DependencyTree::Node *priority_node = nullptr; diff --git a/src/proxy/http2/Http2CommonSession.cc b/src/proxy/http2/Http2CommonSession.cc index b05fcfb35cb..8e07414e971 100644 --- a/src/proxy/http2/Http2CommonSession.cc +++ b/src/proxy/http2/Http2CommonSession.cc @@ -362,25 +362,30 @@ Http2CommonSession::do_process_frame_read(int /* event ATS_UNUSED */, VIO *vio, while (this->_read_buffer_reader->read_avail() >= static_cast(HTTP2_FRAME_HEADER_LEN)) { // Cancel reading if there was an error or connection is closed - if (connection_state.tx_error_code.code != static_cast(Http2ErrorCode::HTTP2_ERROR_NO_ERROR) || - connection_state.is_state_closed()) { + const auto has_fatal_error_code = + (connection_state.tx_error_code.code != static_cast(Http2ErrorCode::HTTP2_ERROR_NO_ERROR) && + connection_state.tx_error_code.code != static_cast(Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM)); + if (has_fatal_error_code || connection_state.is_state_closed()) { Http2SsnDebug("reading a frame has been canceled (%u)", connection_state.tx_error_code.code); - break; + return 0; } - Http2ErrorCode err = Http2ErrorCode::HTTP2_ERROR_NO_ERROR; - if (this->connection_state.get_stream_error_rate() > std::min(1.0, Http2::stream_error_rate_threshold * 2.0)) { + if (this->connection_state.get_stream_error_rate() > std::min(1.0, Http2::stream_error_rate_threshold * 2.0) && + !this->connection_state.get_goaway_sent()) { ip_port_text_buffer ipb; const char *peer_ip = ats_ip_ntop(this->get_proxy_session()->get_remote_addr(), ipb, sizeof(ipb)); SiteThrottledWarning("HTTP/2 session error peer_ip=%s session_id=%" PRId64 " closing a connection, because its stream error rate (%f) exceeded the threshold (%f)", peer_ip, this->get_connection_id(), this->connection_state.get_stream_error_rate(), Http2::stream_error_rate_threshold); - err = Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM; + this->connection_state.send_goaway_frame(this->connection_state.get_latest_stream_id_in(), + Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM); + this->set_half_close_local_flag(true); } // Return if there was an error - if (err > Http2ErrorCode::HTTP2_ERROR_NO_ERROR || do_start_frame_read(err) < 0) { + auto err = Http2ErrorCode::HTTP2_ERROR_NO_ERROR; + if (do_start_frame_read(err) < 0) { // send an error if specified. Otherwise, just go away this->connection_state.restart_receiving(nullptr); if (err > Http2ErrorCode::HTTP2_ERROR_NO_ERROR) { diff --git a/src/proxy/http2/Http2ConnectionState.cc b/src/proxy/http2/Http2ConnectionState.cc index 4b38579574c..8692fffca1b 100644 --- a/src/proxy/http2/Http2ConnectionState.cc +++ b/src/proxy/http2/Http2ConnectionState.cc @@ -118,6 +118,14 @@ Http2ConnectionState::rcv_data_frame(const Http2Frame &frame) "recv data bad frame client id"); } + // After sending a GOAWAY frame, the sender can discard frames for streams initiated by the receiver with + // identifiers higher than the identified last stream. However, DATA frames MUST be counted toward + // the connection flow-control window. (Details in [RFC 9113] 6.8.) + if (this->goaway_sent && id > this->last_stream_id_tx) { + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_REFUSED_STREAM, + "recv data with id higher than last stream id"); + } + Http2Stream *stream = this->find_stream(id); if (stream == nullptr) { if (this->is_valid_streamid(id)) { @@ -330,15 +338,25 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame) "recv headers bad client id"); } + // After sending a GOAWAY frame, the sender can discard frames for streams initiated by the receiver with + // identifiers higher than the identified last stream. However, HEADERS, PUSH_PROMISE, and CONTINUATION + // frames MUST be minimally processed to ensure that the state maintained for field section compression is + // consistent (Details in [RFC 9113] 6.8.) + if (this->goaway_sent && stream_id > this->last_stream_id_tx) { + reset_header_after_decoding = true; + } + if (!stream) { if (reset_header_after_decoding) { free_stream_after_decoding = true; uint32_t const initial_local_stream_window = this->acknowledged_local_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE); ink_assert(dynamic_cast(this->session->get_proxy_session())); - ink_assert(this->session->is_outbound() == true); - stream = THREAD_ALLOC_INIT(http2StreamAllocator, this_ethread(), this->session->get_proxy_session(), stream_id, - this->peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE), initial_local_stream_window, - !STREAM_IS_REGISTERED); + stream = THREAD_ALLOC_INIT(http2StreamAllocator, this_ethread(), this->session->get_proxy_session(), stream_id, + this->peer_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE), initial_local_stream_window, + !STREAM_IS_REGISTERED); + stream->mutex = new_ProxyMutex(); + SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread()); + this->stream_list.enqueue(stream); } else { // Create new stream Http2Error error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE); @@ -370,6 +388,9 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame) } } + stream->reset_header_after_decoding = reset_header_after_decoding; + stream->free_stream_after_decoding = free_stream_after_decoding; + Http2HeadersParameter params; uint32_t header_block_fragment_offset = 0; uint32_t header_block_fragment_length = payload_length; @@ -477,13 +498,19 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame) Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle, this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); - // If this was an outbound connection and the state was already closed, just clear the - // headers after processing. We just processed the heaer blocks to keep the dynamic table in + // We just processed the heaer blocks to keep the dynamic table in // sync with peer to avoid future HPACK compression errors if (reset_header_after_decoding) { stream->reset_receive_headers(); + SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread()); + this->stream_list.remove(stream); if (free_stream_after_decoding) { - THREAD_FREE(stream, http2StreamAllocator, this_ethread()); + stream->initiating_close(); + } + + if (this->goaway_sent && stream_id > this->last_stream_id_tx) { + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_REFUSED_STREAM, + "recv headers with id higher than last stream id"); } return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE); } @@ -1100,6 +1127,27 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle, this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); + // We just processed the heaer blocks to keep the dynamic table in + // sync with peer to avoid future HPACK compression errors + if (stream->reset_header_after_decoding) { + stream->reset_receive_headers(); + SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread()); + this->stream_list.remove(stream); + if (stream->free_stream_after_decoding) { + stream->initiating_close(); + } + + // After sending a GOAWAY frame, the sender can discard frames for streams initiated by the receiver with + // identifiers higher than the identified last stream. However, HEADERS, PUSH_PROMISE, and CONTINUATION + // frames MUST be minimally processed to ensure that the state maintained for field section compression is + // consistent (Details in [RFC 9113] 6.8.) + if (this->goaway_sent && stream_id > this->last_stream_id_tx) { + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_REFUSED_STREAM, + "recv continuation with id higher than last stream id"); + } + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE); + } + if (result != Http2ErrorCode::HTTP2_ERROR_NO_ERROR) { if (result == Http2ErrorCode::HTTP2_ERROR_COMPRESSION_ERROR) { return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_COMPRESSION_ERROR, @@ -1437,12 +1485,13 @@ Http2ConnectionState::rcv_frame(const Http2Frame *frame) { REMEMBER(NO_EVENT, this->recursion); const Http2StreamId stream_id = frame->header().streamid; + const auto type = frame->header().type; Http2Error error; // [RFC 7540] 5.5. Extending HTTP/2 // Implementations MUST discard frames that have unknown or unsupported types. - if (frame->header().type >= HTTP2_FRAME_TYPE_MAX) { - Http2StreamDebug(session, stream_id, "Discard a frame which has unknown type, type=%x", frame->header().type); + if (type >= HTTP2_FRAME_TYPE_MAX) { + Http2StreamDebug(session, stream_id, "Discard a frame which has unknown type, type=%x", type); return; } @@ -1457,15 +1506,28 @@ Http2ConnectionState::rcv_frame(const Http2Frame *frame) // GOAWAY: NO // WINDOW_UPDATE: YES // CONTINUATION: YES (safe http methods only, same as HEADERS frame). - if (frame->is_from_early_data() && - (frame->header().type == HTTP2_FRAME_TYPE_DATA || frame->header().type == HTTP2_FRAME_TYPE_RST_STREAM || - frame->header().type == HTTP2_FRAME_TYPE_PUSH_PROMISE || frame->header().type == HTTP2_FRAME_TYPE_GOAWAY)) { - Http2StreamDebug(session, stream_id, "Discard a frame which is received from early data and has type=%x", frame->header().type); + if (frame->is_from_early_data() && (type == HTTP2_FRAME_TYPE_DATA || type == HTTP2_FRAME_TYPE_RST_STREAM || + type == HTTP2_FRAME_TYPE_PUSH_PROMISE || type == HTTP2_FRAME_TYPE_GOAWAY)) { + Http2StreamDebug(session, stream_id, "Discard a frame which is received from early data and has type=%x", type); return; } - if (this->_frame_handlers[frame->header().type]) { - error = (this->*_frame_handlers[frame->header().type])(*frame); + // After sending a GOAWAY frame, the sender can discard frames for streams initiated by the receiver with + // identifiers higher than the identified last stream. However, HEADERS, PUSH_PROMISE, and CONTINUATION + // frames MUST be minimally processed to ensure that the state maintained for field section compression is + // consistent; similarly, DATA frames MUST be counted toward the connection flow-control window. + // (Details in [RFC 9113] 6.8.) + if (this->goaway_sent && stream_id > this->last_stream_id_tx) { + const auto is_discardable = (type != HTTP2_FRAME_TYPE_HEADERS && type != HTTP2_FRAME_TYPE_PUSH_PROMISE && + type != HTTP2_FRAME_TYPE_CONTINUATION && type != HTTP2_FRAME_TYPE_DATA); + if (is_discardable) { + Http2StreamDebug(session, stream_id, "Discard a frame which is received after sending a GOAWAY and has type=%x", type); + return; + } + } + + if (this->_frame_handlers[type]) { + error = (this->*_frame_handlers[type])(*frame); } else { error = Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_INTERNAL_ERROR, "no handler"); } @@ -2744,7 +2806,9 @@ Http2ConnectionState::send_goaway_frame(Http2StreamId id, Http2ErrorCode ec) Metrics::Counter::increment(http2_rsb.connection_errors_count); } - this->tx_error_code = {ProxyErrorClass::SSN, static_cast(ec)}; + this->tx_error_code = {ProxyErrorClass::SSN, static_cast(ec)}; + this->last_stream_id_tx = id; + this->goaway_sent = true; Http2Goaway goaway; goaway.last_streamid = id; From d98f026c077981ffce20386f2098bf1970b239cd Mon Sep 17 00:00:00 2001 From: Katsutoshi Ikenoya Date: Thu, 12 Mar 2026 14:57:48 +0900 Subject: [PATCH 2/4] Add suppress_rst parameter to initiating_close to prevent duplicate RST_STREAM --- include/proxy/http2/Http2Stream.h | 2 +- src/proxy/http2/Http2ConnectionState.cc | 12 ++++++++---- src/proxy/http2/Http2Stream.cc | 5 +++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/include/proxy/http2/Http2Stream.h b/include/proxy/http2/Http2Stream.h index 650201c13b9..be3dd9b3d53 100644 --- a/include/proxy/http2/Http2Stream.h +++ b/include/proxy/http2/Http2Stream.h @@ -83,7 +83,7 @@ class Http2Stream : public ProxyTransaction Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size); void send_headers(Http2ConnectionState &cstate); - void initiating_close(); + void initiating_close(bool suppress_rst = false); bool is_outbound_connection() const; bool is_tunneling() const; void terminate_if_possible(); diff --git a/src/proxy/http2/Http2ConnectionState.cc b/src/proxy/http2/Http2ConnectionState.cc index 8692fffca1b..ff24eb550b5 100644 --- a/src/proxy/http2/Http2ConnectionState.cc +++ b/src/proxy/http2/Http2ConnectionState.cc @@ -498,14 +498,16 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame) Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle, this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); - // We just processed the heaer blocks to keep the dynamic table in + // We just processed the header blocks to keep the dynamic table in // sync with peer to avoid future HPACK compression errors if (reset_header_after_decoding) { stream->reset_receive_headers(); SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread()); this->stream_list.remove(stream); if (free_stream_after_decoding) { - stream->initiating_close(); + // Suppress RST_STREAM(NO_ERROR): rcv_frame() will send RST_STREAM(REFUSED_STREAM) + // for this stream, so sending NO_ERROR here would produce duplicate frames. + stream->initiating_close(/* suppress_rst= */ true); } if (this->goaway_sent && stream_id > this->last_stream_id_tx) { @@ -1127,14 +1129,16 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle, this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE)); - // We just processed the heaer blocks to keep the dynamic table in + // We just processed the header blocks to keep the dynamic table in // sync with peer to avoid future HPACK compression errors if (stream->reset_header_after_decoding) { stream->reset_receive_headers(); SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread()); this->stream_list.remove(stream); if (stream->free_stream_after_decoding) { - stream->initiating_close(); + // Suppress RST_STREAM(NO_ERROR): rcv_frame() will send RST_STREAM(REFUSED_STREAM) + // for this stream, so sending NO_ERROR here would produce duplicate frames. + stream->initiating_close(/* suppress_rst= */ true); } // After sending a GOAWAY frame, the sender can discard frames for streams initiated by the receiver with diff --git a/src/proxy/http2/Http2Stream.cc b/src/proxy/http2/Http2Stream.cc index a5cdc20b8ac..ec25a4cb347 100644 --- a/src/proxy/http2/Http2Stream.cc +++ b/src/proxy/http2/Http2Stream.cc @@ -628,7 +628,7 @@ Http2Stream::terminate_if_possible() // Initiated from the Http2 side void -Http2Stream::initiating_close() +Http2Stream::initiating_close(bool suppress_rst) { if (!closed) { SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread()); @@ -636,7 +636,8 @@ Http2Stream::initiating_close() Http2StreamDebug("initiating_close client_window=%zd session_window=%zd", _peer_rwnd, this->get_connection_state().get_peer_rwnd()); - if (!this->is_outbound_connection() && this->is_state_writeable()) { // Let the other end know we are going away + if (!suppress_rst && !this->is_outbound_connection() && + this->is_state_writeable()) { // Let the other end know we are going away this->get_connection_state().send_rst_stream_frame(_id, Http2ErrorCode::HTTP2_ERROR_NO_ERROR); } From e6954df8cf26b0c27be32764ac5a3ac6eec516e8 Mon Sep 17 00:00:00 2001 From: Katsutoshi Ikenoya Date: Fri, 13 Mar 2026 19:35:53 +0900 Subject: [PATCH 3/4] Fix the conditions for sending GOAWAY frame --- include/proxy/http2/Http2ConnectionState.h | 7 +++++++ src/proxy/http2/Http2CommonSession.cc | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/proxy/http2/Http2ConnectionState.h b/include/proxy/http2/Http2ConnectionState.h index a506de7ab63..b7bf1a93cdd 100644 --- a/include/proxy/http2/Http2ConnectionState.h +++ b/include/proxy/http2/Http2ConnectionState.h @@ -140,6 +140,7 @@ class Http2ConnectionState : public Continuation Http2StreamId get_latest_stream_id_in() const; Http2StreamId get_latest_stream_id_out() const; int get_stream_requests() const; + Http2StreamId get_last_stream_id_tx() const; bool get_goaway_sent() const; void increment_stream_requests(); bool is_peer_concurrent_stream_ub() const; @@ -453,6 +454,12 @@ Http2ConnectionState::get_goaway_sent() const return goaway_sent; } +inline Http2StreamId +Http2ConnectionState::get_last_stream_id_tx() const +{ + return last_stream_id_tx; +} + inline void Http2ConnectionState::increment_stream_requests() { diff --git a/src/proxy/http2/Http2CommonSession.cc b/src/proxy/http2/Http2CommonSession.cc index 8e07414e971..b26f4850dc9 100644 --- a/src/proxy/http2/Http2CommonSession.cc +++ b/src/proxy/http2/Http2CommonSession.cc @@ -371,7 +371,7 @@ Http2CommonSession::do_process_frame_read(int /* event ATS_UNUSED */, VIO *vio, } if (this->connection_state.get_stream_error_rate() > std::min(1.0, Http2::stream_error_rate_threshold * 2.0) && - !this->connection_state.get_goaway_sent()) { + (!this->connection_state.get_goaway_sent() || this->connection_state.get_last_stream_id_tx() == INT32_MAX)) { ip_port_text_buffer ipb; const char *peer_ip = ats_ip_ntop(this->get_proxy_session()->get_remote_addr(), ipb, sizeof(ipb)); SiteThrottledWarning("HTTP/2 session error peer_ip=%s session_id=%" PRId64 From bcebaad40f9f3bf99a7a6e8c62ea6925ba2e2201 Mon Sep 17 00:00:00 2001 From: Katsutoshi Ikenoya Date: Fri, 13 Mar 2026 16:00:36 +0900 Subject: [PATCH 4/4] Add autest for HTTP/2 POST after GOAWAY --- .../h2/clients/h2_post_after_goaway.py | 257 ++++++++++++++++++ .../h2/http2_post_after_goaway.test.py | 108 ++++++++ 2 files changed, 365 insertions(+) create mode 100644 tests/gold_tests/h2/clients/h2_post_after_goaway.py create mode 100644 tests/gold_tests/h2/http2_post_after_goaway.test.py diff --git a/tests/gold_tests/h2/clients/h2_post_after_goaway.py b/tests/gold_tests/h2/clients/h2_post_after_goaway.py new file mode 100644 index 00000000000..3e0b1506668 --- /dev/null +++ b/tests/gold_tests/h2/clients/h2_post_after_goaway.py @@ -0,0 +1,257 @@ +#!/usr/bin/env python3 +''' +HTTP/2 client that verifies DATA frames for in-flight POST requests are +processed after ATS sends a GOAWAY frame. + +stream_requests and stream_error_count in the scenario below refer to ATS +internal counters in Http2ConnectionState. + +Scenario: +1. Send 4 GET requests (stream_requests = 4). +2. Send 3 raw DATA frames to the already-closed GET streams. ATS replies with + RST_STREAM(STREAM_CLOSED) for each, incrementing stream_error_count to 3. +3. Send POST HEADERS only on a new stream, withhold the DATA body + (stream_requests = 5). With 5 total requests and 3 errors: + error_rate = 3/5 = 0.6 > min(1.0, 0.2 * 2.0) = 0.4 -> GOAWAY fires. +4. Wait for GOAWAY. +5. Send the POST DATA body *after* GOAWAY. +6. Verify that ATS returns 200 OK (not 408 Request Timeout). +''' +# 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. + +from __future__ import annotations + +import argparse +import socket +import ssl +import struct + +import h2.config +import h2.connection +import h2.events +import h2.exceptions + + +def get_socket(port: int) -> ssl.SSLSocket: + """Return a TLS socket connected to localhost:port with h2 negotiated.""" + ctx = ssl.create_default_context() + ctx.check_hostname = False + ctx.verify_mode = ssl.CERT_NONE + ctx.set_alpn_protocols(['h2']) + socket.setdefaulttimeout(5) + raw = socket.create_connection(('localhost', port)) + return ctx.wrap_socket(raw, server_hostname='localhost') + + +def send_raw_data_frame(sock: ssl.SSLSocket, stream_id: int, payload: bytes = b'\x00', end_stream: bool = False) -> None: + """Send a raw HTTP/2 DATA frame, bypassing the h2 library. + + Used to send DATA to already-closed streams so that ATS replies with + RST_STREAM(STREAM_CLOSED) and increments its stream_error_count. + """ + flags = 0x01 if end_stream else 0x00 + length = len(payload) + # Frame header: 3-byte length | 1-byte type | 1-byte flags | 4-byte stream id + header = struct.pack('>I', length)[1:] # 3-byte big-endian length + header += b'\x00' # type: DATA = 0x0 + header += bytes([flags]) + header += struct.pack('>I', stream_id & 0x7FFFFFFF) + sock.sendall(header + payload) + + +def drain_events(sock: ssl.SSLSocket, h2conn: h2.connection.H2Connection) -> list: + """Read one chunk from the socket and return the resulting h2 events.""" + data = sock.recv(65536) + if not data: + return [] + try: + events = h2conn.receive_data(data) + except h2.exceptions.ProtocolError: + events = [] + pending = h2conn.data_to_send() + if pending: + sock.sendall(pending) + return events + + +def _recv_exact(sock: ssl.SSLSocket, n: int) -> bytes | None: + """Read exactly n bytes from the socket, or return None on EOF.""" + buf = b'' + while len(buf) < n: + chunk = sock.recv(n - len(buf)) + if not chunk: + return None + buf += chunk + return buf + + +def recv_response_after_goaway(sock: ssl.SSLSocket, h2conn: h2.connection.H2Connection, stream_id: int) -> str | None: + """Receive the HTTP/2 response for stream_id by parsing raw frames. + + The h2 library raises ProtocolError when receive_data() is called after + GOAWAY has been received, even though RFC 9113 §6.8 allows the peer to + keep sending responses for streams with IDs <= last_stream_id. This + function bypasses the h2 state machine and reads frames at the wire level, + using h2's built-in HPACK decoder to keep header-compression state in sync. + + :returns: The value of the :status pseudo-header, or None on failure. + """ + status = None + stream_ended = False + frame_header_len = 9 + + while not stream_ended: + raw_hdr = _recv_exact(sock, frame_header_len) + if raw_hdr is None: + break + + payload_len = struct.unpack('>I', b'\x00' + raw_hdr[:3])[0] + frame_type = raw_hdr[3] + flags = raw_hdr[4] + frame_sid = struct.unpack('>I', raw_hdr[5:9])[0] & 0x7FFFFFFF + + payload = _recv_exact(sock, payload_len) or b'' + + if frame_sid != stream_id: + continue + + if frame_type == 0x1: # HEADERS + # Strip optional PADDED (0x08) and PRIORITY (0x20) bytes. + block = payload + if flags & 0x08: + pad_len = block[0] + block = block[1:len(block) - pad_len] + if flags & 0x20: + block = block[5:] + # Use h2's HPACK decoder so its dynamic table stays in sync. + # decoder.decode() may return (bytes, bytes) or (str, str) + # depending on the hpack library version, so normalise to str. + for name, value in h2conn.decoder.decode(block): + name_str = name.decode() if isinstance(name, bytes) else name + value_str = value.decode() if isinstance(value, bytes) else value + if name_str == ':status': + status = value_str + if flags & 0x01: # END_STREAM + stream_ended = True + elif frame_type == 0x0: # DATA + if flags & 0x01: # END_STREAM + stream_ended = True + + return status + + +def run(port: int, path: str) -> None: + tls_socket = get_socket(port) + + config = h2.config.H2Configuration(client_side=True) + h2conn = h2.connection.H2Connection(config=config) + h2conn.initiate_connection() + tls_socket.sendall(h2conn.data_to_send()) + + get_headers = [ + (':method', 'GET'), + (':path', path), + (':authority', 'localhost'), + (':scheme', 'https'), + ] + + # Step 1: Send 4 GET requests on streams 1, 3, 5, 7 (stream_requests = 4). + # stream_error_rate_threshold=0.2 requires total >= 1/0.2 = 5 before the + # rate is calculated, so we need at least one more request after these. + for stream_id in [1, 3, 5, 7]: + h2conn.send_headers(stream_id, get_headers, end_stream=True) + tls_socket.sendall(h2conn.data_to_send()) + + # Wait for all 4 GET responses to complete. + completed: set = set() + while len(completed) < 4: + for event in drain_events(tls_socket, h2conn): + if isinstance(event, h2.events.StreamEnded): + completed.add(event.stream_id) + + # Step 2: Send raw DATA frames to the already-closed streams 1, 3, and 5. + # ATS responds with RST_STREAM(STREAM_CLOSED) for each, incrementing + # stream_error_count to 3. + for closed_stream_id in [1, 3, 5]: + send_raw_data_frame(tls_socket, closed_stream_id) + + # Step 3: Start a POST on stream 9 – send HEADERS only, withhold DATA + # (stream_requests = 5). The body will be sent after GOAWAY. + post_headers = [ + (':method', 'POST'), + (':path', path), + (':authority', 'localhost'), + (':scheme', 'https'), + ('content-length', '4'), + ] + last_stream_id = 9 + h2conn.send_headers(last_stream_id, post_headers, end_stream=False) + tls_socket.sendall(h2conn.data_to_send()) + + # Step 4: Wait for GOAWAY with last_stream_id covering the POST stream. + goaway_received = False + goaway_error_code = None + try: + while not goaway_received: + for event in drain_events(tls_socket, h2conn): + if isinstance(event, h2.events.ConnectionTerminated) and event.last_stream_id == last_stream_id: + goaway_received = True + goaway_error_code = event.error_code + except socket.timeout: + print(f"ERROR: Timed out waiting for GOAWAY with last_stream_id={last_stream_id}") + exit(1) + + print(f"GOAWAY received with error_code={goaway_error_code}") + + # Step 5: Send the POST DATA body after GOAWAY. Before the fix, ATS would + # stop reading frames after GOAWAY, causing this DATA frame to be ignored + # and the request to time out with 408. + body = b'body' + try: + h2conn.send_data(last_stream_id, body, end_stream=True) + pending = h2conn.data_to_send() + if pending: + tls_socket.sendall(pending) + except h2.exceptions.ProtocolError: + # The h2 library rejects sends after GOAWAY; fall back to raw bytes. + send_raw_data_frame(tls_socket, last_stream_id, body, end_stream=True) + + # Step 6: Receive the POST response on the POST stream by parsing raw frames. + # h2's receive_data() raises ProtocolError after GOAWAY, so we bypass the + # h2 state machine and decode headers with h2's built-in HPACK decoder. + response_status = recv_response_after_goaway(tls_socket, h2conn, last_stream_id) + + if response_status == '200': + print("SUCCESS: POST request completed with 200 OK after GOAWAY") + else: + print(f"ERROR: Expected 200 OK, got status={response_status}") + exit(1) + + tls_socket.close() + + +def main(): + parser = argparse.ArgumentParser(description='Test in-flight POST DATA handling after GOAWAY.') + parser.add_argument('port', type=int, help='ATS TLS port') + parser.add_argument('path', help='Request path (e.g. /test)') + args = parser.parse_args() + run(args.port, args.path) + + +if __name__ == '__main__': + main() diff --git a/tests/gold_tests/h2/http2_post_after_goaway.test.py b/tests/gold_tests/h2/http2_post_after_goaway.test.py new file mode 100644 index 00000000000..a6df8068984 --- /dev/null +++ b/tests/gold_tests/h2/http2_post_after_goaway.test.py @@ -0,0 +1,108 @@ +# 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 sys + +Test.Summary = ''' +Verify that in-flight POST DATA frames are not dropped after ATS sends +GOAWAY due to a high stream error rate. + +Regression test for: DATA frames for streams initiated before GOAWAY were +incorrectly ignored because the frame-reading loop exited on any non-zero +tx_error_code, including ENHANCE_YOUR_CALM. This caused in-flight POST +requests to time out with 408. +''' + + +class Http2PostAfterGoawayTest: + _path = '/test' + + def __init__(self): + self.__setupOriginServer() + self.__setupTS() + self.__setupClient() + + def __setupOriginServer(self): + self._server = Test.MakeOriginServer("server") + + get_request = { + "headers": f"GET {self._path} HTTP/1.1\r\nHost: localhost\r\n\r\n", + "timestamp": "1469733493.993", + "body": "" + } + get_response = { + "headers": "HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + "timestamp": "1469733493.993", + "body": "" + } + # Register 4 GET responses (one per stream). + for _ in range(4): + self._server.addResponse("sessionlog.json", get_request, get_response) + + post_request = { + "headers": f"POST {self._path} HTTP/1.1\r\nHost: localhost\r\nContent-Length: 4\r\n\r\n", + "timestamp": "1469733493.993", + "body": "body" + } + post_response = { + "headers": "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n", + "timestamp": "1469733493.993", + "body": "ok" + } + self._server.addResponse("sessionlog.json", post_request, post_response) + + def __setupTS(self): + self._ts = Test.MakeATSProcess("ts", enable_tls=True, enable_cache=True) + self._ts.addDefaultSSLFiles() + self._ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.http.cache.http': 0, + 'proxy.config.http.transaction_no_activity_timeout_in': 3, + 'proxy.config.ssl.server.cert.path': f"{self._ts.Variables.SSLDir}", + 'proxy.config.ssl.server.private_key.path': f"{self._ts.Variables.SSLDir}", + # Lower thresholds so GOAWAY(ENHANCE_YOUR_CALM) fires after + # a small number of stream errors: + # total >= 1/0.2 = 5 requests required + # GOAWAY when error_rate > min(1.0, 0.2*2) = 0.4 + # 3 errors / 5 total = 0.6 > 0.4 -> GOAWAY + 'proxy.config.http2.stream_error_rate_threshold': 0.2, + 'proxy.config.http2.stream_error_sampling_threshold': 1, + }) + self._ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.Port}") + self._ts.Disk.ssl_multicert_config.AddLine('dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key') + + def __setupClient(self): + self._ts.Setup.CopyAs("clients/h2_post_after_goaway.py", Test.RunDirectory) + + def run(self): + tr = Test.AddTestRun() + tr.Processes.Default.StartBefore(self._ts) + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.Command = (f"{sys.executable} h2_post_after_goaway.py" + f" {self._ts.Variables.ssl_port} {self._path}") + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "GOAWAY received with error_code=", "ATS must send GOAWAY before the POST completes") + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "SUCCESS", "POST request must complete with 200 OK after GOAWAY") + tr.StillRunningAfter = self._ts + tr.StillRunningAfter = self._server + tr.TimeOut = 5 + + +Http2PostAfterGoawayTest().run()