You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2018/03/13 23:02:07 UTC
[trafficserver] branch 7.1.x updated: Drain the request body if
there is a cache hit. Only drain if the request body has a Content-Length
and all the bytes are in the buffer. Otherwise,
close the connection after the response.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/7.1.x by this push:
new 3d2fdab Drain the request body if there is a cache hit. Only drain if the request body has a Content-Length and all the bytes are in the buffer. Otherwise, close the connection after the response.
3d2fdab is described below
commit 3d2fdab8b0606bc8b35006f7aeb73729d364b333
Author: Bryan Call <bc...@apache.org>
AuthorDate: Wed Mar 7 14:04:25 2018 -0800
Drain the request body if there is a cache hit. Only drain if the
request body has a Content-Length and all the bytes are in the buffer.
Otherwise, close the connection after the response.
(cherry picked from commit c4e4379a615b2a3252c6febfdb72afd9db5ee6ac)
Conflicts:
proxy/http/HttpDebugNames.cc
proxy/http/HttpSM.cc
proxy/http/HttpTransact.h
---
proxy/http/HttpDebugNames.cc | 5 -
proxy/http/HttpSM.cc | 103 +++++----------------
proxy/http/HttpSM.h | 5 -
proxy/http/HttpTransact.h | 4 -
.../gold_tests/headers/cache_and_req_body-hit.gold | 11 +++
.../headers/cache_and_req_body-hit_close.gold | 11 +++
.../headers/cache_and_req_body-miss.gold | 11 +++
.../gold_tests/headers/cache_and_req_body.test.py | 85 +++++++++++++++++
8 files changed, 141 insertions(+), 94 deletions(-)
diff --git a/proxy/http/HttpDebugNames.cc b/proxy/http/HttpDebugNames.cc
index b9e5263..a25ce3b 100644
--- a/proxy/http/HttpDebugNames.cc
+++ b/proxy/http/HttpDebugNames.cc
@@ -367,11 +367,6 @@ HttpDebugNames::get_action_name(HttpTransact::StateMachineAction_t e)
case HttpTransact::SM_ACTION_TRANSFORM_READ:
return ("SM_ACTION_TRANSFORM_READ");
-#ifdef PROXY_DRAIN
- case HttpTransact::SM_ACTION_DRAIN_REQUEST_BODY:
- return ("SM_ACTION_DRAIN_REQUEST_BODY");
-#endif /* PROXY_DRAIN */
-
case HttpTransact::SM_ACTION_API_SM_START:
return ("SM_ACTION_API_SM_START");
case HttpTransact::SM_ACTION_REDIRECT_READ:
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 7658624..0b2737c 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -829,63 +829,6 @@ HttpSM::state_read_client_request_header(int event, void *data)
return 0;
}
-#ifdef PROXY_DRAIN
-int
-HttpSM::state_drain_client_request_body(int event, void *data)
-{
- STATE_ENTER(&HttpSM::state_drain_client_request_body, event);
-
- ink_assert(ua_entry->read_vio == (VIO *)data);
- ink_assert(ua_entry->vc == ua_session);
-
- NetVConnection *netvc = ua_session->get_netvc();
- if (!netvc && event != VC_EVENT_EOS)
- return 0;
-
- switch (event) {
- case VC_EVENT_EOS:
- case VC_EVENT_ERROR:
- case VC_EVENT_ACTIVE_TIMEOUT:
- case VC_EVENT_INACTIVITY_TIMEOUT: {
- // Nothing we can do
- terminate_sm = true;
- break;
- }
- case VC_EVENT_READ_READY: {
- int64_t avail = ua_buffer_reader->read_avail();
- int64_t left = t_state.hdr_info.request_content_length - client_request_body_bytes;
-
- // Since we are only reading what's needed to complete
- // the post, there must be something left to do
- ink_assert(avail < left);
-
- client_request_body_bytes += avail;
- ua_buffer_reader->consume(avail);
- ua_entry->read_vio->reenable_re();
- break;
- }
- case VC_EVENT_READ_COMPLETE: {
- // We've finished draing the POST body
- int64_t avail = ua_buffer_reader->read_avail();
-
- ua_buffer_reader->consume(avail);
- client_request_body_bytes += avail;
- ink_assert(client_request_body_bytes == t_state.hdr_info.request_content_length);
-
- ua_buffer_reader->mbuf->size_index = HTTP_HEADER_BUFFER_SIZE_INDEX;
- ua_entry->vc_handler = &HttpSM::state_watch_for_client_abort;
- ua_entry->read_vio = ua_entry->vc->do_io_read(this, INT64_MAX, ua_buffer_reader->mbuf);
- call_transact_and_set_next_state(NULL);
- break;
- }
- default:
- ink_release_assert(0);
- }
-
- return EVENT_DONE;
-}
-#endif /* PROXY_DRAIN */
-
int
HttpSM::state_watch_for_client_abort(int event, void *data)
{
@@ -5669,29 +5612,34 @@ HttpSM::setup_transform_to_server_transfer()
tunnel.tunnel_run(p);
}
-#ifdef PROXY_DRAIN
void
HttpSM::do_drain_request_body()
{
- int64_t post_bytes = t_state.hdr_info.request_content_length;
- int64_t avail = ua_buffer_reader->read_avail();
-
- int64_t act_on = (avail < post_bytes) ? avail : post_bytes;
-
- client_request_body_bytes = act_on;
- ua_buffer_reader->consume(act_on);
+ int64_t content_length = t_state.hdr_info.client_request.get_content_length();
+ int64_t avail = ua_buffer_reader->read_avail();
- ink_assert(client_request_body_bytes <= post_bytes);
+ if (t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING) {
+ Debug("http", "Chunked body, setting the response to non-keepalive");
+ goto close_connection;
+ }
- if (client_request_body_bytes < post_bytes) {
- ua_buffer_reader->mbuf->size_index = buffer_size_to_index(t_state.hdr_info.request_content_length);
- ua_entry->vc_handler = &HttpSM::state_drain_client_request_body;
- ua_entry->read_vio = ua_entry->vc->do_io_read(this, post_bytes - client_request_body_bytes, ua_buffer_reader->mbuf);
- } else {
- call_transact_and_set_next_state(NULL);
+ if (content_length > 0) {
+ if (avail >= content_length) {
+ Debug("http", "entire body is in the buffer, consuming");
+ int64_t act_on = (avail < content_length) ? avail : content_length;
+ client_request_body_bytes = act_on;
+ ua_buffer_reader->consume(act_on);
+ } else {
+ Debug("http", "entire body is not in the buffer, setting the response to non-keepalive");
+ goto close_connection;
+ }
}
+ return;
+
+close_connection:
+ t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE;
+ t_state.hdr_info.client_response.value_set(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION, "close", 5);
}
-#endif /* PROXY_DRAIN */
void
HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type)
@@ -7531,6 +7479,8 @@ HttpSM::set_next_state()
release_server_session(true);
t_state.source = HttpTransact::SOURCE_CACHE;
+ do_drain_request_body();
+
if (transform_info.vc) {
ink_assert(t_state.hdr_info.client_response.valid() == 0);
ink_assert((t_state.hdr_info.transform_response.valid() ? true : false) == true);
@@ -7721,13 +7671,6 @@ HttpSM::set_next_state()
break;
}
-#ifdef PROXY_DRAIN
- case HttpTransact::SM_ACTION_DRAIN_REQUEST_BODY: {
- do_drain_request_body();
- break;
- }
-#endif /* PROXY_DRAIN */
-
case HttpTransact::SM_ACTION_CONTINUE: {
ink_release_assert(!"Not implemented");
break;
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index 6ce6ece..8f18cc3 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -383,9 +383,6 @@ protected:
int tunnel_handler_100_continue(int event, void *data);
int tunnel_handler_cache_fill(int event, void *data);
-#ifdef PROXY_DRAIN
- int state_drain_client_request_body(int event, void *data);
-#endif /* PROXY_DRAIN */
int state_read_client_request_header(int event, void *data);
int state_watch_for_client_abort(int event, void *data);
int state_read_push_response_header(int event, void *data);
@@ -452,9 +449,7 @@ protected:
void do_api_callout_internal();
void do_redirect();
void redirect_request(const char *redirect_url, const int redirect_len);
-#ifdef PROXY_DRAIN
void do_drain_request_body();
-#endif
bool do_congestion_control_lookup();
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 20ca74d..83f7689 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -472,10 +472,6 @@ public:
SM_ACTION_INTERNAL_REQUEST,
SM_ACTION_SEND_ERROR_CACHE_NOOP,
-#ifdef PROXY_DRAIN
- SM_ACTION_DRAIN_REQUEST_BODY,
-#endif /* PROXY_DRAIN */
-
SM_ACTION_SERVE_FROM_CACHE,
SM_ACTION_SERVER_READ,
SM_ACTION_SERVER_PARSE_NEXT_HDR,
diff --git a/tests/gold_tests/headers/cache_and_req_body-hit.gold b/tests/gold_tests/headers/cache_and_req_body-hit.gold
new file mode 100644
index 0000000..46791e7
--- /dev/null
+++ b/tests/gold_tests/headers/cache_and_req_body-hit.gold
@@ -0,0 +1,11 @@
+HTTP/1.1 200 OK
+Cache-Control: max-age=300
+Content-Length: 3
+Date: ``
+Age: ``
+Connection: keep-alive
+Via: ``
+Server: ``
+X-Cache: hit-fresh
+
+xxx
diff --git a/tests/gold_tests/headers/cache_and_req_body-hit_close.gold b/tests/gold_tests/headers/cache_and_req_body-hit_close.gold
new file mode 100644
index 0000000..5695558
--- /dev/null
+++ b/tests/gold_tests/headers/cache_and_req_body-hit_close.gold
@@ -0,0 +1,11 @@
+HTTP/1.1 200 OK
+Cache-Control: max-age=300
+Content-Length: 3
+Date: ``
+Age: ``
+Connection: close
+Via: ``
+Server: ``
+X-Cache: hit-fresh
+
+xxx
diff --git a/tests/gold_tests/headers/cache_and_req_body-miss.gold b/tests/gold_tests/headers/cache_and_req_body-miss.gold
new file mode 100644
index 0000000..41b57d9
--- /dev/null
+++ b/tests/gold_tests/headers/cache_and_req_body-miss.gold
@@ -0,0 +1,11 @@
+HTTP/1.1 200 OK
+Cache-Control: max-age=300
+Content-Length: 3
+Date: ``
+Age: 0
+Connection: keep-alive
+Via: ``
+Server: ``
+X-Cache: miss
+
+xxx
diff --git a/tests/gold_tests/headers/cache_and_req_body.test.py b/tests/gold_tests/headers/cache_and_req_body.test.py
new file mode 100644
index 0000000..22a307d
--- /dev/null
+++ b/tests/gold_tests/headers/cache_and_req_body.test.py
@@ -0,0 +1,85 @@
+'''
+Test cached responses and requests with bodies
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+Test.Summary = '''
+Test cached responses and requests with bodies
+'''
+
+# Needs Curl
+Test.SkipUnless(
+ Condition.HasProgram("curl", "curl needs to be installed on system for this test to work"),
+ Condition.HasProgram("nc", "nc needs to be installed on system for this test to work")
+)
+Test.ContinueOnFail = True
+
+# Define default ATS
+ts = Test.MakeATSProcess("ts", select_ports=False)
+server = Test.MakeOriginServer("server")
+
+#**testname is required**
+testName = ""
+request_header = {"headers": "GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\nCache-Control: max-age=300\r\n\r\n", "timestamp": "1469733493.993", "body": "xxx"}
+server.addResponse("sessionlog.json", request_header, response_header)
+
+# ATS Configuration
+ts.Disk.plugin_config.AddLine('xdebug.so')
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.http.response_via_str': 3,
+ 'proxy.config.http.cache.http': 1,
+ 'proxy.config.http.wait_for_cache': 1,
+})
+
+ts.Disk.remap_config.AddLine(
+ 'map / http://127.0.0.1:{0}'.format(server.Variables.Port)
+)
+
+# Test 1 - 200 response and cache fill
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.StartBefore(Test.Processes.ts, ready=1)
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "x-debug: x-cache,x-cache-key,via" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "cache_and_req_body-miss.gold"
+tr.StillRunningAfter = ts
+
+# Test 2 - 200 cached response and using netcat
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = "printf 'GET / HTTP/1.1\r\n''x-debug: x-cache,x-cache-key,via\r\n''Host: www.example.com\r\n''\r\n'|nc 127.0.0.1 8080"
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "cache_and_req_body-hit.gold"
+tr.StillRunningAfter = ts
+
+# Test 3 - 200 cached response and trying to hide a request in the body
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = "printf 'GET / HTTP/1.1\r\n''x-debug: x-cache,x-cache-key,via\r\n''Host: www.example.com\r\n''Content-Length: 71\r\n''\r\n''GET /index.html?evil=zorg810 HTTP/1.1\r\n''Host: dummy-host.example.com\r\n''\r\n'|nc 127.0.0.1 8080"
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "cache_and_req_body-hit.gold"
+tr.StillRunningAfter = ts
+
+# Test 4 - 200 cached response and Content-Length larger than bytes sent, MUST close
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = "printf 'GET / HTTP/1.1\r\n''x-debug: x-cache,x-cache-key,via\r\n''Host: dummy-host.example.com\r\n''Cache-control: max-age=300\r\n''Content-Length: 100\r\n''\r\n''GET /index.html?evil=zorg810 HTTP/1.1\r\n''Host: dummy-host.example.com\r\n''\r\n'|nc 127.0.0.1 8080"
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "cache_and_req_body-hit_close.gold"
+tr.StillRunningAfter = ts
+
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.