You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ez...@apache.org on 2022/06/03 15:52:10 UTC
[trafficserver] branch 8.1.x updated: 8.1.x Backport: Move has_request_body to ProxyTransaction (#7499) (#8880)
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 <ro...@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 <sh...@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"})