[trafficserver] 01/01: Respond with 400 code when Content-Length headers mismatch, remove duplicate copies of the Content-Length header with exactly same values, and remove Content-Length headers if Transfer-Encoding header exists.

[bc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch content_length
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit c54b47b4ec5dfad855a5cb342d84796e35ec6be6
Author: Bryan Call <bc...@apache.org>
AuthorDate: Mon Mar 5 16:21:18 2018 -0800

    Respond with 400 code when Content-Length headers mismatch, remove
    duplicate copies of the Content-Length header with exactly same values,
    and remove Content-Length headers if Transfer-Encoding header exists.
---
 proxy/hdrs/HTTP.cc                      | 60 +++++++++++++++++++++++++++++++--
 proxy/hdrs/HTTP.h                       |  1 +
 tests/gold_tests/headers/syntax.test.py | 31 ++++++++++++++---
 3 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/proxy/hdrs/HTTP.cc b/proxy/hdrs/HTTP.cc
index 77d0a6e..32b5aba 100644
--- a/proxy/hdrs/HTTP.cc
+++ b/proxy/hdrs/HTTP.cc
@@ -960,8 +960,12 @@ http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const
       }
 
       ParseResult ret = mime_parser_parse(&parser->m_mime_parser, heap, hh->m_fields_impl, start, end, must_copy_strings, eof);
+      // If we're done with the main parse do some validation
       if (ret == PARSE_RESULT_DONE) {
-        ret = validate_hdr_host(hh); // if we're done with the main parse, check HOST.
+        ret = validate_hdr_host(hh); // check HOST header
+      }
+      if (ret == PARSE_RESULT_DONE) {
+        ret = validate_hdr_content_length(heap, hh);
       }
       return ret;
     }
@@ -1111,8 +1115,12 @@ http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const
     parser->m_parsing_http = false;
 
     ParseResult ret = mime_parser_parse(&parser->m_mime_parser, heap, hh->m_fields_impl, start, end, must_copy_strings, eof);
+    // If we're done with the main parse do some validation
     if (ret == PARSE_RESULT_DONE) {
-      ret = validate_hdr_host(hh); // if we're done with the main parse, check HOST.
+      ret = validate_hdr_host(hh); // check HOST header
+    }
+    if (ret == PARSE_RESULT_DONE) {
+      ret = validate_hdr_content_length(heap, hh);
     }
     return ret;
   }
@@ -1156,6 +1164,54 @@ validate_hdr_host(HTTPHdrImpl *hh)
   return ret;
 }
 
+ParseResult
+validate_hdr_content_length(HdrHeap *heap, HTTPHdrImpl *hh)
+{
+  MIMEField *content_length_field = mime_hdr_field_find(hh->m_fields_impl, MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH);
+
+  if (content_length_field) {
+    // RFC 7230 section 3.3.3:
+    // If a message is received with both a Transfer-Encoding and a
+    // Content-Length header field, the Transfer-Encoding overrides
+    // the Content-Length
+    if (mime_hdr_field_find(hh->m_fields_impl, MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING) != nullptr) {
+      // Delete all Content-Length headers
+      Debug("http", "Transfer-Encoding header and Content-Length headers the request, removing all Content-Length headers");
+      mime_hdr_field_delete(heap, hh->m_fields_impl, content_length_field);
+      return PARSE_RESULT_DONE;
+    }
+
+    // RFC 7230 section 3.3.3:
+    // If a message is received without Transfer-Encoding and with
+    // either multiple Content-Length header fields having differing
+    // field-values or a single Content-Length header field having an
+    // invalid value, then the message framing is invalid and the
+    // recipient MUST treat it as an unrecoverable error.  If this is a
+    // request message, the server MUST respond with a 400 (Bad Request)
+    // status code and then close the connection
+    int content_length_len         = 0;
+    const char *content_length_val = content_length_field->value_get(&content_length_len);
+
+    while (content_length_field->has_dups()) {
+      int content_length_len_2         = 0;
+      const char *content_length_val_2 = content_length_field->m_next_dup->value_get(&content_length_len_2);
+
+      if ((content_length_len != content_length_len_2) ||
+          (memcmp(content_length_val, content_length_val_2, content_length_len) != 0)) {
+        // Values are different, parse error
+        Debug("http", "Content-Length headers don't match, returning parse error");
+        return PARSE_RESULT_ERROR;
+      } else {
+        // Delete the duplicate since it has the same value
+        Debug("http", "Deleting duplicate Content-Length header");
+        mime_hdr_field_delete(heap, hh->m_fields_impl, content_length_field->m_next_dup, false);
+      }
+    }
+  }
+
+  return PARSE_RESULT_DONE;
+}
+
 /*-------------------------------------------------------------------------
   -------------------------------------------------------------------------*/
 
diff --git a/proxy/hdrs/HTTP.h b/proxy/hdrs/HTTP.h
index eb99817..0f59213 100644
--- a/proxy/hdrs/HTTP.h
+++ b/proxy/hdrs/HTTP.h
@@ -442,6 +442,7 @@ void http_parser_clear(HTTPParser *parser);
 ParseResult http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const char **start, const char *end,
                                   bool must_copy_strings, bool eof, bool strict_uri_parsing);
 ParseResult validate_hdr_host(HTTPHdrImpl *hh);
+ParseResult validate_hdr_content_length(HdrHeap *heap, HTTPHdrImpl *hh);
 ParseResult http_parser_parse_resp(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const char **start, const char *end,
                                    bool must_copy_strings, bool eof);
 
diff --git a/tests/gold_tests/headers/syntax.test.py b/tests/gold_tests/headers/syntax.test.py
index e38b97e..2938304 100644
--- a/tests/gold_tests/headers/syntax.test.py
+++ b/tests/gold_tests/headers/syntax.test.py
@@ -64,21 +64,42 @@ tr = Test.AddTestRun()
 tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(Test.Processes.ts)
 tr.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.ssl_port))
-tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H " foo: bar"  -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H " foo: bar" -H "Host: www.example.com" http://localhost:8080/'
 tr.Processes.Default.ReturnCode = 0
 tr.Processes.Default.Streams.stdout = "syntax.200.gold"
 tr.StillRunningAfter = ts
 
-# Test 2 - 400 Response
+# Test 2 - 400 Response - Single space after field name
 tr = Test.AddTestRun()
-tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo : bar"  -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo : bar" -H "Host: www.example.com" http://localhost:8080/'
 tr.Processes.Default.ReturnCode = 0
 tr.Processes.Default.Streams.stdout = "syntax.400.gold"
 tr.StillRunningAfter = ts
 
-# Test 3 - 400 Response
+# Test 3 - 400 Response - Double space after field name
 tr = Test.AddTestRun()
-tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo  : bar"  -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo  : bar" -H "Host: www.example.com" http://localhost:8080/'
 tr.Processes.Default.ReturnCode = 0
 tr.Processes.Default.Streams.stdout = "syntax.400.gold"
 tr.StillRunningAfter = ts
+
+# Test 4 - 400 Response - Three different Content-Length headers
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -d "hello world" -H "Content-Length: 11" -H "Content-Length: 10" -H "Content-Length: 9" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "syntax.400.gold"
+tr.StillRunningAfter = ts
+
+# Test 4 - 200 Response - Three same Content-Length headers
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -d "hello world" -H "Content-Length: 11" -H "Content-Length: 11" -H "Content-Length: 11" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "syntax.200.gold"
+tr.StillRunningAfter = ts
+
+# Test 4 - 200 Response - Three different Content-Length headers with a Transfer ecoding header
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -d "hello world" -H "Transfer-Encoding: chunked" -H "Content-Length: 11" -H "Content-Length: 10" -H "Content-Length: 9" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "syntax.200.gold"
+tr.StillRunningAfter = ts

-- 
To stop receiving notification emails like this one, please contact
bcall@apache.org.