Drain the request body if there is a cache hit

proxy/http/HttpDebugNames.cc

@@ -347,13 +347,9 @@ 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_WAIT_FOR_FULL_BODY:
     return ("SM_ACTION_WAIT_FOR_FULL_BODY");
+
   case HttpTransact::SM_ACTION_REQUEST_BUFFER_READ_COMPLETE:
     return ("SM_ACTION_REQUEST_BUFFER_READ_COMPLETE");
   case HttpTransact::SM_ACTION_API_SM_START:

proxy/http/HttpSM.cc

@@ -781,63 +781,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_txn);
-
-  NetVConnection *netvc = ua_txn->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 */
-
 void
 HttpSM::wait_for_full_body()
 {
@@ -5617,29 +5560,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) {
+    SMDebug("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) {
+      SMDebug("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 {
+      SMDebug("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)
@@ -7470,6 +7418,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);
@@ -7646,13 +7596,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_WAIT_FOR_FULL_BODY: {
     wait_for_full_body();
     break;

proxy/http/HttpSM.h

@@ -405,9 +405,6 @@ class HttpSM : public Continuation
 
   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);
@@ -471,9 +468,7 @@ class HttpSM : public Continuation
   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
 
   void wait_for_full_body();
 

proxy/http/HttpTransact.h

@@ -421,10 +421,6 @@ class HttpTransact
     SM_ACTION_INTERNAL_REQUEST,
     SM_ACTION_SEND_ERROR_CACHE_NOOP,
 
-#ifdef PROXY_DRAIN
-    SM_ACTION_DRAIN_REQUEST_BODY,
-#endif /* PROXY_DRAIN */
-
     SM_ACTION_WAIT_FOR_FULL_BODY,
     SM_ACTION_REQUEST_BUFFER_READ_COMPLETE,
     SM_ACTION_SERVE_FROM_CACHE,

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

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

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

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
+