From 48ecf6e1bb3ae3c007a0913108f4f582345ce2a6 Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Mon, 26 Jul 2021 17:05:03 +0000 Subject: [PATCH 1/3] Abstract adding Connection: close header to avoid triggering H2 draining logic --- proxy/ProxyTransaction.cc | 8 ++++++++ proxy/ProxyTransaction.h | 4 ++++ proxy/http/Http1Transaction.h | 7 +++++++ proxy/http/HttpSM.cc | 2 +- proxy/http/HttpTransact.cc | 8 ++++++-- 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/proxy/ProxyTransaction.cc b/proxy/ProxyTransaction.cc index 3f1ace9cb86..cb80b3cc2cc 100644 --- a/proxy/ProxyTransaction.cc +++ b/proxy/ProxyTransaction.cc @@ -240,3 +240,11 @@ ProxyTransaction::allow_half_open() const { return false; } + +// Most protocols will not want to set the Connection: header +// For H2 it will initiate the drain logic. So we make do nothing +// the default action. +void +ProxyTransaction::set_close_connection(HTTPHdr &hdr) const +{ +} diff --git a/proxy/ProxyTransaction.h b/proxy/ProxyTransaction.h index 82a07d6e757..261af6829bd 100644 --- a/proxy/ProxyTransaction.h +++ b/proxy/ProxyTransaction.h @@ -87,6 +87,10 @@ class ProxyTransaction : public VConnection // Returns true if there is a request body for this request virtual bool has_request_body(int64_t content_length, bool is_chunked_set) const; + // Worker function to set Connection:close header if appropriate for + // underlying protocol + virtual void set_close_connection(HTTPHdr &hdr) const; + sockaddr const *get_remote_addr() const; virtual HTTPVersion get_version(HTTPHdr &hdr) const; diff --git a/proxy/http/Http1Transaction.h b/proxy/http/Http1Transaction.h index be0a174d236..aeb1d3a0c4f 100644 --- a/proxy/http/Http1Transaction.h +++ b/proxy/http/Http1Transaction.h @@ -43,6 +43,7 @@ class Http1Transaction : public ProxyTransaction // Methods int get_transaction_id() const override; void set_reader(IOBufferReader *reader); + void set_close_connection(HTTPHdr &hdr) const override; //////////////////// // Variables @@ -71,3 +72,9 @@ Http1Transaction::set_reader(IOBufferReader *reader) { _reader = reader; } + +inline void +Http1Transaction::set_close_connection(HTTPHdr &hdr) const +{ + hdr.value_set(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION, "close", 5); +} diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 1d697baeb3b..4efe9342bda 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -5872,7 +5872,7 @@ HttpSM::do_drain_request_body(HTTPHdr &response) close_connection: t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE; - response.value_set(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION, "close", 5); + ua_txn->set_close_connection(response); } void diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 6e66aa4589c..85d0b3d4bb9 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -6891,7 +6891,7 @@ HttpTransact::handle_request_keep_alive_headers(State *s, HTTPVersion ver, HTTPH if (s->current.request_to == PARENT_PROXY && parent_is_proxy(s)) { heads->value_set(MIME_FIELD_PROXY_CONNECTION, MIME_LEN_PROXY_CONNECTION, "close", 5); } else { - heads->value_set(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION, "close", 5); + s->state_machine->get_server_txn()->set_close_connection(*heads); } } // Note: if we are 1.1, we always need to send the close @@ -7050,7 +7050,11 @@ HttpTransact::handle_response_keep_alive_headers(State *s, HTTPVersion ver, HTTP case KA_CLOSE: case KA_DISABLED: if (s->client_info.keep_alive != HTTP_NO_KEEPALIVE || (ver == HTTP_1_1)) { - heads->value_set(c_hdr_field_str, c_hdr_field_len, "close", 5); + if (s->client_info.proxy_connect_hdr) { + heads->value_set(c_hdr_field_str, c_hdr_field_len, "close", 5); + } else { + s->state_machine->ua_txn->set_close_connection(*heads); + } s->client_info.keep_alive = HTTP_NO_KEEPALIVE; } // Note: if we are 1.1, we always need to send the close From c84149132ba570ada04da6fa13602c6457344fc5 Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Mon, 26 Jul 2021 19:51:05 +0000 Subject: [PATCH 2/3] Add null check for regression tests --- proxy/http/HttpTransact.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 85d0b3d4bb9..45e8d59f66b 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -7052,7 +7052,7 @@ HttpTransact::handle_response_keep_alive_headers(State *s, HTTPVersion ver, HTTP if (s->client_info.keep_alive != HTTP_NO_KEEPALIVE || (ver == HTTP_1_1)) { if (s->client_info.proxy_connect_hdr) { heads->value_set(c_hdr_field_str, c_hdr_field_len, "close", 5); - } else { + } else if (s->state_machine->ua_txn != nullptr) { s->state_machine->ua_txn->set_close_connection(*heads); } s->client_info.keep_alive = HTTP_NO_KEEPALIVE; From b612446086b7b2f10b7369ba73c4dc4dacf1b17d Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Mon, 26 Jul 2021 22:59:58 +0000 Subject: [PATCH 3/3] Fix slice tests --- proxy/http/HttpTransact.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 45e8d59f66b..ce2d7fae035 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -6891,7 +6891,10 @@ HttpTransact::handle_request_keep_alive_headers(State *s, HTTPVersion ver, HTTPH if (s->current.request_to == PARENT_PROXY && parent_is_proxy(s)) { heads->value_set(MIME_FIELD_PROXY_CONNECTION, MIME_LEN_PROXY_CONNECTION, "close", 5); } else { - s->state_machine->get_server_txn()->set_close_connection(*heads); + ProxyTransaction *svr = s->state_machine->get_server_txn(); + if (svr) { + svr->set_close_connection(*heads); + } } } // Note: if we are 1.1, we always need to send the close