trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a..@apache.org
Subject [trafficserver] branch master updated: Fixes transfers for error responses without body
Date Tue, 27 Jun 2017 16:39:38 GMT
This is an automated email from the ASF dual-hosted git repository.

amc 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 ea60fd3  Fixes transfers for error responses without body
ea60fd3 is described below

commit ea60fd3fd97d7e443ae0368afa6f5ec839ccc964
Author: Derek Dagit <derekd@yahoo-inc.com>
AuthorDate: Tue Jun 27 03:31:34 2017 +0000

    Fixes transfers for error responses without body
---
 proxy/http/HttpBodyFactory.cc                      |   6 +-
 proxy/http/HttpSM.cc                               |   4 +-
 proxy/http/HttpTransact.cc                         |   3 +-
 proxy/http/HttpTransact.h                          |  34 ++++---
 .../body_factory/data/www.default304.test_get.txt  |   2 +
 .../body_factory/data/www.example.test_get_200.txt |   3 +
 .../body_factory/data/www.example.test_get_304.txt |   4 +
 .../body_factory/data/www.example.test_head.txt    |   3 +
 .../data/www.example.test_head_200.txt             |   3 +
 tests/gold_tests/body_factory/gold/http-304.gold   |   4 +
 .../gold_tests/body_factory/gold/http-get-200.gold |   6 ++
 .../gold_tests/body_factory/gold/http-get-304.gold |   5 +
 .../body_factory/gold/http-head-200.gold           |   4 +
 .../body_factory/gold/http-head-no-origin.gold     |   7 ++
 .../body_factory/http304_response.test.py          |  56 +++++++++++
 .../body_factory/http_head_no_origin.test.py       |  45 +++++++++
 .../body_factory/http_with_origin.test.py          | 108 +++++++++++++++++++++
 17 files changed, 277 insertions(+), 20 deletions(-)

diff --git a/proxy/http/HttpBodyFactory.cc b/proxy/http/HttpBodyFactory.cc
index ea09481..0d496e2 100644
--- a/proxy/http/HttpBodyFactory.cc
+++ b/proxy/http/HttpBodyFactory.cc
@@ -138,7 +138,7 @@ HttpBodyFactory::fabricate_with_old_api(const char *type, HttpTransact::State
*c
   // if failed, try to fabricate the default custom response //
   /////////////////////////////////////////////////////////////
   if (buffer == nullptr) {
-    if (is_response_body_precluded(context->http_return_code, context->method)) {
+    if (is_response_body_precluded(context->http_return_code)) {
       *resulting_buffer_length = 0;
       unlock();
       return nullptr;
@@ -411,7 +411,7 @@ HttpBodyFactory::fabricate(StrList *acpt_language_list, StrList *acpt_charset_li
     set = determine_set_by_language(acpt_language_list, acpt_charset_list);
   } else if (enable_customizations == 3) {
     set = determine_set_by_host(context);
-  } else if (is_response_body_precluded(context->http_return_code, context->method))
{
+  } else if (is_response_body_precluded(context->http_return_code)) {
     return nullptr;
   } else {
     set = "default";
@@ -431,7 +431,7 @@ HttpBodyFactory::fabricate(StrList *acpt_language_list, StrList *acpt_charset_li
 
   // Check for base customizations if specializations didn't work.
   if (t == nullptr) {
-    if (is_response_body_precluded(context->http_return_code, context->method)) {
+    if (is_response_body_precluded(context->http_return_code)) {
       return nullptr;
     }
     t = find_template(set, type, &body_set); // this executes if the template_base is
wrong and doesn't exist
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 1ad43b2..7d2859f 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -6119,10 +6119,10 @@ HttpSM::setup_100_continue_transfer()
 void
 HttpSM::setup_error_transfer()
 {
-  if (t_state.internal_msg_buffer || t_state.http_return_code == HTTP_STATUS_NO_CONTENT)
{
+  if (t_state.internal_msg_buffer || is_response_body_precluded(t_state.http_return_code))
{
     // Since we need to send the error message, call the API
     //   function
-    ink_assert(t_state.internal_msg_buffer_size > 0 || t_state.http_return_code == HTTP_STATUS_NO_CONTENT);
+    ink_assert(t_state.internal_msg_buffer_size > 0 || is_response_body_precluded(t_state.http_return_code));
     t_state.api_next_action = HttpTransact::SM_ACTION_API_SEND_RESPONSE_HDR;
     do_api_callout();
   } else {
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index f3c0326..bd4adfd 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -8031,8 +8031,7 @@ HttpTransact::build_error_response(State *s, HTTPStatus status_code,
const char
   s->internal_msg_buffer_size                = len;
   s->internal_msg_buffer_fast_allocator_size = -1;
 
-  if (!is_response_body_precluded(status_code, s->method) || len > 0) {
-    // Plugins may create response bodies despite an HTTP spec violation.
+  if (len > 0) {
     s->hdr_info.client_response.value_set(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE,
body_type, strlen(body_type));
     s->hdr_info.client_response.value_set(MIME_FIELD_CONTENT_LANGUAGE, MIME_LEN_CONTENT_LANGUAGE,
body_language,
                                           strlen(body_language));
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index ed1956a..4a104f7 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -1149,23 +1149,31 @@ public:
 
 typedef void (*TransactEntryFunc_t)(HttpTransact::State *s);
 
+////////////////////////////////////////////////////////
+// the spec says about message body the following:    //
+// All responses to the HEAD request method MUST NOT  //
+// include a message-body, even though the presence   //
+// of entity-header fields might lead one to believe  //
+// they do. All 1xx (informational), 204 (no content),//
+// and 304 (not modified) responses MUST NOT include  //
+// a message-body.                                    //
+////////////////////////////////////////////////////////
 inline bool
-is_response_body_precluded(HTTPStatus status_code, int method)
+is_response_body_precluded(HTTPStatus status_code)
 {
-  ////////////////////////////////////////////////////////
-  // the spec says about message body the following:    //
-  // All responses to the HEAD request method MUST NOT  //
-  // include a message-body, even though the presence   //
-  // of entity-header fields might lead one to believe  //
-  // they do. All 1xx (informational), 204 (no content),//
-  // and 304 (not modified) responses MUST NOT include  //
-  // a message-body.                                    //
-  ////////////////////////////////////////////////////////
-
   if (((status_code != HTTP_STATUS_OK) &&
        ((status_code == HTTP_STATUS_NOT_MODIFIED) || ((status_code < HTTP_STATUS_OK) &&
(status_code >= HTTP_STATUS_CONTINUE)) ||
-        (status_code == HTTP_STATUS_NO_CONTENT))) ||
-      (method == HTTP_WKSIDX_HEAD)) {
+        (status_code == HTTP_STATUS_NO_CONTENT)))) {
+    return true;
+  } else {
+    return false;
+  }
+}
+
+inline bool
+is_response_body_precluded(HTTPStatus status_code, int method)
+{
+  if ((method == HTTP_WKSIDX_HEAD) || is_response_body_precluded(status_code)) {
     return true;
   } else {
     return false;
diff --git a/tests/gold_tests/body_factory/data/www.default304.test_get.txt b/tests/gold_tests/body_factory/data/www.default304.test_get.txt
new file mode 100644
index 0000000..c9064fa
--- /dev/null
+++ b/tests/gold_tests/body_factory/data/www.default304.test_get.txt
@@ -0,0 +1,2 @@
+GET HTTP://www.default304.test/ HTTP/1.1
+
diff --git a/tests/gold_tests/body_factory/data/www.example.test_get_200.txt b/tests/gold_tests/body_factory/data/www.example.test_get_200.txt
new file mode 100644
index 0000000..c53681f
--- /dev/null
+++ b/tests/gold_tests/body_factory/data/www.example.test_get_200.txt
@@ -0,0 +1,3 @@
+GET /get200 HTTP/1.1
+Host: www.example.test
+
diff --git a/tests/gold_tests/body_factory/data/www.example.test_get_304.txt b/tests/gold_tests/body_factory/data/www.example.test_get_304.txt
new file mode 100644
index 0000000..8d0aecf
--- /dev/null
+++ b/tests/gold_tests/body_factory/data/www.example.test_get_304.txt
@@ -0,0 +1,4 @@
+GET /get304 HTTP/1.1
+Host: www.example.test
+If-Modified-Since: Thu, 1 Jan 1970 00:00:00 GMT
+
diff --git a/tests/gold_tests/body_factory/data/www.example.test_head.txt b/tests/gold_tests/body_factory/data/www.example.test_head.txt
new file mode 100644
index 0000000..c5a5c97
--- /dev/null
+++ b/tests/gold_tests/body_factory/data/www.example.test_head.txt
@@ -0,0 +1,3 @@
+HEAD http://www.example.test/ HTTP/1.1
+Host: www.example.test
+
diff --git a/tests/gold_tests/body_factory/data/www.example.test_head_200.txt b/tests/gold_tests/body_factory/data/www.example.test_head_200.txt
new file mode 100644
index 0000000..6d214e1
--- /dev/null
+++ b/tests/gold_tests/body_factory/data/www.example.test_head_200.txt
@@ -0,0 +1,3 @@
+HEAD /head200 HTTP/1.1
+Host: www.example.test
+
diff --git a/tests/gold_tests/body_factory/gold/http-304.gold b/tests/gold_tests/body_factory/gold/http-304.gold
new file mode 100644
index 0000000..1931f8b
--- /dev/null
+++ b/tests/gold_tests/body_factory/gold/http-304.gold
@@ -0,0 +1,4 @@
+HTTP/1.1 304 Not Modified
+Connection: keep-alive
+Cache-Control: no-store
+
diff --git a/tests/gold_tests/body_factory/gold/http-get-200.gold b/tests/gold_tests/body_factory/gold/http-get-200.gold
new file mode 100644
index 0000000..a5c3c38
--- /dev/null
+++ b/tests/gold_tests/body_factory/gold/http-get-200.gold
@@ -0,0 +1,6 @@
+HTTP/1.1 200 OK
+Content-Length: 47
+Age: 0
+Connection: keep-alive
+
+This body should be returned for a GET request.
diff --git a/tests/gold_tests/body_factory/gold/http-get-304.gold b/tests/gold_tests/body_factory/gold/http-get-304.gold
new file mode 100644
index 0000000..b737519
--- /dev/null
+++ b/tests/gold_tests/body_factory/gold/http-get-304.gold
@@ -0,0 +1,5 @@
+HTTP/1.1 304 Not Modified
+Age: 0
+Connection: keep-alive
+Warning: 199 ApacheTrafficServer/8.0.0 Proxy received unexpected 304 response; content may
be stale
+
diff --git a/tests/gold_tests/body_factory/gold/http-head-200.gold b/tests/gold_tests/body_factory/gold/http-head-200.gold
new file mode 100644
index 0000000..045c1e1
--- /dev/null
+++ b/tests/gold_tests/body_factory/gold/http-head-200.gold
@@ -0,0 +1,4 @@
+HTTP/1.1 200 OK
+Age: 0
+Connection: keep-alive
+
diff --git a/tests/gold_tests/body_factory/gold/http-head-no-origin.gold b/tests/gold_tests/body_factory/gold/http-head-no-origin.gold
new file mode 100644
index 0000000..6e1d0fd
--- /dev/null
+++ b/tests/gold_tests/body_factory/gold/http-head-no-origin.gold
@@ -0,0 +1,7 @@
+HTTP/1.1 404 Not Found
+Connection: keep-alive
+Cache-Control: no-store
+Content-Type: text/html
+Content-Language: en
+Content-Length: 297
+
diff --git a/tests/gold_tests/body_factory/http304_response.test.py b/tests/gold_tests/body_factory/http304_response.test.py
new file mode 100644
index 0000000..c9219a0
--- /dev/null
+++ b/tests/gold_tests/body_factory/http304_response.test.py
@@ -0,0 +1,56 @@
+'''
+Tests 304 responses
+'''
+#  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 = '''
+Tests 304 responses
+'''
+
+Test.SkipUnless(Condition.HasProgram("grep", "grep needs to be installed on system for this
test to work"))
+
+ts = Test.MakeATSProcess("ts")
+server = Test.MakeOriginServer("server")
+
+DEFAULT_304_HOST = 'www.default304.test'
+
+
+regex_remap_conf_file = "maps.reg"
+
+ts.Disk.remap_config.AddLine(
+    'map http://{0} http://127.0.0.1:{1} @plugin=regex_remap.so @pparam={2} @pparam=no-query-string
@pparam=host'
+                    .format(DEFAULT_304_HOST, server.Variables.Port, regex_remap_conf_file)
+    )
+ts.Disk.MakeConfigFile(regex_remap_conf_file).AddLine(
+    '//.*/ http://127.0.0.1:{0} @status=304'
+    .format(server.Variables.Port)
+)
+
+Test.Setup.Copy(os.path.join(os.pardir, os.pardir, 'tools', 'tcp_client.py'))
+Test.Setup.Copy('data')
+
+tr = Test.AddTestRun("Test domain {0}".format(DEFAULT_304_HOST))
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.StillRunningAfter = ts
+
+tr.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | grep -v '^Date:
'| grep -v '^Server: ATS/'".\
+    format(ts.Variables.port, 'data/{0}_get.txt'.format(DEFAULT_304_HOST))
+tr.Processes.Default.TimeOut = 5  # seconds
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "gold/http-304.gold"
diff --git a/tests/gold_tests/body_factory/http_head_no_origin.test.py b/tests/gold_tests/body_factory/http_head_no_origin.test.py
new file mode 100644
index 0000000..246e13f
--- /dev/null
+++ b/tests/gold_tests/body_factory/http_head_no_origin.test.py
@@ -0,0 +1,45 @@
+'''
+Tests that HEAD requests return proper responses when origin fails
+'''
+#  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 = '''
+Tests that HEAD requests return proper responses when origin fails
+'''
+
+Test.SkipUnless(Condition.HasProgram("grep", "grep needs to be installed on system for this
test to work"))
+
+ts = Test.MakeATSProcess("ts")
+server = Test.MakeOriginServer("server")
+
+HOST = 'www.example.test'
+
+
+Test.Setup.Copy(os.path.join(os.pardir, os.pardir, 'tools', 'tcp_client.py'))
+Test.Setup.Copy('data')
+
+tr = Test.AddTestRun("Test domain {0}".format(HOST))
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.StillRunningAfter = ts
+
+tr.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | grep -v '^Date:
'| grep -v '^Server: ATS/'".\
+    format(ts.Variables.port, 'data/{0}_head.txt'.format(HOST))
+tr.Processes.Default.TimeOut = 5  # seconds
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "gold/http-head-no-origin.gold"
diff --git a/tests/gold_tests/body_factory/http_with_origin.test.py b/tests/gold_tests/body_factory/http_with_origin.test.py
new file mode 100644
index 0000000..38fc558
--- /dev/null
+++ b/tests/gold_tests/body_factory/http_with_origin.test.py
@@ -0,0 +1,108 @@
+'''
+Tests that HEAD requests return proper responses
+'''
+#  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 = '''
+Tests that HEAD requests return proper responses
+'''
+
+Test.SkipUnless(Condition.HasProgram("grep", "grep needs to be installed on system for this
test to work"))
+
+ts = Test.MakeATSProcess("ts")
+
+HOST = 'www.example.test'
+
+server = Test.MakeOriginServer("server")
+
+ts.Disk.remap_config.AddLine(
+    'map http://{0} http://127.0.0.1:{1}'.format(HOST, server.Variables.Port)
+)
+
+server.addResponse("sessionfile.log", {
+    "headers": "HEAD /head200 HTTP/1.1\r\nHost: {0}\r\n\r\n".format(HOST),
+    "timestamp": "1469733493.993",
+    "body": ""
+}, {
+    "headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n",
+    "timestamp": "1469733493.993",
+    "body": "This body should not be returned for a HEAD request."
+})
+
+server.addResponse("sessionfile.log", {
+    "headers": "GET /get200 HTTP/1.1\r\nHost: {0}\r\n\r\n".format(HOST),
+    "timestamp": "1469733493.993",
+    "body": ""
+}, {
+    "headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n",
+    "timestamp": "1469733493.993",
+    "body": "This body should be returned for a GET request."
+})
+
+server.addResponse("sessionfile.log", {
+    "headers": "GET /get304 HTTP/1.1\r\nHost: {0}\r\n\r\n".format(HOST),
+    "timestamp": "1469733493.993",
+    "body": ""
+}, {
+    "headers": "HTTP/1.1 304 Not Modified\r\nConnection: close\r\n\r\n",
+    "timestamp": "1469733493.993",
+    "body": ""
+})
+
+
+Test.Setup.Copy(os.path.join(os.pardir, os.pardir, 'tools', 'tcp_client.py'))
+Test.Setup.Copy('data')
+
+trhead200 = Test.AddTestRun("Test domain {0}".format(HOST))
+trhead200.Processes.Default.StartBefore(Test.Processes.ts)
+trhead200.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port))
+trhead200.StillRunningAfter = ts
+trhead200.StillRunningAfter = server
+
+trhead200.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | grep -v '^Date:
'| grep -v '^Server: ATS/'".\
+    format(ts.Variables.port, 'data/{0}_head_200.txt'.format(HOST))
+trhead200.Processes.Default.TimeOut = 5  # seconds
+trhead200.Processes.Default.ReturnCode = 0
+trhead200.Processes.Default.Streams.stdout = "gold/http-head-200.gold"
+
+
+trget200 = Test.AddTestRun("Test domain {0}".format(HOST))
+trget200.StillRunningBefore = ts
+trget200.StillRunningBefore = server
+trget200.StillRunningAfter = ts
+trget200.StillRunningAfter = server
+
+trget200.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | grep -v '^Date:
'| grep -v '^Server: ATS/'".\
+    format(ts.Variables.port, 'data/{0}_get_200.txt'.format(HOST))
+trget200.Processes.Default.TimeOut = 5  # seconds
+trget200.Processes.Default.ReturnCode = 0
+trget200.Processes.Default.Streams.stdout = "gold/http-get-200.gold"
+
+
+trget304 = Test.AddTestRun("Test domain {0}".format(HOST))
+trget304.StillRunningBefore = ts
+trget304.StillRunningBefore = server
+trget304.StillRunningAfter = ts
+trget304.StillRunningAfter = server
+
+trget304.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | grep -v '^Date:
'| grep -v '^Server: ATS/'".\
+    format(ts.Variables.port, 'data/{0}_get_304.txt'.format(HOST))
+trget304.Processes.Default.TimeOut = 5  # seconds
+trget304.Processes.Default.ReturnCode = 0
+trget304.Processes.Default.Streams.stdout = "gold/http-get-304.gold"

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Mime
View raw message