Don't consider '\0' as whitespace when parsing HTTP headers.

net/http/http_util_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 
 #include "base/strings/string_util.h"
 #include "net/http/http_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ skipping 360 matching lines @@
       {"foo\nbar\n\r\njunk", 10},
       {"foo\nbar\r\n\njunk", 10},
   };
   for (size_t i = 0; i < arraysize(tests); ++i) {
     int input_len = static_cast<int>(strlen(tests[i].input));
     int eoh = HttpUtil::LocateEndOfAdditionalHeaders(tests[i].input, input_len);
     EXPECT_EQ(tests[i].expected_result, eoh);
   }
 }
 TEST(HttpUtilTest, AssembleRawHeaders) {
+  // clang-format off
   struct {
     const char* const input;  // with '|' representing '\0'
     const char* const expected_result;  // with '\0' changed to '|'
   } tests[] = {
     { "HTTP/1.0 200 OK\r\nFoo: 1\r\nBar: 2\r\n\r\n",
       "HTTP/1.0 200 OK|Foo: 1|Bar: 2||" },
 
     { "HTTP/1.0 200 OK\nFoo: 1\nBar: 2\n\n",
       "HTTP/1.0 200 OK|Foo: 1|Bar: 2||" },
 
@@ skipping 281 matching lines @@
       "HTTP/1.0 200 OK|Bar2:0|Baz2:1\r\nFoo: 1\r\nBar: 2\r\n\r\n",
       "HTTP/1.0 200 OKBar2:0Baz2:1|Foo: 1|Bar: 2||"
     },
 
     // Embed NULLs in a header line. They should not be understood as
     // line separators.
     {
       "HTTP/1.0 200 OK\nFoo: 1|Foo2: 3\nBar: 2\n\n",
       "HTTP/1.0 200 OK|Foo: 1Foo2: 3|Bar: 2||"
     },
+
+    // An embedded NUL should not be interpreted as LWS (leading to line
+    // continuations)
+    {
+      "HTTP/1.0 200 OK\n"
+      "Foo: 1\n"
+      "|EmbeddedNull: 3\n"
+      "Bar: 2\n\n",
+      "HTTP/1.0 200 OK|Foo: 1|EmbeddedNull: 3|Bar: 2||"

[mmenke, 2016/12/06 19:08:24]

EmbeddedNull?  Where's the magic for that.

[eroman, 2016/12/06 19:19:34]

There is nothing special about "EmbeddedNull", I just chose that name to draw
attention to what is going on in that line.

The special part is the vertical pipe '|'. This is used as shorthand in the test
for NUL [1]

[1]
https://cs.chromium.org/chromium/src/net/http/http_util_unittest.cc?q=http_ut...

[eroman, 2016/12/06 19:25:33]

Want me to rename this to something more generic like "Blah:" ?

[eroman, 2016/12/06 19:29:00]

Done.

+    },
   };
+  // clang-format on
   for (size_t i = 0; i < arraysize(tests); ++i) {
     std::string input = tests[i].input;
     std::replace(input.begin(), input.end(), '|', '\0');
     std::string raw = HttpUtil::AssembleRawHeaders(input.data(), input.size());
     std::replace(raw.begin(), raw.end(), '\0', '|');
     EXPECT_EQ(tests[i].expected_result, raw);
   }
 }
 
 // Test SpecForRequest().
@@ skipping 708 matching lines @@
   EXPECT_FALSE(HttpUtil::IsToken(base::StringPiece()));
   EXPECT_FALSE(HttpUtil::IsToken("hello, world"));
   EXPECT_FALSE(HttpUtil::IsToken(" "));
   EXPECT_FALSE(HttpUtil::IsToken(base::StringPiece("\0", 1)));
   EXPECT_FALSE(HttpUtil::IsToken("\x01"));
   EXPECT_FALSE(HttpUtil::IsToken("\x7F"));
   EXPECT_FALSE(HttpUtil::IsToken("\x80"));
   EXPECT_FALSE(HttpUtil::IsToken("\xff"));
 }
 
+TEST(HttpUtilTest, IsLWS) {
+  EXPECT_FALSE(HttpUtil::IsLWS('\v'));
+  EXPECT_FALSE(HttpUtil::IsLWS('\0'));
+  EXPECT_FALSE(HttpUtil::IsLWS('1'));
+  EXPECT_FALSE(HttpUtil::IsLWS('a'));
+  EXPECT_FALSE(HttpUtil::IsLWS('.'));
+  EXPECT_FALSE(HttpUtil::IsLWS('\n'));
+  EXPECT_FALSE(HttpUtil::IsLWS('\r'));
+
+  EXPECT_TRUE(HttpUtil::IsLWS('\t'));
+  EXPECT_TRUE(HttpUtil::IsLWS(' '));

[mmenke, 2016/12/06 19:08:24]

Should we have a whitelist here?  i.e.
for (int c = 0; c < 0x100; c++) {
  EXPECT_EQ(c == '\t' || c =='\t', IsLWS(static_cast<char>(c)));
}

(Or go from -0x80 to < 0x80, if you want to avoid the overflow)

[eroman, 2016/12/06 19:19:34]

I think that is a bit overkill, but can switch to explicit enumeration if you
would like.

My thinking given the implementation (and bug fix) was to spot check a couple
character classes

[mmenke, 2016/12/06 19:25:29]

ok

+}
+
 }  // namespace net