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
zw...@apache.org.

Reply via email to