You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2021/02/16 15:35:20 UTC
[trafficserver] branch master updated: Move has_request_body to
ProxyTransaction (#7499)
This is an automated email from the ASF dual-hosted git repository.
shinrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new d8f3970 Move has_request_body to ProxyTransaction (#7499)
d8f3970 is described below
commit d8f39709cfd5c71c31fa0e4acd7c5d6cd27b1fce
Author: Susan Hinrichs <sh...@yahoo-inc.com>
AuthorDate: Tue Feb 16 09:35:03 2021 -0600
Move has_request_body to ProxyTransaction (#7499)
---
proxy/ProxyTransaction.cc | 6 ++++++
proxy/ProxyTransaction.h | 3 +++
proxy/http/HttpSM.cc | 17 ++++++++++-------
proxy/http/HttpTransact.cc | 13 +++----------
proxy/http/HttpTransact.h | 2 --
proxy/http2/Http2ConnectionState.cc | 6 +++++-
proxy/http2/Http2Stream.cc | 8 ++++++++
proxy/http2/Http2Stream.h | 3 +++
proxy/http3/Http3Transaction.cc | 7 +++++++
proxy/http3/Http3Transaction.h | 3 +++
10 files changed, 48 insertions(+), 20 deletions(-)
diff --git a/proxy/ProxyTransaction.cc b/proxy/ProxyTransaction.cc
index 3f80e77..15601f4 100644
--- a/proxy/ProxyTransaction.cc
+++ b/proxy/ProxyTransaction.cc
@@ -215,3 +215,9 @@ ProxyTransaction::reenable(VIO *vio)
{
_proxy_ssn->reenable(vio);
}
+
+bool
+ProxyTransaction::has_request_body(int64_t request_content_length, bool is_chunked) const
+{
+ return request_content_length > 0 || is_chunked;
+}
diff --git a/proxy/ProxyTransaction.h b/proxy/ProxyTransaction.h
index 7c1e3de..e264ab9 100644
--- a/proxy/ProxyTransaction.h
+++ b/proxy/ProxyTransaction.h
@@ -83,6 +83,9 @@ public:
virtual void set_proxy_ssn(ProxySession *set_proxy_ssn);
+ // 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;
+
/// Non-Virtual Methods
//
const char *get_protocol_string();
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index a2eb853..6af9602 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1990,7 +1990,7 @@ HttpSM::state_read_server_response_header(int event, void *data)
server_session->set_inactivity_timeout(get_server_inactivity_timeout());
// 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();
}
}
@@ -2110,7 +2110,9 @@ HttpSM::state_send_server_request_header(int event, void *data)
free_MIOBuffer(server_entry->write_buffer);
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 && HttpTransact::has_request_body(&t_state, ua_txn)) {
+ if (!t_state.api_server_request_body_set && method != HTTP_WKSIDX_TRACE &&
+ 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 {
@@ -3620,7 +3622,7 @@ HttpSM::tunnel_handler_post_ua(int event, HttpTunnelProducer *p)
// Now that we have communicated the post body, turn off the inactivity timeout
// until the server starts sending data back
- 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();
}
@@ -5033,7 +5035,7 @@ HttpSM::do_http_server_open(bool raw)
}
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() &&
+ (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
@@ -6152,7 +6154,8 @@ HttpSM::attach_server_session(PoolableSession *s)
server_session->set_active_timeout(get_server_active_timeout());
// Do we need Transfer_Encoding?
- if (HttpTransact::has_request_body(&t_state, ua_txn)) {
+ 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.
@@ -7497,7 +7500,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;
@@ -7542,7 +7545,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 279c532..6e53a9a 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -1501,7 +1501,9 @@ HttpTransact::HandleRequest(State *s)
}
}
}
- if (s->txn_conf->request_buffer_enabled && has_request_body(s, s->state_machine->ua_txn)) {
+ if (s->txn_conf->request_buffer_enabled &&
+ 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);
}
}
@@ -8849,15 +8851,6 @@ HttpTransact::change_response_header_because_of_range_request(State *s, HTTPHdr
}
}
-bool
-HttpTransact::has_request_body(State *s, ProxyTransaction *txn)
-{
- int method = s->hdr_info.client_request.method_get_wksidx();
- return (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
- (s->hdr_info.request_content_length > 0 || s->client_info.transfer_encoding == CHUNKED_ENCODING ||
- !txn->is_chunked_encoding_supported());
-}
-
#if TS_HAS_TESTS
void forceLinkRegressionHttpTransact();
void
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 8cc53ba..686d8b4 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -1018,8 +1018,6 @@ public:
static bool is_cache_hit(CacheLookupResult_t r);
static bool is_fresh_cache_hit(CacheLookupResult_t r);
- static bool has_request_body(State *s, ProxyTransaction *txn);
-
static void build_request(State *s, HTTPHdr *base_request, HTTPHdr *outgoing_request, HTTPVersion outgoing_version);
static void build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing_response, HTTPVersion outgoing_version,
HTTPStatus status_code, const char *reason_phrase = nullptr);
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index d1256cd..1d68105 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -245,9 +245,12 @@ rcv_headers_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
if (stream == nullptr) {
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 if (!stream->has_trailing_header()) {
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
- "recv headers cannot find existing stream_id");
+ "stream not expecting trailer header");
}
} else {
// Create new stream
@@ -1798,6 +1801,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(hdr);
stream->new_transaction();
+ stream->recv_end_stream = true; // No more data with the request
stream->send_request(*this);
return true;
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index acb955a..c45a997 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -191,6 +191,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);
}
}
@@ -1012,3 +1014,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 cb48229..544cd1e 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -115,6 +115,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);
@@ -176,6 +178,7 @@ private:
Milestones<Http2StreamMilestone, static_cast<size_t>(Http2StreamMilestone::LAST_ENTRY)> _milestones;
bool trailing_header = false;
+ bool has_body = false;
// A brief discussion of similar flags and state variables: _state, closed, terminate_stream
//
diff --git a/proxy/http3/Http3Transaction.cc b/proxy/http3/Http3Transaction.cc
index ccb43a6..1be472c 100644
--- a/proxy/http3/Http3Transaction.cc
+++ b/proxy/http3/Http3Transaction.cc
@@ -603,6 +603,13 @@ Http3Transaction::_on_qpack_decode_complete()
return 1;
}
+// TODO: Just a place holder for now
+bool
+Http3Transaction::has_request_body(int64_t content_length, bool is_chunked_set) const
+{
+ return false;
+}
+
//
// Http09Transaction
//
diff --git a/proxy/http3/Http3Transaction.h b/proxy/http3/Http3Transaction.h
index 5eebfba..033a052 100644
--- a/proxy/http3/Http3Transaction.h
+++ b/proxy/http3/Http3Transaction.h
@@ -106,6 +106,9 @@ public:
bool is_response_header_sent() const;
bool is_response_body_sent() const;
+ // TODO: Just a place holder for now
+ bool has_request_body(int64_t content_length, bool is_chunked_set) const override;
+
private:
int64_t _process_read_vio() override;
int64_t _process_write_vio() override;