From f8d19d640b96ad197ab1084676e308f8c1d409e5 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Tue, 11 Jul 2023 17:34:07 -0600 Subject: [PATCH 1/3] Give a chance to send a response before receiving next request on H2 --- proxy/http2/Http2CommonSession.cc | 10 ++++++++++ proxy/http2/Http2CommonSession.h | 5 +++++ proxy/http2/Http2ConnectionState.cc | 4 ++++ 3 files changed, 19 insertions(+) diff --git a/proxy/http2/Http2CommonSession.cc b/proxy/http2/Http2CommonSession.cc index 80d16a6cfc4..613d6a1de42 100644 --- a/proxy/http2/Http2CommonSession.cc +++ b/proxy/http2/Http2CommonSession.cc @@ -406,10 +406,20 @@ Http2CommonSession::do_process_frame_read(int event, VIO *vio, bool inside_frame bool Http2CommonSession::_should_do_something_else() { + if (this->_interrupt_reading_frames) { + this->_interrupt_reading_frames = false; + return true; + } // Do something else every 128 incoming frames if connection state isn't closed return (this->_n_frame_read & 0x7F) == 0 && !connection_state.is_state_closed(); } +void +Http2CommonSession::interrupt_reading_frames() +{ + this->_interrupt_reading_frames = true; +} + int64_t Http2CommonSession::write_avail() { diff --git a/proxy/http2/Http2CommonSession.h b/proxy/http2/Http2CommonSession.h index 6eb42c3dd02..8209cf00bac 100644 --- a/proxy/http2/Http2CommonSession.h +++ b/proxy/http2/Http2CommonSession.h @@ -114,6 +114,8 @@ class Http2CommonSession virtual void set_no_activity_timeout() = 0; + void interrupt_reading_frames(); + /////////////////// // Variables Http2ConnectionState connection_state; @@ -166,6 +168,9 @@ class Http2CommonSession int64_t read_from_early_data = 0; bool cur_frame_from_early_data = false; + +private: + bool _interrupt_reading_frames = false; }; /////////////////////////////////////////////// diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index f99ef6bbf08..fbc53d9f382 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -487,6 +487,8 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame) stream->send_request(*this); } } + // Give a chance to send response before reading next frame. + this->session->interrupt_reading_frames(); } else { // NOTE: Expect CONTINUATION Frame. Do NOT change state of stream or decode // Header Blocks. @@ -1047,6 +1049,8 @@ Http2ConnectionState::rcv_continuation_frame(const Http2Frame &frame) stream->new_transaction(frame.is_from_early_data()); // Send request header to SM stream->send_request(*this); + // Give a chance to send response before reading next frame. + this->session->interrupt_reading_frames(); } else { // NOTE: Expect another CONTINUATION Frame. Do nothing. Http2StreamDebug(this->session, stream_id, "No END_HEADERS flag, expecting CONTINUATION frame"); From fa4b612dd218beb64f8ca6234ad6118501025777 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 28 Jul 2023 14:49:55 -0600 Subject: [PATCH 2/3] Keep client connections open to let ATS receive RST_STREAM --- .../http2_rst_stream_client_after_data.yaml | 18 ++++++++++++++++++ .../http2_rst_stream_client_after_headers.yaml | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml index d505778f5ea..423052f6133 100644 --- a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml +++ b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml @@ -52,3 +52,21 @@ sessions: encoding: plain data: server_test verify: {as: equal} + + # Unrelated request just to keep the connection open + - client-request: + delay: 1s + frames: + - HEADERS: + headers: + fields: + - [:method, GET] + - [:scheme, https] + - [:authority, example.data.com] + - [:path, /a/path] + - [uuid, 2] + + server-response: + headers: + fields: + - [:status, 200] diff --git a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml index 58415705a00..8e510d625de 100644 --- a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml +++ b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml @@ -52,3 +52,21 @@ sessions: encoding: plain data: server_test verify: {as: equal} + + # Unrelated request just to keep the connection open + - client-request: + delay: 1s + frames: + - HEADERS: + headers: + fields: + - [:method, GET] + - [:scheme, https] + - [:authority, example.data.com] + - [:path, /a/path] + - [uuid, 2] + + server-response: + headers: + fields: + - [:status, 200] From e73a9966dbc1c3e2c079aaf04b4fc0aa79914c32 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Wed, 6 Sep 2023 13:59:12 -0600 Subject: [PATCH 3/3] Use new ProxyVerifier that supports keep-connection-open --- .../http2_rst_stream_client_after_data.yaml | 19 +------------------ ...http2_rst_stream_client_after_headers.yaml | 19 +------------------ 2 files changed, 2 insertions(+), 36 deletions(-) diff --git a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml index 423052f6133..60bfa6f7ace 100644 --- a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml +++ b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_data.yaml @@ -9,6 +9,7 @@ sessions: - name: tcp - name: ip version: 4 + keep-connection-open: 2s transactions: - client-request: frames: @@ -52,21 +53,3 @@ sessions: encoding: plain data: server_test verify: {as: equal} - - # Unrelated request just to keep the connection open - - client-request: - delay: 1s - frames: - - HEADERS: - headers: - fields: - - [:method, GET] - - [:scheme, https] - - [:authority, example.data.com] - - [:path, /a/path] - - [uuid, 2] - - server-response: - headers: - fields: - - [:status, 200] diff --git a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml index 8e510d625de..27df8510302 100644 --- a/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml +++ b/tests/gold_tests/h2/replay_rst_stream/http2_rst_stream_client_after_headers.yaml @@ -9,6 +9,7 @@ sessions: - name: tcp - name: ip version: 4 + keep-connection-open: 2s transactions: - client-request: frames: @@ -52,21 +53,3 @@ sessions: encoding: plain data: server_test verify: {as: equal} - - # Unrelated request just to keep the connection open - - client-request: - delay: 1s - frames: - - HEADERS: - headers: - fields: - - [:method, GET] - - [:scheme, https] - - [:authority, example.data.com] - - [:path, /a/path] - - [uuid, 2] - - server-response: - headers: - fields: - - [:status, 200]