From d2127b4cfc98357c199693dc1354a7172ebe6957 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Tue, 28 Jul 2026 20:23:11 -0500 Subject: [PATCH] HTTP/2 to origin session pool and reliability fixes Long-lived HTTP/2 origin connections could remain discoverable after entering half-close, become unreachable when global pooling was configured, or be poisoned by a single request's Connection: close header. Requests assigned to those sessions could fail immediately. Valid responses could also be rejected when headers used CONTINUATION, when their payload length followed HEAD or 304 rules, or when normal SETTINGS acknowledgments crossed the receive-rate limit. This makes outbound HTTP/2 session lifecycle consistently thread-local. It always probes the thread pool before configured global pools, ignores Connection: close for outbound session shutdown, refuses to re-pool half-closed sessions, and evicts sessions as soon as they enter local half-close. HTTP/1.x global pooling behavior remains unchanged. This also carries the protocol's not-processed guarantee from GOAWAY and RST_STREAM REFUSED_STREAM into retry selection. It permits safe replay of non-idempotent requests and rearms a fully buffered request body before a retry. SETTINGS acknowledgments are excluded from the peer abuse limit because their rate is bounded by settings sent by Traffic Server. This accepts CONTINUATION frames in valid outbound stream states without creating a second transaction and preserves request metadata needed to validate HEAD and conditional 304 responses. Replay-based AuTests and custom HTTP/2 origins cover session reuse, half-close handling, retries, split response headers, payload validation, and SETTINGS accounting. --- doc/admin-guide/files/records.yaml.en.rst | 7 + include/proxy/ProxyTransaction.h | 16 ++ include/proxy/http2/Http2CommonSession.h | 16 +- include/proxy/http2/Http2ServerSession.h | 14 ++ include/proxy/http2/Http2Stream.h | 82 +++++++- src/proxy/ProxyTransaction.cc | 6 + src/proxy/http/HttpSM.cc | 21 +- src/proxy/http/HttpSessionManager.cc | 22 ++- src/proxy/http2/Http2ConnectionState.cc | 116 +++++++++-- src/proxy/http2/Http2ServerSession.cc | 21 ++ src/proxy/http2/Http2Stream.cc | 45 ++++- tests/gold_tests/h2/continuation_origin.py | 141 +++++++++++++ .../h2/gold/h2-settings-ack-metrics.gold | 2 + .../h2/gold/h2o-pool-reuse-metrics.gold | 2 + .../h2/h2_settings_ack_not_counted.test.py | 100 ++++++++++ .../h2/h2_to_origin_continuation.test.py | 104 ++++++++++ .../h2_to_origin_payload_validation.test.py | 75 +++++++ .../h2/h2_to_origin_pool_reuse.test.py | 106 ++++++++++ .../h2/h2_to_origin_safe_retry.test.py | 115 +++++++++++ .../settings_ack.replay.yaml | 187 ++++++++++++++++++ .../continuation.replay.yaml | 134 +++++++++++++ .../payload_validation.replay.yaml | 146 ++++++++++++++ .../pool_reuse.replay.yaml | 173 ++++++++++++++++ .../safe_retry.replay.yaml | 67 +++++++ tests/gold_tests/h2/safe_retry_origin.py | 163 +++++++++++++++ 25 files changed, 1828 insertions(+), 53 deletions(-) create mode 100644 tests/gold_tests/h2/continuation_origin.py create mode 100644 tests/gold_tests/h2/gold/h2-settings-ack-metrics.gold create mode 100644 tests/gold_tests/h2/gold/h2o-pool-reuse-metrics.gold create mode 100644 tests/gold_tests/h2/h2_settings_ack_not_counted.test.py create mode 100644 tests/gold_tests/h2/h2_to_origin_continuation.test.py create mode 100644 tests/gold_tests/h2/h2_to_origin_payload_validation.test.py create mode 100644 tests/gold_tests/h2/h2_to_origin_pool_reuse.test.py create mode 100644 tests/gold_tests/h2/h2_to_origin_safe_retry.test.py create mode 100644 tests/gold_tests/h2/replay_h2_settings_ack/settings_ack.replay.yaml create mode 100644 tests/gold_tests/h2/replay_h2o_continuation/continuation.replay.yaml create mode 100644 tests/gold_tests/h2/replay_h2o_payload_validation/payload_validation.replay.yaml create mode 100644 tests/gold_tests/h2/replay_h2o_pool_reuse/pool_reuse.replay.yaml create mode 100644 tests/gold_tests/h2/replay_h2o_safe_retry/safe_retry.replay.yaml create mode 100644 tests/gold_tests/h2/safe_retry_origin.py diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index f14b03b5449..2aa0617e5da 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -5102,6 +5102,13 @@ HTTP/2 Configuration code of ENHANCE_YOUR_CALM. Any negative value configures no limit to the number of SETTINGS frames received. + SETTINGS frames carrying the ACK flag are not counted against this limit. + They are mandatory protocol responses to SETTINGS frames |TS| sent and + therefore cannot be used by a peer to flood |TS|; counting them would + spuriously close healthy connections (in particular outbound HTTP/2 + sessions where :ts:cv:`proxy.config.http2.flow_control.policy_out` causes + |TS| to send a SETTINGS frame per outbound stream). + .. ts:cv:: CONFIG proxy.config.http2.max_ping_frames_per_minute INT 60 :reloadable: diff --git a/include/proxy/ProxyTransaction.h b/include/proxy/ProxyTransaction.h index 7665392ec50..1b2b010747d 100644 --- a/include/proxy/ProxyTransaction.h +++ b/include/proxy/ProxyTransaction.h @@ -146,6 +146,22 @@ class ProxyTransaction : public VConnection virtual void set_rx_error_code(ProxyError e); virtual void set_tx_error_code(ProxyError e); + /** Whether the request on this transaction is known by the protocol layer to + * be safe to retry on a fresh origin connection. + * + * The default is @c false. A subclass should return @c true only when the + * underlying protocol guarantees the origin did not process (and could not + * have observed) the request -- for example, when an HTTP/2 origin sends a + * GOAWAY whose last_stream_id is below this transaction's stream id, or + * sends a RST_STREAM with the REFUSED_STREAM error code (RFC 9113 6.8 and + * 8.7). HttpSM uses this to allow retrying non-idempotent methods that + * would otherwise be considered too risky to replay. + * + * @return @c true if HttpSM may safely retry the request on a different + * origin connection regardless of method. + */ + virtual bool is_safe_to_retry() const; + bool support_sni() const; void mark_as_tunnel_endpoint() override; diff --git a/include/proxy/http2/Http2CommonSession.h b/include/proxy/http2/Http2CommonSession.h index 0b1d9b8112b..bafd9dbd273 100644 --- a/include/proxy/http2/Http2CommonSession.h +++ b/include/proxy/http2/Http2CommonSession.h @@ -101,14 +101,14 @@ class Http2CommonSession //////////////////// // Accessors - void set_dying_event(int event); - int get_dying_event() const; - bool ready_to_free() const; - bool is_recursing() const; - void set_half_close_local_flag(bool flag); - bool get_half_close_local_flag() const; - bool is_url_pushed(const char *url, int url_len); - void add_url_to_pushed_table(const char *url, int url_len); + void set_dying_event(int event); + int get_dying_event() const; + bool ready_to_free() const; + bool is_recursing() const; + virtual void set_half_close_local_flag(bool flag); + bool get_half_close_local_flag() const; + bool is_url_pushed(const char *url, int url_len); + void add_url_to_pushed_table(const char *url, int url_len); // Record history from Http2ConnectionState void remember(const SourceLocation &location, int event, int reentrant = NO_REENTRANT); diff --git a/include/proxy/http2/Http2ServerSession.h b/include/proxy/http2/Http2ServerSession.h index 5b98c78e8e9..07cf3884673 100644 --- a/include/proxy/http2/Http2ServerSession.h +++ b/include/proxy/http2/Http2ServerSession.h @@ -58,6 +58,20 @@ class Http2ServerSession : public PoolableSession, public Http2CommonSession void add_session() override; void remove_session(); + /** Eagerly drop a half-closed session out of the server-session pool. + * + * Sets the half-close-local flag (delegating to the base class) and, when + * the flag transitions to @c true, immediately removes this session from + * the per-thread server-session pool. Once @c half_close_local is set, + * every subsequent @c create_initiating_stream on this session + * short-circuits with @c REFUSED_STREAM, so leaving it discoverable in + * the pool produces a stream of REFUSED_STREAM aborts on otherwise + * unrelated requests until the session is finally torn down. The pool + * must be made aware as soon as the half-close decision is made, not at + * session destruction time. + */ + void set_half_close_local_flag(bool flag) override; + //////////////////// // Accessors sockaddr const *get_remote_addr() const override; diff --git a/include/proxy/http2/Http2Stream.h b/include/proxy/http2/Http2Stream.h index dbf64edb4b9..953c279f9f9 100644 --- a/include/proxy/http2/Http2Stream.h +++ b/include/proxy/http2/Http2Stream.h @@ -156,6 +156,19 @@ class Http2Stream : public ProxyTransaction void set_rx_error_code(ProxyError e) override; void set_tx_error_code(ProxyError e) override; + bool is_safe_to_retry() const override; + + /** Mark this stream as known-not-processed by the origin. + * + * Called by Http2ConnectionState when the origin has explicitly indicated + * that this stream's request was not (and will not be) processed -- either + * because a GOAWAY arrived whose @c last_stream_id is below this stream's + * id, or because a RST_STREAM with REFUSED_STREAM was received for it (RFC + * 9113 sections 6.8 and 8.7). HttpSM consults @c is_safe_to_retry() when + * deciding whether a non-idempotent request may be retried. + */ + void set_safe_to_retry(); + bool has_request_body(int64_t content_length, bool is_chunked_set) const override; HTTPVersion get_version(HTTPHdr &hdr) const override; @@ -163,6 +176,7 @@ class Http2Stream : public ProxyTransaction void increment_data_length(uint64_t length); bool payload_length_is_valid() const; + void cache_send_request_for_response_validation(); bool is_write_vio_done() const; void update_sent_count(unsigned num_bytes); Http2StreamId get_id() const; @@ -260,6 +274,13 @@ class Http2Stream : public ProxyTransaction /** Whether the stream has been registered with the connection state. */ bool _registered_stream = true; + // Set by Http2ConnectionState when the origin has explicitly indicated + // (via GOAWAY whose last_stream_id is below this stream's id, or via + // RST_STREAM with REFUSED_STREAM) that this stream's request was not + // processed and may be safely retried even for non-idempotent methods. + // See Http2Stream::is_safe_to_retry / set_safe_to_retry. + bool _safe_to_retry = false; + // A brief discussion of similar flags and state variables: _state, closed, terminate_stream // // _state tracks the HTTP2 state of the stream. This field completely coincides with the H2 spec. @@ -286,6 +307,17 @@ class Http2Stream : public ProxyTransaction uint64_t data_length = 0; uint64_t bytes_sent = 0; + // Snapshot of the send-side request taken before `_send_header` is destroyed + // in `update_write_request`. These are used by `payload_length_is_valid` to + // apply the [RFC 9110] 8.6 / [RFC 7230] 3.3.2 payload preclusion rules to + // origin responses on outbound streams. Without this snapshot the request + // method (e.g. HEAD) and the presence of conditional request headers would + // already be lost by the time the response is validated, causing valid HEAD + // and 304 responses with non-zero Content-Length to be rejected as protocol + // errors. + int _cached_send_method_wksidx = -1; + uint64_t _cached_send_conditional_field = 0; + ssize_t _peer_rwnd = 0; ssize_t _local_rwnd = 0; @@ -398,18 +430,50 @@ Http2Stream::increment_data_length(uint64_t length) data_length += length; } +inline void +Http2Stream::cache_send_request_for_response_validation() +{ + // On outbound streams `_send_header` is destroyed in `update_write_request` + // immediately after the request HEADERS frame is encoded and sent. Capture + // the request method and the presence of any conditional request headers + // here so that the response-side `payload_length_is_valid` check can still + // honor the [RFC 9110] 8.6 payload preclusion rules for HEAD responses and + // for 304 responses to conditional GETs. + if (!this->is_outbound_connection() || !_send_header.valid() || _send_header.type_get() != HTTPType::REQUEST) { + return; + } + uint64_t const conditional_mask = (MIME_PRESENCE_IF_UNMODIFIED_SINCE | MIME_PRESENCE_IF_MODIFIED_SINCE | MIME_PRESENCE_IF_RANGE | + MIME_PRESENCE_IF_MATCH | MIME_PRESENCE_IF_NONE_MATCH); + _cached_send_method_wksidx = _send_header.method_get_wksidx(); + _cached_send_conditional_field = _send_header.presence(conditional_mask); +} + inline bool Http2Stream::payload_length_is_valid() const { - uint32_t content_length = _receive_header.get_content_length(); - uint64_t mask = (MIME_PRESENCE_IF_UNMODIFIED_SINCE | MIME_PRESENCE_IF_MODIFIED_SINCE | MIME_PRESENCE_IF_RANGE | - MIME_PRESENCE_IF_MATCH | MIME_PRESENCE_IF_NONE_MATCH); - - // 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)); + uint32_t const content_length = _receive_header.get_content_length(); + + // Apply the [RFC 9110] 8.6 / [RFC 7230] 3.3.2 payload preclusion rules to + // origin responses on outbound streams. The send-side `_send_header` may + // already have been torn down by this point, so consult the cached + // request metadata captured by `cache_send_request_for_response_validation`. + bool is_payload_precluded = false; + if (this->is_outbound_connection()) { + if (_cached_send_method_wksidx == HTTP_WKSIDX_HEAD) { + is_payload_precluded = true; + } else if (_cached_send_method_wksidx == HTTP_WKSIDX_GET && _cached_send_conditional_field != 0) { + // `HTTPHdr::status_get()` asserts the underlying header has response + // polarity, but on the outbound origin-response path `_receive_header` + // is still in HTTP/2 form at this point and has not yet been converted, + // so the polarity may not be set. Read the `:status` pseudo-header + // directly to detect a 304 response to a conditional GET. + if (MIMEField const *const status_field = _receive_header.field_find(PSEUDO_HEADER_STATUS); status_field != nullptr) { + auto const sv{status_field->value_get()}; + HTTPStatus const status = http_parse_status(sv.data(), sv.data() + sv.length()); + is_payload_precluded = (status == 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/ProxyTransaction.cc b/src/proxy/ProxyTransaction.cc index 05fb0c8877e..6993c5cdc8f 100644 --- a/src/proxy/ProxyTransaction.cc +++ b/src/proxy/ProxyTransaction.cc @@ -95,6 +95,12 @@ ProxyTransaction::set_tx_error_code(ProxyError e) } } +bool +ProxyTransaction::is_safe_to_retry() const +{ + return false; +} + NetVConnection * ProxyTransaction::get_netvc() const { diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 799f72b771a..8d3c9be5459 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -6430,7 +6430,26 @@ HttpSM::handle_server_setup_error(int event, void *data) [[maybe_unused]] UnixNetVConnection *dbg_vc = nullptr; switch (event) { case VC_EVENT_EOS: - t_state.current.state = HttpTransact::CONNECTION_CLOSED; + // If the underlying transport (e.g. an HTTP/2 stream) has signaled that + // the origin guaranteed it never processed this request -- because of a + // GOAWAY whose last_stream_id is below this stream's id, or a RST_STREAM + // with REFUSED_STREAM (RFC 9113 6.8 and 8.7) -- treat the failure as a + // connection-level error so HttpTransact::is_request_retryable allows + // retrying even non-idempotent methods such as POST. Otherwise this + // would surface to the client as ERR_CLIENT_ABORT despite the request + // being explicitly safe to replay on a fresh connection. + if (server_txn != nullptr && server_txn->is_safe_to_retry()) { + t_state.current.state = HttpTransact::CONNECTION_ERROR; + // A retry with a request body needs to use the complete copy retained + // by request buffering. The first origin attempt clears + // is_buffering_request_body after consuming that copy, so restore the + // flag before setting up the retry's request-body tunnel. + if (this->is_postbuf_valid() && this->get_postbuf_done()) { + is_buffering_request_body = true; + } + } else { + t_state.current.state = HttpTransact::CONNECTION_CLOSED; + } t_state.set_connect_fail(EPIPE); break; case VC_EVENT_ERROR: diff --git a/src/proxy/http/HttpSessionManager.cc b/src/proxy/http/HttpSessionManager.cc index 8827f60c2eb..9a4f4dce5f4 100644 --- a/src/proxy/http/HttpSessionManager.cc +++ b/src/proxy/http/HttpSessionManager.cc @@ -420,19 +420,27 @@ HttpSessionManager::acquire_session(HttpSM *sm, sockaddr const *ip, const char * to_return = nullptr; } - // Otherwise, check the thread pool first - if (this->get_pool_type() == TS_SERVER_SESSION_SHARING_POOL_THREAD || - this->get_pool_type() == TS_SERVER_SESSION_SHARING_POOL_HYBRID) { - retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_THREAD); - } + // Always check the thread-local pool first. Multiplexing server sessions + // (HTTP/2, HTTP/3) cannot be safely shared across threads -- their state is + // owned by the EThread that drives their connection -- so they are filed + // exclusively in the per-thread pool by `Http2ServerSession::add_session` + // (and analogous code for HTTP/3). If the configured pool type is `global` + // or `global_locked`, only the global pool would otherwise be consulted, + // which means an existing H/2 origin connection on this thread is invisible + // to the lookup. Each new request would then open a fresh TCP+TLS+H/2 + // handshake to the origin, defeating multiplexing entirely. Trying the + // thread-local pool first restores within-thread H/2 origin reuse without + // changing behavior for HTTP/1.x sessions, which fall through to the + // configured pool below on a thread-local miss. + retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_THREAD); - // If you didn't get a match, and the global pool is an option go there. if (retval != HSMresult_t::DONE) { if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL == this->get_pool_type() || TS_SERVER_SESSION_SHARING_POOL_HYBRID == this->get_pool_type()) { retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_GLOBAL); - } else if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED == this->get_pool_type()) + } else if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED == this->get_pool_type()) { retval = _acquire_session(ip, hostname_hash, sm, match_style, TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED); + } } return retval; diff --git a/src/proxy/http2/Http2ConnectionState.cc b/src/proxy/http2/Http2ConnectionState.cc index cb448000145..cae861f9b2f 100644 --- a/src/proxy/http2/Http2ConnectionState.cc +++ b/src/proxy/http2/Http2ConnectionState.cc @@ -710,6 +710,16 @@ Http2ConnectionState::rcv_rst_stream_frame(const Http2Frame &frame) Http2StreamDebug(this->session, stream_id, "Parsed RST_STREAM frame: Error Code: %u", rst_stream.error_code); ATS_PROBE3(http2_rst_stream_rcvd, this->session->get_connection_id(), stream_id, rst_stream.error_code); stream->set_rx_error_code({ProxyErrorClass::TXN, static_cast(rst_stream.error_code)}); + // Per RFC 9113 8.7: REFUSED_STREAM is the one stream-level error code with + // an explicit guarantee that the request was not processed by the peer, + // and so the request is safe to retry on a fresh connection -- including + // for non-idempotent methods. Tag the stream so HttpSM converts the + // resulting EOS into a connection-level retry rather than surfacing it as + // ERR_CLIENT_ABORT to the client. + if (this->session->is_outbound() && + static_cast(rst_stream.error_code) == Http2ErrorCode::HTTP2_ERROR_REFUSED_STREAM) { + stream->set_safe_to_retry(); + } stream->initiating_close(); } @@ -730,18 +740,6 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) Warning("Setting frame for zombied session %" PRId64, this->session->get_connection_id()); } - // Update SETTINGS frame count per minute - this->increment_received_settings_frame_count(); - // Close this connection if its SETTINGS frame count exceeds a limit - if (configured_max_settings_frames_per_minute >= 0 && - this->get_received_settings_frame_count() > static_cast(configured_max_settings_frames_per_minute)) { - Metrics::Counter::increment(http2_rsb.max_settings_frames_per_minute_exceeded); - Http2StreamDebug(this->session, stream_id, "Observed too frequent SETTINGS frames: %u frames within a last minute", - this->get_received_settings_frame_count()); - return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM, - "recv settings too frequent SETTINGS frames"); - } - // [RFC 7540] 6.5. The stream identifier for a SETTINGS frame MUST be zero. // If an endpoint receives a SETTINGS frame whose stream identifier field is // anything other than 0x0, the endpoint MUST respond with a connection @@ -754,6 +752,21 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) // [RFC 7540] 6.5. Receipt of a SETTINGS frame with the ACK flag set and a // length field value other than 0 MUST be treated as a connection // error of type FRAME_SIZE_ERROR. + // + // SETTINGS-ACK frames are intentionally not counted against + // `max_settings_frames_per_minute`. The rate limit exists to defend + // against a peer flooding us with SETTINGS updates (RFC 9113 6.5: each + // received SETTINGS forces us to apply state changes and queue an ACK), + // but a SETTINGS-ACK is a mandatory protocol response to a SETTINGS we + // ourselves sent and cannot arrive faster than our own send rate. With + // `proxy.config.http2.flow_control.policy_out=2` + // (LARGE_SESSION_AND_DYNAMIC_STREAM) ATS sends a SETTINGS frame at the + // start of every outbound stream, and the origin returns one ACK per + // stream; counting those inbound ACKs against the receive limit can trip + // the limit on a healthy origin connection within seconds and tear down + // a perfectly good multiplexed session with ENHANCE_YOUR_CALM. nghttp2 + // applies the analogous defense (`max_outbound_ack`) to the *outbound* + // ACK queue depth, not to the inbound ACK count, for the same reason. if (frame.header().flags & HTTP2_FLAGS_SETTINGS_ACK) { if (frame.header().length == 0) { return this->_process_incoming_settings_ack_frame(); @@ -763,6 +776,18 @@ Http2ConnectionState::rcv_settings_frame(const Http2Frame &frame) } } + // Update SETTINGS frame count per minute (non-ACK only; see above). + this->increment_received_settings_frame_count(); + // Close this connection if its SETTINGS frame count exceeds a limit. + if (configured_max_settings_frames_per_minute >= 0 && + this->get_received_settings_frame_count() > static_cast(configured_max_settings_frames_per_minute)) { + Metrics::Counter::increment(http2_rsb.max_settings_frames_per_minute_exceeded); + Http2StreamDebug(this->session, stream_id, "Observed too frequent SETTINGS frames: %u frames within a last minute", + this->get_received_settings_frame_count()); + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM, + "recv settings too frequent SETTINGS frames"); + } + // A SETTINGS frame with a length other than a multiple of 6 octets MUST // be treated as a connection error (Section 5.4.1) of type // FRAME_SIZE_ERROR. @@ -918,6 +943,24 @@ Http2ConnectionState::rcv_goaway_frame(const Http2Frame &frame) Http2StreamDebug(this->session, stream_id, "GOAWAY: last stream id=%d, error code=%d", goaway.last_streamid, static_cast(goaway.error_code)); + // Per RFC 9113 6.8: streams whose id is greater than `last_streamid` were + // not (and will not be) processed by the peer, and the requests they carry + // may be safely retried on a fresh connection. On an outbound H/2 session + // the streams in question are the ones we initiated toward the origin + // (peer-initiated-by-us), which use the "client" stream id space. Tagging + // them here -- before do_io_close() tears the streams down -- lets HttpSM + // decide to retry non-idempotent requests (e.g. POST) that would otherwise + // surface to the client as ERR_CLIENT_ABORT. This is especially important + // for AWS-style origin load balancers that aggressively send + // GOAWAY(last_stream_id=0, NO_ERROR) when draining a connection. + if (this->session->is_outbound()) { + for (Http2Stream *s = stream_list.head; s != nullptr; s = static_cast(s->link.next)) { + if (http2_is_client_streamid(s->get_id()) && s->get_id() > goaway.last_streamid) { + s->set_safe_to_retry(); + } + } + } + this->rx_error_code = {ProxyErrorClass::SSN, static_cast(goaway.error_code)}; this->session->get_proxy_session()->do_io_close(); @@ -1069,12 +1112,26 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) "continuation stream freed with invalid id"); } } else { + bool const is_outbound = this->session->is_outbound(); switch (stream->get_state()) { case Http2StreamState::HTTP2_STREAM_STATE_HALF_CLOSED_REMOTE: return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_STREAM_CLOSED, "continuation half close remote"); case Http2StreamState::HTTP2_STREAM_STATE_IDLE: break; + case Http2StreamState::HTTP2_STREAM_STATE_OPEN: + case Http2StreamState::HTTP2_STREAM_STATE_HALF_CLOSED_LOCAL: + // On outbound (origin-side) connections, response HEADERS may be split + // across CONTINUATION frames. The associated stream is OPEN if our + // request body is still in flight, or HALF_CLOSED_LOCAL once we have + // sent the request with END_STREAM (e.g. for GET or HEAD). [RFC 7540] + // 6.10 only forbids interleaving CONTINUATION with frames of other + // types or other streams, not these stream states. + if (!is_outbound) { + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR, + "continuation bad state"); + } + break; default: return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR, "continuation bad state"); @@ -1159,13 +1216,24 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) } // Set up the State Machine - SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread()); - stream->mark_milestone(Http2StreamMilestone::START_TXN); - // This should be fine, need to verify whether we need to replace this with the - // "from_early_data" flag from the associated HEADERS frame. - stream->new_transaction(frame.is_from_early_data()); - // Send request header to SM - stream->send_headers(*this); + if (!stream->is_outbound_connection() && !stream->trailing_header_is_possible()) { + SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread()); + stream->mark_milestone(Http2StreamMilestone::START_TXN); + stream->cancel_active_timeout(); + stream->new_transaction(frame.is_from_early_data()); + // Send request header to SM + stream->send_headers(*this); + } else { + // If this is a trailer, first signal to the SM that the body is done + if (stream->trailing_header_is_possible()) { + stream->set_expect_receive_trailer(); + // Propagate the trailer header + stream->send_headers(*this); + } else { + // Propagate the response + stream->send_headers(*this); + } + } // Give a chance to send response before reading next frame. this->session->interrupt_reading_frames(); } else { @@ -2117,7 +2185,15 @@ Http2ConnectionState::delete_stream(Http2Stream *stream) if (http2_is_client_streamid(stream->get_id())) { ink_release_assert(peer_streams_count_in > 0); --peer_streams_count_in; - if (!fini_received && is_peer_concurrent_stream_lb()) { + // Do not put a session that has already entered local half-close back in + // the pool. Once `set_half_close_local_flag(true)` has been called (for + // example because we have started a graceful GOAWAY) every subsequent + // `create_initiating_stream` on this session will fast-fail with + // REFUSED_STREAM, so handing it back out via `acquire_session` only + // causes spurious aborts. The session will be torn down once its + // remaining in-flight streams finish; until then it must stay out of + // the pool. + if (!fini_received && !session->get_half_close_local_flag() && is_peer_concurrent_stream_lb()) { session->add_session(); } } else { diff --git a/src/proxy/http2/Http2ServerSession.cc b/src/proxy/http2/Http2ServerSession.cc index e3e550e1bf9..075127ed86b 100644 --- a/src/proxy/http2/Http2ServerSession.cc +++ b/src/proxy/http2/Http2ServerSession.cc @@ -375,6 +375,27 @@ Http2ServerSession::remove_session() } } +void +Http2ServerSession::set_half_close_local_flag(bool flag) +{ + // Detect the OFF -> ON transition before delegating to the base, which + // is what flips the underlying flag. + bool const transitioning_to_half_close = !this->get_half_close_local_flag() && flag; + Http2CommonSession::set_half_close_local_flag(flag); + if (transitioning_to_half_close) { + // Once `half_close_local` is set, `create_initiating_stream` on this + // session short-circuits to REFUSED_STREAM. Any future + // `acquire_session` lookup that finds this session in the pool will + // therefore hand back a session that immediately fails the next + // origin request with `HTTP/2 stream error code=0x07 refused to + // create new stream, because session is in half_close state`. Evict + // it now so the next request opens (or matches) a healthy session + // instead. The session itself stays alive long enough for any + // already-attached transactions to drain. + this->remove_session(); + } +} + bool Http2ServerSession::is_multiplexing() const { diff --git a/src/proxy/http2/Http2Stream.cc b/src/proxy/http2/Http2Stream.cc index 6be67db03b9..23fd89fe255 100644 --- a/src/proxy/http2/Http2Stream.cc +++ b/src/proxy/http2/Http2Stream.cc @@ -867,14 +867,25 @@ Http2Stream::update_write_request(bool call_update) this->parsing_header_done = true; Http2StreamDebug("update_write_request parsing done, read %d bytes", bytes_used); - // Schedule session shutdown if response header has "Connection: close" - MIMEField *field = this->_send_header.field_find(static_cast(MIME_FIELD_CONNECTION)); - if (field) { - auto value{field->value_get()}; - if (value == static_cast(HTTP_VALUE_CLOSE)) { - SCOPED_MUTEX_LOCK(lock, _proxy_ssn->mutex, this_ethread()); - if (connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) { - connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED, Http2ErrorCode::HTTP2_ERROR_NO_ERROR); + // Only honor `Connection: close` -> graceful-shutdown on the inbound + // (client-facing) H/2 session, where the header lives on a response we + // are about to send back to the client and is a per-client signal to + // drain the H/2 client session. On an outbound (origin-facing) H/2 + // session the same parsing path runs against the outgoing *request*, + // and a single client request carrying `Connection: close` must not + // be allowed to graceful-shutdown a shared H/2 origin connection that + // is multiplexing many other clients' transactions. (`Connection` is + // also a hop-by-hop header forbidden on the H/2 wire by RFC 9113 + // 8.2.2, so it cannot represent an end-to-end origin signal here.) + if (!this->is_outbound_connection()) { + MIMEField *field = this->_send_header.field_find(static_cast(MIME_FIELD_CONNECTION)); + if (field) { + auto value{field->value_get()}; + if (value == static_cast(HTTP_VALUE_CLOSE)) { + SCOPED_MUTEX_LOCK(lock, _proxy_ssn->mutex, this_ethread()); + if (connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) { + connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED, Http2ErrorCode::HTTP2_ERROR_NO_ERROR); + } } } } @@ -890,6 +901,12 @@ Http2Stream::update_write_request(bool call_update) this->parsing_header_done = false; } if (this->is_outbound_connection() || this->_send_header.expect_final_response()) { + // The send-side request header is about to be torn down on outbound + // streams, so snapshot the request method and any conditional-header + // presence first. The snapshot is needed later by + // `payload_length_is_valid` to apply [RFC 9110] 8.6 payload preclusion + // for HEAD responses and 304 responses to conditional GETs. + this->cache_send_request_for_response_validation(); _send_header.destroy(); _send_header.create(this->is_outbound_connection() ? HTTPType::REQUEST : HTTPType::RESPONSE, HTTP_2_0); http_parser_clear(&http_parser); @@ -1320,6 +1337,18 @@ Http2Stream::set_tx_error_code(ProxyError e) } } +bool +Http2Stream::is_safe_to_retry() const +{ + return _safe_to_retry; +} + +void +Http2Stream::set_safe_to_retry() +{ + _safe_to_retry = true; +} + HTTPVersion Http2Stream::get_version(HTTPHdr & /* hdr ATS_UNUSED */) const { diff --git a/tests/gold_tests/h2/continuation_origin.py b/tests/gold_tests/h2/continuation_origin.py new file mode 100644 index 00000000000..88bb1306c0b --- /dev/null +++ b/tests/gold_tests/h2/continuation_origin.py @@ -0,0 +1,141 @@ +#!/usr/bin/env python3 +"""Serve HTTP/2 responses whose header blocks require CONTINUATION frames.""" + +# 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 argparse +import socket +import ssl +import sys + +from h2.config import H2Configuration +from h2.connection import H2Connection +from h2.events import DataReceived, StreamEnded + +HTTP2_FRAME_HEADER_SIZE = 9 +HTTP2_FRAME_TYPE_CONTINUATION = 0x09 + + +def count_frames(payload: bytes, frame_type: int) -> int: + """Count frames of ``frame_type`` in an HTTP/2 wire payload.""" + count = 0 + offset = 0 + while offset < len(payload): + if len(payload) - offset < HTTP2_FRAME_HEADER_SIZE: + raise RuntimeError("truncated HTTP/2 frame header") + + length = int.from_bytes(payload[offset:offset + 3], "big") + frame_end = offset + HTTP2_FRAME_HEADER_SIZE + length + if frame_end > len(payload): + raise RuntimeError("truncated HTTP/2 frame payload") + if payload[offset + 3] == frame_type: + count += 1 + offset = frame_end + return count + + +def serve_connection(tls_socket: ssl.SSLSocket, expected_responses: int) -> int: + """Serve requests on one HTTP/2 connection.""" + connection = H2Connection(config=H2Configuration(client_side=False, header_encoding="utf-8")) + connection.initiate_connection() + tls_socket.sendall(connection.data_to_send()) + + responses_sent = 0 + # Do not initiate connection shutdown after sending the expected + # responses. A large header block spans multiple TLS records, and closing + # here can race the peer draining the final CONTINUATION and DATA frames. + # Instead, keep servicing the connection until ATS closes it (or AuTest + # terminates this process during cleanup). + while True: + data = tls_socket.recv(65535) + if not data: + return responses_sent + + for event in connection.receive_data(data): + if isinstance(event, DataReceived): + connection.acknowledge_received_data(event.flow_controlled_length, event.stream_id) + elif isinstance(event, StreamEnded): + # Together these values are intentionally larger than the + # default 16 KiB maximum frame size even after HPACK Huffman + # encoding. Each field remains below ATS's per-field limit. + padding_one = f"{event.stream_id:08x}-" + ("0123456789abcdef" * 1024) + padding_two = f"{event.stream_id:08x}-" + ("fedcba9876543210" * 1024) + connection.send_headers( + event.stream_id, + [ + (":status", "200"), + ("content-length", "4"), + ("x-continuation-padding-one", padding_one), + ("x-continuation-padding-two", padding_two), + ], + ) + connection.send_data(event.stream_id, b"okay", end_stream=True) + + wire_bytes = connection.data_to_send() + continuation_frames = count_frames(wire_bytes, HTTP2_FRAME_TYPE_CONTINUATION) + if continuation_frames == 0: + raise RuntimeError("large response header did not generate a CONTINUATION frame") + + print( + f"stream={event.stream_id} sent_continuation_frames={continuation_frames}", + flush=True, + ) + tls_socket.sendall(wire_bytes) + responses_sent += 1 + + +def run_server(port: int, certificate: str, private_key: str, expected_responses: int) -> int: + """Accept TLS connections until ATS closes after the expected responses.""" + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(certificate, private_key) + context.set_alpn_protocols(["h2"]) + + responses_sent = 0 + with socket.create_server(("127.0.0.1", port)) as listener: + listener.settimeout(30) + while responses_sent < expected_responses: + plain_socket, _ = listener.accept() + try: + with context.wrap_socket(plain_socket, server_side=True) as tls_socket: + if tls_socket.selected_alpn_protocol() != "h2": + raise RuntimeError("ATS did not negotiate HTTP/2 with the origin") + responses_sent += serve_connection(tls_socket, expected_responses - responses_sent) + except ssl.SSLError: + # AuTest's readiness probe opens and closes a plain TCP socket. + plain_socket.close() + + return 0 if responses_sent == expected_responses else 1 + + +def parse_args() -> argparse.Namespace: + """Parse command-line arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("port", type=int) + parser.add_argument("certificate") + parser.add_argument("private_key") + parser.add_argument("expected_responses", type=int) + return parser.parse_args() + + +def main() -> int: + """Run the test origin.""" + args = parse_args() + return run_server(args.port, args.certificate, args.private_key, args.expected_responses) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/gold_tests/h2/gold/h2-settings-ack-metrics.gold b/tests/gold_tests/h2/gold/h2-settings-ack-metrics.gold new file mode 100644 index 00000000000..6ce0a1c5ee0 --- /dev/null +++ b/tests/gold_tests/h2/gold/h2-settings-ack-metrics.gold @@ -0,0 +1,2 @@ +proxy.process.http2.max_settings_frames_per_minute_exceeded 0 +proxy.process.http2.connection_errors 0 diff --git a/tests/gold_tests/h2/gold/h2o-pool-reuse-metrics.gold b/tests/gold_tests/h2/gold/h2o-pool-reuse-metrics.gold new file mode 100644 index 00000000000..3cef613120b --- /dev/null +++ b/tests/gold_tests/h2/gold/h2o-pool-reuse-metrics.gold @@ -0,0 +1,2 @@ +proxy.process.http2.total_server_connections 1 +proxy.process.http2.total_server_streams 5 diff --git a/tests/gold_tests/h2/h2_settings_ack_not_counted.test.py b/tests/gold_tests/h2/h2_settings_ack_not_counted.test.py new file mode 100644 index 00000000000..d03c9eabce6 --- /dev/null +++ b/tests/gold_tests/h2/h2_settings_ack_not_counted.test.py @@ -0,0 +1,100 @@ +''' +Verify that SETTINGS frames carrying the ACK flag do not count against +`proxy.config.http2.max_settings_frames_per_minute`. + +A SETTINGS-ACK is a mandatory protocol response to a SETTINGS frame ATS +itself sent (RFC 7540 / 9113 6.5), so it cannot be used by a peer to +flood ATS. Counting inbound ACKs against the per-minute receive limit +spuriously closed otherwise-healthy connections with ENHANCE_YOUR_CALM, +which is especially visible with +`proxy.config.http2.flow_control.policy_in=2` +(LARGE_SESSION_AND_DYNAMIC_STREAM): ATS sends a SETTINGS frame per +inbound stream and the client returns one ACK per stream. +''' +# 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 SETTINGS-ACK frames are not counted against +`proxy.config.http2.max_settings_frames_per_minute`. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2_settings_ack/settings_ack.replay.yaml" + +server = Test.MakeVerifierServerProcess("settings-ack-origin", replay_file) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +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.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http2_cs', + # Set the per-minute SETTINGS receive limit very low. The verifier + # client will send 1 ACK in response to ATS's preface SETTINGS plus + # 1 ACK per stream that ATS opens via dynamic-window SETTINGS. With + # five separate inbound H/2 sessions, the buggy code would tear at + # least one of them down with ENHANCE_YOUR_CALM after the second + # inbound ACK. With the fix, ACKs are not counted and the limit is + # never tripped. + 'proxy.config.http2.max_settings_frames_per_minute': 2, + # `LARGE_SESSION_AND_DYNAMIC_STREAM` makes ATS send a SETTINGS + # frame whenever a new inbound stream is created (to readjust the + # per-stream window). Each of those SETTINGS frames triggers a + # client ACK -- exactly the pattern that exposed the bug in + # production. + 'proxy.config.http2.flow_control.policy_in': 2, + 'proxy.config.http.cache.http': 0, + }) + +ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server.Variables.http_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr = Test.AddTestRun("Drive 5 H/2 client sessions, each with one request") +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-settings-ack", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.TimeOut = 60 + +# Once the client run finishes, assert that ATS never tripped the +# SETTINGS-frames-per-minute limit and never recorded a connection-level +# error caused by it. `stdout_wait` retries until the gold matches, so it +# tolerates the brief settle time between the last response and the +# metric updates. +tr = Test.AddTestRun("Assert SETTINGS-ACK frames did not trip the per-minute limit") +tr.Processes.Default.Command = ( + f"{Test.Variables.AtsTestToolsDir}/stdout_wait" + f" 'traffic_ctl metric get" + f" proxy.process.http2.max_settings_frames_per_minute_exceeded" + f" proxy.process.http2.connection_errors'" + f" {Test.TestDirectory}/gold/h2-settings-ack-metrics.gold") +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + "too frequent SETTINGS frames", "must not GOAWAY ENHANCE_YOUR_CALM on inbound SETTINGS-ACK frames") diff --git a/tests/gold_tests/h2/h2_to_origin_continuation.test.py b/tests/gold_tests/h2/h2_to_origin_continuation.test.py new file mode 100644 index 00000000000..8f7850628e5 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_continuation.test.py @@ -0,0 +1,104 @@ +''' +Verify that ATS does not tear down outbound HTTP/2 connections, and does +not crash on a `_sm == nullptr` assertion, when the origin response +headers may be split across HEADERS + CONTINUATION frames on a stream +that has already advanced past IDLE. +''' +# 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 os +import sys + +from ports import get_port + +Test.Summary = ''' +Verify that ATS does not tear down outbound HTTP/2 connections and does +not crash on a `_sm == nullptr` assertion when receiving the origin's +response on a stream that is past the IDLE state. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2o_continuation/continuation.replay.yaml" + +tr = Test.AddTestRun("Outbound HTTP/2 CONTINUATION on a non-IDLE stream") +tr.Setup.Copy("continuation_origin.py") + +server = tr.Processes.Process("h2-continuation-origin") +server_port = get_port(server, "https_port") +server_pem = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.pem") +server_key = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.key") +server.Setup.Copy(server_pem) +server.Setup.Copy(server_key) +server.Command = (f"{sys.executable} {tr.RunDirectory}/continuation_origin.py " + f"{server_port} server.pem server.key 2") +server.Ready = When.PortOpen(server_port) +server.ReturnCode = Any(0, -2) +server.Streams.stdout += Testers.ContainsExpression( + r"sent_continuation_frames=[1-9][0-9]*", + "The origin must positively verify that it emitted CONTINUATION frames.", +) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +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.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http2', + 'proxy.config.exec_thread.autoconfig.enabled': 0, + 'proxy.config.exec_thread.limit': 4, + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + 'proxy.config.http.server_session_sharing.pool': 'thread', + 'proxy.config.http.server_session_sharing.match': 'ip,sni,cert', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + 'proxy.config.http.cache.http': 0, + # The custom origin emits a header block larger than this value and + # positively verifies that it generated CONTINUATION frames. + 'proxy.config.http2.max_frame_size': 16384, + 'proxy.config.http2.max_header_list_size': 1048576, + 'proxy.config.http.response_header_max_size': 65536, + }) + +ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-continuation", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.TimeOut = 60 + +# Regression guards for the two outbound CONTINUATION bugs: +# 1) Connection-level PROTOCOL_ERROR ("continuation bad state") would +# mean rcv_continuation_frame still rejects the OPEN / +# HALF_CLOSED_LOCAL stream states on outbound connections. +# 2) The `_sm == nullptr` assertion (visible as a fatal in +# traffic.out) would mean rcv_continuation_frame called +# `new_transaction` on an outbound stream whose state machine +# already exists (the SM was created when ATS issued the request). +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + "continuation bad state", "ATS must not raise a PROTOCOL_ERROR for outbound CONTINUATION frames in OPEN/HALF_CLOSED_LOCAL") +ts.Disk.traffic_out.Content = Testers.ExcludesExpression( + "_sm == nullptr", "ATS must not re-create an outbound HTTP/2 transaction on receipt of CONTINUATION") diff --git a/tests/gold_tests/h2/h2_to_origin_payload_validation.test.py b/tests/gold_tests/h2/h2_to_origin_payload_validation.test.py new file mode 100644 index 00000000000..fc18db5f0d1 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_payload_validation.test.py @@ -0,0 +1,75 @@ +''' +Verify that ATS forwards origin HTTP/2 responses for HEAD requests and +304 responses to conditional GETs without spurious "Bad payload length" +stream errors when both client and origin use HTTP/2. +''' +# 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 outbound HTTP/2 payload-length validation honors RFC 9110 8.6 for +HEAD responses and for 304 responses to conditional GETs. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2o_payload_validation/payload_validation.replay.yaml" + +server = Test.MakeVerifierServerProcess("h2-payload-origin", replay_file) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +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.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http2', + 'proxy.config.exec_thread.autoconfig.enabled': 0, + 'proxy.config.exec_thread.limit': 4, + # Negotiate HTTP/2 to the origin via ALPN. + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + 'proxy.config.http.server_session_sharing.pool': 'thread', + 'proxy.config.http.server_session_sharing.match': 'ip,sni,cert', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + # Disable caching so the conditional GET reaches the origin. + 'proxy.config.http.cache.http': 0, + }) + +ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server.Variables.https_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr = Test.AddTestRun("HEAD/304 with Content-Length over outbound HTTP/2") +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-payload", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.TimeOut = 60 + +# These warnings would indicate a regression of the outbound payload +# preclusion logic. They must not appear when ATS correctly recognizes the +# original HEAD or conditional GET request method. +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + "Bad payload length", "ATS must not report a bad payload length for HEAD/304 outbound H2 responses") +ts.Disk.diags_log.Content += Testers.ExcludesExpression( + "recv data bad payload length", "ATS must not raise a stream PROTOCOL_ERROR for HEAD/304 outbound H2 responses") diff --git a/tests/gold_tests/h2/h2_to_origin_pool_reuse.test.py b/tests/gold_tests/h2/h2_to_origin_pool_reuse.test.py new file mode 100644 index 00000000000..84557a1e4a5 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_pool_reuse.test.py @@ -0,0 +1,106 @@ +''' +Verify that ATS multiplexes multiple outbound HTTP/2 origin requests onto +a single H/2 connection when the configured server-session sharing pool is +`global`. This is a regression test for the latent bug where outbound H/2 +sessions were filed exclusively in the per-thread pool while +`HttpSessionManager::acquire_session` was searching only the global pool, +silently disabling H/2 origin reuse and forcing a fresh TCP+TLS+H/2 +handshake for every single request. +''' +# 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 outbound HTTP/2 origin connection reuse / multiplexing across +multiple requests when proxy.config.http.server_session_sharing.pool is +`global`. +''' + +Test.ContinueOnFail = True + +replay_file = "replay_h2o_pool_reuse/pool_reuse.replay.yaml" + +server = Test.MakeVerifierServerProcess("h2-pool-origin", replay_file) + +ts = Test.MakeATSProcess("ts", enable_tls=True) +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.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.exec_thread.autoconfig.enabled': 0, + # Pin to a single net thread so all transactions land on the same + # EThread. Cross-thread sharing of multiplexing H/2 origin sessions + # is intentionally not supported (their state is owned by the + # EThread driving the connection); reuse is only expected within a + # single thread, so single-thread is what the test asserts on. + 'proxy.config.exec_thread.limit': 1, + 'proxy.config.ssl.client.alpn_protocols': 'h2,http/1.1', + # The bug being verified is specific to pool=global: H/2 origin + # sessions live in the per-thread pool but pool=global only consults + # the global pool, so reuse silently fails. With the fix in place, + # `_acquire_session` also checks the thread pool first, so reuse + # works regardless of the configured pool type. + 'proxy.config.http.server_session_sharing.pool': 'global', + 'proxy.config.http.server_session_sharing.match': 'ip,sni,cert', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + 'proxy.config.http.cache.http': 0, + # Keep the original Host header through remap so SNI matching for + # session reuse uses the same hostname for every request rather + # than the IP literal `127.0.0.1` (which TLS will not send as an + # SNI). + 'proxy.config.url_remap.pristine_host_hdr': 1, + }) + +ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server.Variables.https_port}') +ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr = Test.AddTestRun("Drive 5 sequential H/2 requests over a single client session") +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.AddVerifierClientProcess("client-pool-reuse", replay_file, http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.TimeOut = 60 + +# `stdout_wait` retries the command until its output matches the gold file +# (or the run times out), so we don't need a separate settling step -- +# once ATS finishes releasing all 5 streams the metrics will agree. +tr = Test.AddTestRun("Assert exactly one H/2 origin connection carried all 5 streams") +tr.Processes.Default.Command = ( + f"{Test.Variables.AtsTestToolsDir}/stdout_wait" + f" 'traffic_ctl metric get" + f" proxy.process.http2.total_server_connections" + f" proxy.process.http2.total_server_streams'" + f" {Test.TestDirectory}/gold/h2o-pool-reuse-metrics.gold") +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# Regression guard: if the bug is reintroduced, every transaction opens a +# fresh outbound H/2 connection and `Add session to pool` is logged once +# per connection. Catch the case of more than 5 outbound H/2 sessions. +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + r"(?:.*Add session to pool.*\n.*){5,}.*Add session to pool", + "must not open more than 5 outbound H/2 origin sessions for 5 requests") diff --git a/tests/gold_tests/h2/h2_to_origin_safe_retry.test.py b/tests/gold_tests/h2/h2_to_origin_safe_retry.test.py new file mode 100644 index 00000000000..731042bad27 --- /dev/null +++ b/tests/gold_tests/h2/h2_to_origin_safe_retry.test.py @@ -0,0 +1,115 @@ +''' +Verify ATS retries non-idempotent outbound HTTP/2 requests when the origin +guarantees that it did not process them. +''' +# 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 os +import sys + +from ports import get_port + +Test.Summary = ''' +Verify that ATS safely retries a POST after the HTTP/2 origin either sends +RST_STREAM(REFUSED_STREAM) or GOAWAY(last_stream_id=0). +''' + +Test.ContinueOnFail = True + + +class SafeRetryScenario: + """Configure one HTTP/2 safe-retry scenario.""" + + replay_file = "replay_h2o_safe_retry/safe_retry.replay.yaml" + + def __init__(self, mode: str, replay_key: str) -> None: + tr = Test.AddTestRun(f"Safe POST retry after outbound HTTP/2 {mode}") + tr.Setup.Copy("safe_retry_origin.py") + + server = tr.Processes.Process(f"safe-retry-origin-{mode}") + server_port = get_port(server, "https_port") + server_pem = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.pem") + server_key = os.path.join(Test.Variables.AtsTestToolsDir, "ssl", "server.key") + server.Setup.Copy(server_pem) + server.Setup.Copy(server_key) + server.Command = ( + f"{sys.executable} {tr.RunDirectory}/safe_retry_origin.py " + f"{mode} {server_port} server.pem server.key") + server.Ready = When.PortOpen(server_port) + server.ReturnCode = Any(0, -2) + + action = "REFUSED_STREAM" if mode == "rst" else "GOAWAY" + server.Streams.stdout += Testers.ContainsExpression( + rf"action={action} attempt=1", + f"The origin must reject the first POST with {action}.", + ) + server.Streams.stdout += Testers.ContainsExpression( + r"retry_succeeded attempts=2 method=POST body=request-body", + "ATS must retry the POST once, including its request body.", + ) + + ts = tr.MakeATSProcess(f"ts-safe-retry-{mode}", enable_tls=True, enable_cache=False) + 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.alpn_protocols': 'h2,http/1.1', + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', + 'proxy.config.http.server_session_sharing.pool': 'thread', + # The RST_STREAM retry may reuse the healthy H2 connection. + # Match only on the loopback IP because an IP-literal remap + # does not send SNI, while ATS proposes the literal as the + # lookup SNI when it searches the session pool. + 'proxy.config.http.server_session_sharing.match': 'ip', + 'proxy.config.http.connect_attempts_max_retries': 3, + # Retrying a request with a body requires ATS's existing + # request buffer so the second origin attempt can replay it. + 'proxy.config.http.request_buffer_enabled': 1, + 'proxy.config.http.post_copy_size': 4096, + 'proxy.config.exec_thread.autoconfig.enabled': 0, + 'proxy.config.exec_thread.limit': 1, + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|http2', + }) + ts.Disk.remap_config.AddLine(f'map / https://127.0.0.1:{server_port}') + ts.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + + tr.Processes.Default.StartBefore(server) + tr.Processes.Default.StartBefore(ts) + tr.AddVerifierClientProcess( + f"safe-retry-client-{mode}", + self.replay_file, + http_ports=[ts.Variables.port], + keys=replay_key, + ) + tr.TimeOut = 60 + + ts.Disk.traffic_out.Content = Testers.ExcludesExpression( + "ERR_CLIENT_ABORT", + "A request guaranteed unprocessed by the origin must not be returned as a client abort.", + ) + + +SafeRetryScenario("rst", "rst-refused") +SafeRetryScenario("goaway", "goaway-last-stream-zero") diff --git a/tests/gold_tests/h2/replay_h2_settings_ack/settings_ack.replay.yaml b/tests/gold_tests/h2/replay_h2_settings_ack/settings_ack.replay.yaml new file mode 100644 index 00000000000..2da2a10c0c0 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2_settings_ack/settings_ack.replay.yaml @@ -0,0 +1,187 @@ +# 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. + +meta: + version: '1.0' + +# Five separate H/2 client sessions, each issuing a single request. Each +# session generates: +# * 1 SETTINGS-ACK in response to ATS's connection-preface SETTINGS, +# * 1 SETTINGS-ACK in response to the per-stream dynamic-window SETTINGS +# that ATS sends with `flow_control.policy_in=2`, +# i.e. ~2 inbound SETTINGS-ACK frames per session. Across the five +# sessions ATS receives ~10 inbound SETTINGS-ACK frames in well under a +# minute. With `max_settings_frames_per_minute=2` and the buggy +# implementation, ATS would close the connection with ENHANCE_YOUR_CALM +# and the verifier client would observe its stream being torn down. With +# the fix in place, ACKs are not counted toward the limit, the limit +# stays at 0, and all five sessions complete cleanly. + +sessions: + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-1 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/1 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-1 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'one1' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-2 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/2 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-2 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'two2' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-3 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/3 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-3 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 6 ] + content: { encoding: plain, data: 'three3' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-4 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/4 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-4 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 5 ] + content: { encoding: plain, data: 'four4' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '2' + - name: tls + sni: settings-ack.example.com + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, settings-ack-5 ]]}} + client-request: + version: '2' + scheme: https + method: GET + url: /settings-ack/5 + headers: + encoding: esc_json + fields: + - [ Host, settings-ack.example.com ] + - [ uuid, settings-ack-5 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 5 ] + content: { encoding: plain, data: 'five5' } + proxy-response: + status: 200 diff --git a/tests/gold_tests/h2/replay_h2o_continuation/continuation.replay.yaml b/tests/gold_tests/h2/replay_h2o_continuation/continuation.replay.yaml new file mode 100644 index 00000000000..7eb1f981b53 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_continuation/continuation.replay.yaml @@ -0,0 +1,134 @@ +# 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. + +meta: + version: '1.0' + +# Two outbound HTTP/2 transactions whose responses are generated by the +# custom origin in `continuation_origin.py`. The origin sends a header block +# larger than 16 KiB and positively verifies that it emits CONTINUATION +# frames. This drives the outbound response path on streams that are past +# IDLE so the regression guards in `h2_to_origin_continuation.test.py` catch: +# +# - "continuation bad state" PROTOCOL_ERROR if the outbound +# CONTINUATION state-machine guard is ever re-tightened. +# - The `_sm == nullptr` assertion if `rcv_continuation_frame` ever +# unconditionally calls `new_transaction` on outbound streams again. +sessions: + - protocol: + - name: http + version: '2' + - name: tls + version: TLSv1.3 + sni: cont.h2o.example.com + proxy-verify-mode: 0 + proxy-provided-cert: true + - name: tcp + - name: ip + version: '4' + + transactions: + - all: { headers: { fields: [[ uuid, h2o-cont-1 ]]}} + + client-request: + version: '2' + scheme: https + method: GET + url: /continuation/first + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: GET + url: + - [ path, { value: /continuation/first, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + + server-response: + version: '2' + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: + encoding: plain + data: 'okay' + + proxy-response: + version: '2' + status: 200 + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + + - all: { headers: { fields: [[ uuid, h2o-cont-2 ]]}} + + client-request: + version: '2' + scheme: https + method: GET + url: /continuation/second + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: GET + url: + - [ path, { value: /continuation/second, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, cont.h2o.example.com ] + + server-response: + version: '2' + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: + encoding: plain + data: 'okay' + + proxy-response: + version: '2' + status: 200 + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] diff --git a/tests/gold_tests/h2/replay_h2o_payload_validation/payload_validation.replay.yaml b/tests/gold_tests/h2/replay_h2o_payload_validation/payload_validation.replay.yaml new file mode 100644 index 00000000000..3df925b33a6 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_payload_validation/payload_validation.replay.yaml @@ -0,0 +1,146 @@ +# 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. + +meta: + version: '1.0' + +# +# These transactions exercise the payload-length validation code path on +# outbound (origin-side) HTTP/2 streams. They cover the [RFC 9110] 8.6 cases +# where a non-zero Content-Length is permitted with no message body: +# +# * Test 1: A HEAD request with a Content-Length response from the origin. +# * Test 2: A conditional GET (If-None-Match) that the origin satisfies +# with a 304 (Not Modified) response carrying Content-Length. +# +# Without the Http2Stream send-side request snapshot, ATS would tear down +# these streams with a STREAM PROTOCOL_ERROR ("recv data bad payload length") +# because `_send_header` is destroyed before the response is validated. + +sessions: + - protocol: + - name: http + version: '2' + - name: tls + version: TLSv1.3 + sni: payload.h2o.example.com + proxy-verify-mode: 0 + proxy-provided-cert: true + - name: tcp + - name: ip + version: '4' + + transactions: + # + # Test 1: HEAD request with a non-zero Content-Length response. + # + - all: { headers: { fields: [[ uuid, head-with-content-length ]]}} + + client-request: + version: '2' + scheme: https + method: HEAD + url: /payload/head + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: HEAD + url: + - [ path, { value: /payload/head, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + + server-response: + version: '2' + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 7 ] + content: + encoding: plain + size: 0 + + proxy-response: + version: '2' + status: 200 + headers: + encoding: esc_json + fields: + - [ Content-Length, 7 ] + + # + # Test 2: Conditional GET satisfied with a 304 carrying Content-Length. + # + - all: { headers: { fields: [[ uuid, conditional-get-304 ]]}} + + client-request: + version: '2' + scheme: https + method: GET + url: /payload/conditional + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + - [ If-None-Match, '"abc"' ] + content: + encoding: plain + size: 0 + + proxy-request: + version: '2' + scheme: https + method: GET + url: + - [ path, { value: /payload/conditional, as: equal } ] + headers: + encoding: esc_json + fields: + - [ Host, payload.h2o.example.com ] + - [ If-None-Match, { value: '"abc"', as: equal } ] + + server-response: + version: '2' + status: 304 + reason: Not Modified + headers: + encoding: esc_json + fields: + - [ Content-Length, 42 ] + - [ ETag, '"abc"' ] + content: + encoding: plain + size: 0 + + proxy-response: + version: '2' + status: 304 + headers: + encoding: esc_json + fields: + - [ Content-Length, 42 ] diff --git a/tests/gold_tests/h2/replay_h2o_pool_reuse/pool_reuse.replay.yaml b/tests/gold_tests/h2/replay_h2o_pool_reuse/pool_reuse.replay.yaml new file mode 100644 index 00000000000..742bddef3c6 --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_pool_reuse/pool_reuse.replay.yaml @@ -0,0 +1,173 @@ +# 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. + +meta: + version: '1.0' + +# Five separate H/1 client sessions, each issuing a single request to the +# same origin (matching SNI + IP + cert). This is the production-style +# pattern that exposes the H/2 origin session pooling bug: every new +# client session triggers a fresh `acquire_session` lookup with no +# session attached to the user-agent transaction, so the pool search is +# what determines whether the existing H/2 origin connection is reused. +# If reuse works, all 5 requests should be multiplexed onto a single H/2 +# origin connection (1 connection, 5 streams). + +sessions: + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-1 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/1 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-1 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'one1' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-2 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/2 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-2 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'two2' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-3 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/3 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-3 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'thr3' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-4 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/4 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-4 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'fou4' } + proxy-response: + status: 200 + + - protocol: + - name: http + version: '1.1' + - name: tcp + - name: ip + version: '4' + transactions: + - all: { headers: { fields: [[ uuid, pool-reuse-5 ]]}} + client-request: + version: '1.1' + scheme: http + method: GET + url: /pool/reuse/5 + headers: + encoding: esc_json + fields: + - [ Host, pool.h2o.example.com ] + - [ uuid, pool-reuse-5 ] + server-response: + status: 200 + reason: OK + headers: + encoding: esc_json + fields: + - [ Content-Length, 4 ] + content: { encoding: plain, data: 'fiv5' } + proxy-response: + status: 200 diff --git a/tests/gold_tests/h2/replay_h2o_safe_retry/safe_retry.replay.yaml b/tests/gold_tests/h2/replay_h2o_safe_retry/safe_retry.replay.yaml new file mode 100644 index 00000000000..a089a3718ac --- /dev/null +++ b/tests/gold_tests/h2/replay_h2o_safe_retry/safe_retry.replay.yaml @@ -0,0 +1,67 @@ +# 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. + +meta: + version: '1.0' + +# The custom HTTP/2 origin rejects the first copy of each POST with a signal +# that guarantees the request was not processed. A successful response proves +# that ATS retries the non-idempotent method and replays its body. +sessions: + - transactions: + - all: { headers: { fields: [[ uuid, rst-refused ]]}} + client-request: + method: POST + version: '1.1' + scheme: http + url: /rst-refused + headers: + fields: + - [ Host, safe-retry.example.com ] + - [ Content-Length, 12 ] + content: + encoding: plain + data: request-body + proxy-response: + status: 200 + headers: + fields: + - [ Content-Length, 7 ] + content: + encoding: plain + data: retried + + - all: { headers: { fields: [[ uuid, goaway-last-stream-zero ]]}} + client-request: + method: POST + version: '1.1' + scheme: http + url: /goaway-last-stream-zero + headers: + fields: + - [ Host, safe-retry.example.com ] + - [ Content-Length, 12 ] + content: + encoding: plain + data: request-body + proxy-response: + status: 200 + headers: + fields: + - [ Content-Length, 7 ] + content: + encoding: plain + data: retried diff --git a/tests/gold_tests/h2/safe_retry_origin.py b/tests/gold_tests/h2/safe_retry_origin.py new file mode 100644 index 00000000000..7ecc864498f --- /dev/null +++ b/tests/gold_tests/h2/safe_retry_origin.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +"""Exercise safe retries after HTTP/2 REFUSED_STREAM and GOAWAY.""" + +# 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 argparse +import socket +import ssl +import sys + +from h2.config import H2Configuration +from h2.connection import H2Connection +from h2.errors import ErrorCodes +from h2.events import ConnectionTerminated, DataReceived, RequestReceived, StreamEnded + +EXPECTED_BODY = b"request-body" +RESPONSE_BODY = b"retried" + + +class RetryOrigin: + """An HTTP/2 origin that rejects the first POST and accepts its retry.""" + + def __init__(self, mode: str) -> None: + self.mode = mode + self.attempts = 0 + + def _validate_request(self, headers: dict[str, str], body: bytes) -> None: + """Verify that ATS replayed the non-idempotent request intact.""" + if headers.get(":method") != "POST": + raise RuntimeError(f"expected POST, got {headers.get(':method')}") + if body != EXPECTED_BODY: + raise RuntimeError(f"expected body {EXPECTED_BODY!r}, got {body!r}") + + def _reject_first_attempt(self, connection: H2Connection, stream_id: int) -> bool: + """Reject the first request and return whether to close the socket.""" + if self.mode == "rst": + connection.reset_stream(stream_id, error_code=ErrorCodes.REFUSED_STREAM) + print("action=REFUSED_STREAM attempt=1", flush=True) + return False + + connection.close_connection( + error_code=ErrorCodes.NO_ERROR, + last_stream_id=0, + ) + print("action=GOAWAY attempt=1 last_stream_id=0", flush=True) + return True + + def serve_connection(self, tls_socket: ssl.SSLSocket) -> bool: + """Serve one HTTP/2 connection and report whether the retry succeeded.""" + connection = H2Connection(config=H2Configuration(client_side=False, header_encoding="utf-8")) + connection.initiate_connection() + tls_socket.sendall(connection.data_to_send()) + + headers_by_stream: dict[int, dict[str, str]] = {} + bodies_by_stream: dict[int, bytearray] = {} + + while True: + data = tls_socket.recv(65535) + if not data: + return False + + close_socket = False + retry_succeeded = False + for event in connection.receive_data(data): + if isinstance(event, RequestReceived): + headers_by_stream[event.stream_id] = dict(event.headers) + bodies_by_stream[event.stream_id] = bytearray() + elif isinstance(event, DataReceived): + bodies_by_stream[event.stream_id].extend(event.data) + connection.acknowledge_received_data(event.flow_controlled_length, event.stream_id) + elif isinstance(event, StreamEnded): + headers = headers_by_stream[event.stream_id] + body = bytes(bodies_by_stream[event.stream_id]) + self._validate_request(headers, body) + self.attempts += 1 + + if self.attempts == 1: + close_socket = self._reject_first_attempt(connection, event.stream_id) + elif self.attempts == 2: + connection.send_headers( + event.stream_id, + [ + (":status", "200"), + ("content-length", str(len(RESPONSE_BODY))), + ], + ) + connection.send_data(event.stream_id, RESPONSE_BODY, end_stream=True) + retry_succeeded = True + else: + raise RuntimeError(f"received unexpected request attempt {self.attempts}") + elif isinstance(event, ConnectionTerminated): + close_socket = True + + wire_bytes = connection.data_to_send() + if wire_bytes: + tls_socket.sendall(wire_bytes) + + if retry_succeeded: + print( + f"retry_succeeded attempts={self.attempts} method=POST body={EXPECTED_BODY.decode()}", + flush=True, + ) + return True + if close_socket: + return False + + +def run_server(mode: str, port: int, certificate: str, private_key: str) -> int: + """Accept connections until ATS successfully retries the request.""" + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(certificate, private_key) + context.set_alpn_protocols(["h2"]) + + origin = RetryOrigin(mode) + with socket.create_server(("127.0.0.1", port)) as listener: + listener.settimeout(30) + while origin.attempts < 2: + plain_socket, _ = listener.accept() + try: + with context.wrap_socket(plain_socket, server_side=True) as tls_socket: + if tls_socket.selected_alpn_protocol() != "h2": + raise RuntimeError("ATS did not negotiate HTTP/2 with the origin") + if origin.serve_connection(tls_socket): + return 0 + except ssl.SSLError: + # AuTest's readiness probe opens and closes a plain TCP socket. + plain_socket.close() + + return 1 + + +def parse_args() -> argparse.Namespace: + """Parse command-line arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("mode", choices=("rst", "goaway")) + parser.add_argument("port", type=int) + parser.add_argument("certificate") + parser.add_argument("private_key") + return parser.parse_args() + + +def main() -> int: + """Run the test origin.""" + args = parse_args() + return run_server(args.mode, args.port, args.certificate, args.private_key) + + +if __name__ == "__main__": + sys.exit(main())