[new http] Normalize line continuations in response headers.

BUG=1272571

Review URL: http://codereview.chromium.org/458

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@1818 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 347 insertions, 27 deletions

diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index d4ba447..1c9f518 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -25,6 +25,8 @@ class HttpResponseHeaders :
   // Parses the given raw_headers.  raw_headers should be formatted thus:
   // includes the http status response line, each line is \0-terminated, and
   // it's terminated by an empty line (ie, 2 \0s in a row).
+  // (Note that line continuations should have already been joined;
+  // see HttpUtil::AssembleRawHeaders)
   //
   // NOTE: For now, raw_headers is not really 'raw' in that this constructor is
   // called with a 'NativeMB' string on Windows because WinHTTP does not allow
@@ -204,13 +206,6 @@ class HttpResponseHeaders :
   void ParseStatusLine(std::string::const_iterator line_begin,
                        std::string::const_iterator line_end);
 
-  // Tries to extract the header line from a header block, given a single
-  // line of said header block.  If the header is malformed, we skip it.
-  // Example input:
-  //    Content-Length : text/html; charset=utf-8
-  void ParseHeaderLine(std::string::const_iterator line_begin,
-                       std::string::const_iterator line_end);
-
   // Find the header in our list (case-insensitive) starting with parsed_ at
   // index |from|.  Returns string::npos if not found.
   size_t FindHeader(size_t from, const std::string& name) const;
@@ -254,9 +249,10 @@ class HttpResponseHeaders :
 
   // The raw_headers_ consists of the normalized status line (terminated with a
   // null byte) and then followed by the raw null-terminated headers from the
-  // input that was passed to our constructor.  We preserve the input to
+  // input that was passed to our constructor.  We preserve the input [*] to
   // maintain as much ancillary fidelity as possible (since it is sometimes
   // hard to tell what may matter down-stream to a consumer of XMLHttpRequest).
+  // [*] The status line may be modified.
   std::string raw_headers_;
 
   // This is the parsed HTTP response code.
diff --git a/net/http/http_response_headers_unittest.cc b/net/http/http_response_headers_unittest.cc
index 8892682..e403a86 100644
--- a/net/http/http_response_headers_unittest.cc
+++ b/net/http/http_response_headers_unittest.cc
@@ -68,7 +68,7 @@ void TestCommon(const TestData& test) {
 TEST(HttpResponseHeadersTest, NormalizeHeadersWhitespace) {
   TestData test = {
     "HTTP/1.1    202   Accepted  \n"
-    "  Content-TYPE  : text/html; charset=utf-8  \n"
+    "Content-TYPE  : text/html; charset=utf-8  \n"
     "Set-Cookie: a \n"
     "Set-Cookie:   b \n",
 
@@ -81,6 +81,24 @@ TEST(HttpResponseHeadersTest, NormalizeHeadersWhitespace) {
   TestCommon(test);
 }
 
+// Check that we normalize headers properly (header name is invalid if starts
+// with LWS).
+TEST(HttpResponseHeadersTest, NormalizeHeadersLeadingWhitespace) {
+  TestData test = {
+    "HTTP/1.1    202   Accepted  \n"
+    // Starts with space -- will be skipped as invalid.
+    "  Content-TYPE  : text/html; charset=utf-8  \n"
+    "Set-Cookie: a \n"
+    "Set-Cookie:   b \n",
+
+    "HTTP/1.1 202 Accepted\n"
+    "Set-Cookie: a, b\n",
+
+    202
+  };
+  TestCommon(test);
+}
+
 TEST(HttpResponseHeadersTest, BlankHeaders) {
   TestData test = {
     "HTTP/1.1 200 OK\n"
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index ef311dd..7d2bf4a 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -10,6 +10,7 @@
 #include <algorithm>
 
 #include "base/logging.h"
+#include "base/string_piece.h"
 #include "base/string_util.h"
 
 using std::string;
@@ -218,14 +219,18 @@ bool HttpUtil::IsNonCoalescingHeader(string::const_iterator name_begin,
   return false;
 }
 
+bool HttpUtil::IsLWS(char c) {
+  return strchr(HTTP_LWS, c) != NULL;
+}
+
 void HttpUtil::TrimLWS(string::const_iterator* begin,
                        string::const_iterator* end) {
   // leading whitespace
-  while (*begin < *end && strchr(HTTP_LWS, (*begin)[0]))
+  while (*begin < *end && IsLWS((*begin)[0]))
     ++(*begin);
 
   // trailing whitespace
-  while (*begin < *end && strchr(HTTP_LWS, (*end)[-1]))
+  while (*begin < *end && IsLWS((*end)[-1]))
     --(*end);
 }
 
@@ -246,27 +251,79 @@ int HttpUtil::LocateEndOfHeaders(const char* buf, int buf_len) {
   return -1;
 }
 
-std::string HttpUtil::AssembleRawHeaders(const char* buf, int buf_len) {
+// In order for a line to be continuable, it must specify a
+// non-blank header-name. Line continuations are specifically for
+// header values -- do not allow headers names to span lines.
+static bool IsLineSegmentContinuable(const char* begin, const char* end) {
+  if (begin == end)
+    return false;
+
+  const char* colon = std::find(begin, end, ':');
+  if (colon == end)
+    return false;
+
+  const char* name_begin = begin;
+  const char* name_end = colon;
+
+  // Name can't be empty.
+  if (name_begin == name_end)
+    return false;
+
+  // Can't start with LWS (this would imply the segment is a continuation)
+  if (HttpUtil::IsLWS(*name_begin))
+    return false;
+
+  return true;
+}
+
+// Helper used by AssembleRawHeaders, to find the end of the status line.
+static const char* FindStatusLineEnd(const char* begin, const char* end) {
+  size_t i = StringPiece(begin, end - begin).find_first_of("\r\n");
+  if (i == StringPiece::npos)
+    return end;
+  return begin + i;
+}
+
+std::string HttpUtil::AssembleRawHeaders(const char* input_begin,
+                                         int input_len) {
   std::string raw_headers;
+  raw_headers.reserve(input_len);
 
-  // TODO(darin):
-  //   - Handle header line continuations.
-  //   - Be careful about CRs that appear spuriously mid header line.
+  const char* input_end = input_begin + input_len;
 
-  int line_start = 0;
-  for (int i = 0; i < buf_len; ++i) {
-    char c = buf[i];
-    if (c == '\r' || c == '\n') {
-      if (line_start != i) {
-        // (line_start,i) is a header line.
-        raw_headers.append(buf + line_start, buf + i);
-        raw_headers.push_back('\0');
-      }
-      line_start = i + 1;
-    }
+  // Copy the status line.
+  const char* status_line_end = FindStatusLineEnd(input_begin, input_end);
+  raw_headers.append(input_begin, status_line_end);
+
+  // After the status line, every subsequent line is a header line segment.
+  // Should a segment start with LWS, it is a continuation of the previous
+  // line's field-value.
+
+  // TODO(ericroman): is this too permissive? (delimits on [\r\n]+)
+  CStringTokenizer lines(status_line_end, input_end, "\r\n");
+
+  // This variable is true when the previous line was continuable.
+  bool can_append_continuation = false;
+
+  while (lines.GetNext()) {
+    const char* line_begin = lines.token_begin();
+    const char* line_end = lines.token_end();
+     
+    bool is_continuation = can_append_continuation && IsLWS(*line_begin);
+
+    // Terminate the previous line.
+    if (!is_continuation)
+      raw_headers.push_back('\0');
+
+    // Copy the raw data to output.
+    raw_headers.append(line_begin, line_end);
+
+    // Check if the current line can be continued.
+    if (!is_continuation)
+      can_append_continuation = IsLineSegmentContinuable(line_begin, line_end);
   }
-  raw_headers.push_back('\0');
 
+  raw_headers.append("\0\0", 2);
   return raw_headers;
 }
 
@@ -296,6 +353,13 @@ bool HttpUtil::HeadersIterator::GetNext() {
       continue;  // skip malformed header
 
     name_end_ = colon;
+
+    // If the name starts with LWS, it is an invalid line.
+    // Leading LWS implies a line continuation, and these should have
+    // already been joined by AssembleRawHeaders().
+    if (name_begin_ == name_end_ || IsLWS(*name_begin_))
+      continue;
+
     TrimLWS(&name_begin_, &name_end_);
     if (name_begin_ == name_end_)
       continue;  // skip malformed header
diff --git a/net/http/http_util.h b/net/http/http_util.h
index 0c31195..5bfb71a 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -46,6 +46,11 @@ class HttpUtil {
     return IsNonCoalescingHeader(name.begin(), name.end());
   }
 
+  // Return true if the character is HTTP "linear white space" (SP | HT).
+  // This definition corresponds with the HTTP_LWS macro, and does not match
+  // newlines.
+  static bool IsLWS(char c);
+
   // Trim HTTP_LWS chars from the beginning and end of the string.
   static void TrimLWS(std::string::const_iterator* begin,
                       std::string::const_iterator* end);
@@ -66,6 +71,8 @@ class HttpUtil {
 
   // Used to iterate over the name/value pairs of HTTP headers.  To iterate
   // over the values in a multi-value header, use ValuesIterator.
+  // See AssembleRawHeaders for joining line continuations (this iterator
+  // does not expect any).
   class HeadersIterator {
    public:
     HeadersIterator(std::string::const_iterator headers_begin,
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index 84777f9..aaff8f5 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -124,6 +124,241 @@ TEST(HttpUtilTest, AssembleRawHeaders) {
 
     { "HTTP/1.0 200 OK\nFoo: 1\nBar: 2\n\n",
       "HTTP/1.0 200 OK|Foo: 1|Bar: 2||" },
+
+    // Valid line continuation (single SP).
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      " continuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1 continuation|"
+      "Bar: 2||"
+    },
+
+    // Valid line continuation (single HT).
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "\tcontinuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1\tcontinuation|"
+      "Bar: 2||"
+    },
+
+    // Valid line continuation (multiple SP).
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "   continuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1   continuation|"
+      "Bar: 2||"
+    },
+
+    // Valid line continuation (multiple HT).
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "\t\t\tcontinuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1\t\t\tcontinuation|"
+      "Bar: 2||"
+    },
+
+    // Valid line continuation (mixed HT, SP).
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      " \t \t continuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1 \t \t continuation|"
+      "Bar: 2||"
+    },
+
+    // Valid multi-line continuation
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      " continuation1\n"
+      "\tcontinuation2\n"
+      "  continuation3\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1 continuation1\tcontinuation2  continuation3|"
+      "Bar: 2||"
+    },
+
+    // Valid line continuation (No value bytes in first line).
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo:\n"
+      " value\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: value|"
+      "Bar: 2||"
+    },
+
+    // Not a line continuation (can't continue status line).
+    {
+      "HTTP/1.0 200 OK\n"
+      " Foo: 1\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      " Foo: 1|"
+      "Bar: 2||"
+    },
+
+    // Not a line continuation (can't continue status line).
+    {
+      "HTTP/1.0\n"
+      " 200 OK\n"
+      "Foo: 1\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0|"
+      " 200 OK|"
+      "Foo: 1|"
+      "Bar: 2||"
+    },
+
+    // Not a line continuation (can't continue status line).
+    {
+      "HTTP/1.0 404\n"
+      " Not Found\n"
+      "Foo: 1\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 404|"
+      " Not Found|"
+      "Foo: 1|"
+      "Bar: 2||"
+    },
+
+    // Unterminated status line.
+    {
+      "HTTP/1.0 200 OK",
+      
+      "HTTP/1.0 200 OK||"
+    },
+
+    // Single terminated, with headers
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "Bar: 2\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1|"
+      "Bar: 2||"
+    },
+
+    // Not terminated, with headers
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "Bar: 2",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1|"
+      "Bar: 2||"
+    },
+
+    // Not a line continuation (VT)
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "\vInvalidContinuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1|"
+      "\vInvalidContinuation|"
+      "Bar: 2||"
+    },
+
+    // Not a line continuation (formfeed)
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "\fInvalidContinuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1|"
+      "\fInvalidContinuation|"
+      "Bar: 2||"
+    },
+
+    // Not a line continuation -- can't continue header names.
+    {
+      "HTTP/1.0 200 OK\n"
+      "Serv\n"
+      " er: Apache\n"
+      "\tInvalidContinuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Serv|"
+      " er: Apache|"
+      "\tInvalidContinuation|"
+      "Bar: 2||"
+    },
+
+    // Not a line continuation -- no value to continue.
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "garbage\n"
+      "  not-a-continuation\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "Foo: 1|"
+      "garbage|"
+      "  not-a-continuation|"
+      "Bar: 2||",
+    },
+
+    // Not a line continuation -- no valid name.
+    {
+      "HTTP/1.0 200 OK\n"
+      ": 1\n"
+      "  garbage\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      ": 1|"
+      "  garbage|"
+      "Bar: 2||",
+    },
+
+    // Not a line continuation -- no valid name (whitespace)
+    {
+      "HTTP/1.0 200 OK\n"
+      "   : 1\n"
+      "  garbage\n"
+      "Bar: 2\n\n",
+
+      "HTTP/1.0 200 OK|"
+      "   : 1|"
+      "  garbage|"
+      "Bar: 2||",
+    },
+
   };
   for (size_t i = 0; i < arraysize(tests); ++i) {
     int input_len = static_cast<int>(strlen(tests[i].input));