This is an automated email from the ASF dual-hosted git repository. eze pushed a commit to branch 8.1.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/8.1.x by this push: new 03fcfd7ad 8.1.x Backport: Move has_request_body to ProxyTransaction (#7499) (#8880) 03fcfd7ad is described below commit 03fcfd7ad811381b3964d7be005cdc638a6f8192 Author: Robert O Butts <rob...@users.noreply.github.com> AuthorDate: Fri Jun 3 09:52:03 2022 -0600 8.1.x Backport: Move has_request_body to ProxyTransaction (#7499) (#8880) * Adjust so transfer-encoding header can be treated hop-by-hop (#7473) (cherry picked from commit 926dd71b32ade8615fbba6737816240d6df0c9ad) * Move has_request_body to ProxyTransaction (#7499) (cherry picked from commit d8f39709cfd5c71c31fa0e4acd7c5d6cd27b1fce) Co-authored-by: Susan Hinrichs <shinr...@yahoo-inc.com> --- proxy/ProxyClientTransaction.cc | 6 ++++++ proxy/ProxyClientTransaction.h | 3 +++ proxy/hdrs/HdrToken.cc | 3 ++- proxy/http/HttpSM.cc | 28 ++++++++++++++++++------- proxy/http/HttpTransact.cc | 41 +++++++++++++++++++++---------------- proxy/http2/Http2ConnectionState.cc | 4 ++++ proxy/http2/Http2Stream.cc | 8 ++++++++ proxy/http2/Http2Stream.h | 3 +++ tests/gold_tests/h2/http2.test.py | 4 ++-- 9 files changed, 72 insertions(+), 28 deletions(-) diff --git a/proxy/ProxyClientTransaction.cc b/proxy/ProxyClientTransaction.cc index 643fe5f13..e2fc03ed7 100644 --- a/proxy/ProxyClientTransaction.cc +++ b/proxy/ProxyClientTransaction.cc @@ -127,3 +127,9 @@ ProxyClientTransaction::get_transaction_priority_dependence() const { return 0; } + +bool +ProxyClientTransaction::has_request_body(int64_t request_content_length, bool is_chunked) const +{ + return request_content_length > 0 || is_chunked; +} diff --git a/proxy/ProxyClientTransaction.h b/proxy/ProxyClientTransaction.h index 2666b8b22..e6b8a81c6 100644 --- a/proxy/ProxyClientTransaction.h +++ b/proxy/ProxyClientTransaction.h @@ -227,6 +227,9 @@ public: virtual int get_transaction_priority_dependence() const; virtual bool allow_half_open() const = 0; + // 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; + virtual const char * get_protocol_string() { diff --git a/proxy/hdrs/HdrToken.cc b/proxy/hdrs/HdrToken.cc index 55fe5200a..a518972a3 100644 --- a/proxy/hdrs/HdrToken.cc +++ b/proxy/hdrs/HdrToken.cc @@ -225,7 +225,8 @@ static HdrTokenFieldInfo _hdrtoken_strs_field_initializers[] = { {"Subject", MIME_SLOTID_NONE, MIME_PRESENCE_SUBJECT, HTIF_NONE}, {"Summary", MIME_SLOTID_NONE, MIME_PRESENCE_SUMMARY, HTIF_NONE}, {"TE", MIME_SLOTID_TE, MIME_PRESENCE_TE, (HTIF_COMMAS | HTIF_MULTVALS | HTIF_HOPBYHOP)}, - {"Transfer-Encoding", MIME_SLOTID_TRANSFER_ENCODING, MIME_PRESENCE_TRANSFER_ENCODING, (HTIF_COMMAS | HTIF_MULTVALS)}, + {"Transfer-Encoding", MIME_SLOTID_TRANSFER_ENCODING, MIME_PRESENCE_TRANSFER_ENCODING, + (HTIF_COMMAS | HTIF_MULTVALS | HTIF_HOPBYHOP)}, {"Upgrade", MIME_SLOTID_NONE, MIME_PRESENCE_UPGRADE, (HTIF_COMMAS | HTIF_MULTVALS | HTIF_HOPBYHOP)}, {"User-Agent", MIME_SLOTID_USER_AGENT, MIME_PRESENCE_USER_AGENT, HTIF_NONE}, {"Vary", MIME_SLOTID_VARY, MIME_PRESENCE_VARY, (HTIF_COMMAS | HTIF_MULTVALS)}, diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index f22271487..87f0b98cf 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -1841,7 +1841,7 @@ HttpSM::state_read_server_response_header(int event, void *data) } // For requests that contain a body, we can cancel the ua inactivity timeout. - if (ua_txn && t_state.hdr_info.request_content_length) { + if (ua_txn && t_state.hdr_info.request_content_length > 0) { ua_txn->cancel_inactivity_timeout(); } } @@ -1977,7 +1977,8 @@ HttpSM::state_send_server_request_header(int event, void *data) server_entry->write_buffer = nullptr; method = t_state.hdr_info.server_request.method_get_wksidx(); if (!t_state.api_server_request_body_set && method != HTTP_WKSIDX_TRACE && - (t_state.hdr_info.request_content_length > 0 || t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING)) { + ua_txn->has_request_body(t_state.hdr_info.request_content_length, + t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING)) { if (post_transform_info.vc) { setup_transform_to_server_transfer(); } else { @@ -4791,8 +4792,8 @@ HttpSM::do_http_server_open(bool raw) set_server_session_private(true); } - if (raw == false && TS_SERVER_SESSION_SHARING_MATCH_NONE != t_state.txn_conf->server_session_sharing_match && - (t_state.txn_conf->keep_alive_post_out == 1 || t_state.hdr_info.request_content_length == 0) && !is_private() && + if ((raw == false) && TS_SERVER_SESSION_SHARING_MATCH_NONE != t_state.txn_conf->server_session_sharing_match && + (t_state.txn_conf->keep_alive_post_out == 1 || t_state.hdr_info.request_content_length <= 0) && !is_private() && ua_txn != nullptr) { HSMresult_t shared_result; shared_result = httpSessionManager.acquire_session(this, // state machine @@ -5603,7 +5604,8 @@ close_connection: void HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type) { - bool chunked = (t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING); + bool chunked = t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING || + t_state.hdr_info.request_content_length == HTTP_UNDEFINED_CL; bool post_redirect = false; HttpTunnelProducer *p = nullptr; @@ -5970,6 +5972,18 @@ HttpSM::attach_server_session(HttpServerSession *s) server_session->get_netvc()->set_active_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_active_timeout_out)); } + // Do we need Transfer_Encoding? + if (ua_txn->has_request_body(t_state.hdr_info.request_content_length, + t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING)) { + // See if we need to insert a chunked header + if (!t_state.hdr_info.server_request.presence(MIME_PRESENCE_CONTENT_LENGTH)) { + // Stuff in a TE setting so we treat this as chunked, sort of. + t_state.server_info.transfer_encoding = HttpTransact::CHUNKED_ENCODING; + t_state.hdr_info.server_request.value_append(MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING, HTTP_VALUE_CHUNKED, + HTTP_LEN_CHUNKED, true); + } + } + if (plugin_tunnel_type != HTTP_NO_PLUGIN_TUNNEL || will_be_private_ss) { SMDebug("http_ss", "Setting server session to private"); set_server_session_private(true); @@ -7325,7 +7339,7 @@ HttpSM::set_next_state() // light of this dependency, TS must ensure that the client finishes // sending its request and for this reason, the inactivity timeout // cannot be cancelled. - if (ua_txn && !t_state.hdr_info.request_content_length) { + if (ua_txn && t_state.hdr_info.request_content_length <= 0) { ua_txn->cancel_inactivity_timeout(); } else if (!ua_txn) { terminate_sm = true; @@ -7370,7 +7384,7 @@ HttpSM::set_next_state() // light of this dependency, TS must ensure that the client finishes // sending its request and for this reason, the inactivity timeout // cannot be cancelled. - if (ua_txn && !t_state.hdr_info.request_content_length) { + if (ua_txn && t_state.hdr_info.request_content_length <= 0) { ua_txn->cancel_inactivity_timeout(); } else if (!ua_txn) { terminate_sm = true; diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 0348b6521..c5a37a3b2 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -1183,7 +1183,8 @@ HttpTransact::HandleRequest(State *s) } } if (s->txn_conf->request_buffer_enabled && - (s->hdr_info.request_content_length > 0 || s->client_info.transfer_encoding == CHUNKED_ENCODING)) { + s->state_machine->ua_txn->has_request_body(s->hdr_info.request_content_length, + s->client_info.transfer_encoding == CHUNKED_ENCODING)) { TRANSACT_RETURN(SM_ACTION_WAIT_FOR_FULL_BODY, nullptr); } } @@ -5156,8 +5157,11 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr) // than in initialize_state_variables_from_request // ///////////////////////////////////////////////////// if (method != HTTP_WKSIDX_TRACE) { - int64_t length = incoming_hdr->get_content_length(); - s->hdr_info.request_content_length = (length >= 0) ? length : HTTP_UNDEFINED_CL; // content length less than zero is invalid + if (incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) { + s->hdr_info.request_content_length = incoming_hdr->get_content_length(); + } else { + s->hdr_info.request_content_length = HTTP_UNDEFINED_CL; // content length less than zero is invalid + } TxnDebug("http_trans", "[init_stat_vars_from_req] set req cont length to %" PRId64, s->hdr_info.request_content_length); @@ -5198,22 +5202,23 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr) if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) && (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) && s->client_info.transfer_encoding != CHUNKED_ENCODING) { - if (!incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) { - // In normal operation there will always be a ua_txn at this point, but in one of the -R1 regression tests a request is - // created - // independent of a transaction and this method is called, so we must null check - if (s->txn_conf->post_check_content_length_enabled && - (!s->state_machine->ua_txn || s->state_machine->ua_txn->is_chunked_encoding_supported())) { - return NO_POST_CONTENT_LENGTH; - } else { - // Stuff in a TE setting so we treat this as chunked, sort of. - s->client_info.transfer_encoding = HttpTransact::CHUNKED_ENCODING; - incoming_hdr->value_append(MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING, HTTP_VALUE_CHUNKED, HTTP_LEN_CHUNKED, - true); + // In normal operation there will always be a ua_txn at this point, but in one of the -R1 regression tests a request is + // createdindependent of a transaction and this method is called, so we must null check + if (!s->state_machine->ua_txn || s->state_machine->ua_txn->is_chunked_encoding_supported()) { + // See if we need to insert a chunked header + if (!incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) { + if (s->txn_conf->post_check_content_length_enabled) { + return NO_POST_CONTENT_LENGTH; + } else { + // Stuff in a TE setting so we treat this as chunked, sort of. + s->client_info.transfer_encoding = HttpTransact::CHUNKED_ENCODING; + incoming_hdr->value_append(MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING, HTTP_VALUE_CHUNKED, + HTTP_LEN_CHUNKED, true); + } + } + if (HTTP_UNDEFINED_CL == s->hdr_info.request_content_length) { + return INVALID_POST_CONTENT_LENGTH; } - } - if (HTTP_UNDEFINED_CL == s->hdr_info.request_content_length) { - return INVALID_POST_CONTENT_LENGTH; } } } diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index adf7fc1de..308c5d5d5 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -241,6 +241,9 @@ rcv_headers_frame(Http2ConnectionState &cstate, const Http2Frame &frame) if (stream == nullptr || !stream->has_trailing_header()) { return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_STREAM_CLOSED, "recv headers cannot find existing stream_id"); + } else if (stream->get_state() == Http2StreamState::HTTP2_STREAM_STATE_CLOSED) { + return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_STREAM_CLOSED, + "recv_header to closed stream"); } } else { // Create new stream @@ -1784,6 +1787,7 @@ Http2ConnectionState::send_push_promise_frame(Http2Stream *stream, URL &url, con stream->change_state(HTTP2_FRAME_TYPE_PUSH_PROMISE, HTTP2_FLAGS_PUSH_PROMISE_END_HEADERS); stream->set_request_headers(h2_hdr); stream->new_transaction(); + stream->recv_end_stream = true; // No more data with the request stream->send_request(*this); h2_hdr.destroy(); diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc index 6cbfe0a0c..46486bc1e 100644 --- a/proxy/http2/Http2Stream.cc +++ b/proxy/http2/Http2Stream.cc @@ -197,6 +197,8 @@ Http2Stream::send_request(Http2ConnectionState &cstate) this->read_vio.nbytes = bufindex; this->signal_read_event(VC_EVENT_READ_COMPLETE); } else { + // End of header but not end of stream, must have some body frames coming + this->has_body = true; this->signal_read_event(VC_EVENT_READ_READY); } } @@ -1032,3 +1034,9 @@ Http2Stream::read_vio_read_avail() return 0; } + +bool +Http2Stream::has_request_body(int64_t content_length, bool is_chunked_set) const +{ + return has_body; +} diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h index e544339db..cff0def67 100644 --- a/proxy/http2/Http2Stream.h +++ b/proxy/http2/Http2Stream.h @@ -111,6 +111,8 @@ public: bool is_closed() const; IOBufferReader *response_get_data_reader() const; + bool has_request_body(int64_t content_length, bool is_chunked_set) const override; + void mark_milestone(Http2StreamMilestone type); void increment_data_length(uint64_t length); @@ -173,6 +175,7 @@ private: Milestones<Http2StreamMilestone, static_cast<size_t>(Http2StreamMilestone::LAST_ENTRY)> _milestones; bool trailing_header = false; + bool has_body = false; // A brief disucssion of similar flags and state variables: _state, closed, terminate_stream // diff --git a/tests/gold_tests/h2/http2.test.py b/tests/gold_tests/h2/http2.test.py index b28d13432..22ed0da7f 100644 --- a/tests/gold_tests/h2/http2.test.py +++ b/tests/gold_tests/h2/http2.test.py @@ -50,14 +50,14 @@ server.addResponse("sessionlog.json", # For Test Case 6 - /postchunked post_body = "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" -server.addResponse("sessionlog.jason", +server.addResponse("sessionlog.json", {"headers": "POST /postchunked HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": post_body}, {"headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\nContent-Length: 10\r\n\r\n", "timestamp": "1469733493.993", "body": "0123456789"}) # For Test Case 7 - /bigpostchunked # Make a post body that will be split across at least two frames big_post_body = "0123456789" * 131070 -server.addResponse("sessionlog.jason", +server.addResponse("sessionlog.json", {"headers": "POST /bigpostchunked HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": big_post_body}, {"headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\nContent-Length: 10\r\n\r\n", "timestamp": "1469733493.993", "body": "0123456789"})