Reject line terminators in HttpUtil::IsValidHeaderValue()

net/http/http_util_unittest.cc

Index: net/http/http_util_unittest.cc
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index 7162d7a0a3c82b1c968b0f1adc8fbb25260358cb..ef6d519b934b76b5cc41a0b3f4280f5dd755d22f 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -1367,4 +1367,27 @@ TEST(HttpUtilTest, HasValidators) {
   EXPECT_TRUE(HttpUtil::HasValidators(v1_1, kEtagEmpty, kLastModifiedInvalid));
 }
 
+TEST(HttpUtilTest, IsValidHeaderValue) {
+  static const char* const invalid_values[] = {

[mmenke, 2016/07/12 18:24:49]

nit:  Don't use static (I think the consensus is it's a bad idea?)

[Adam Rice, 2016/07/13 01:47:00]

I wasn't aware of that.

I'm never quite sure when the compiler can avoid the copy, so I have tended to
use static when in doubt (but for POD types only).

[mmenke, 2016/07/13 01:52:23]

I think the issue is that function-scoped statics tend to be initialized on
first call, which has threading implications.  Anyhow, I'm no expert on it.

+      "X-Requested-With: chrome${NUL}Sec-Unsafe: injected",
+      "X-Requested-With: chrome\r\nSec-Unsafe: injected",
+      "X-Requested-With: chrome\nSec-Unsafe: injected",
+      "X-Requested-With: chrome\rSec-Unsafe: injected",
+  };
+  for (std::string value : invalid_values) {

[mmenke, 2016/07/12 18:24:49]

nit:  const std::string&

[Adam Rice, 2016/07/13 01:47:00]

Done.

+    base::ReplaceSubstringsAfterOffset(&value, 0, "${NUL}",
+                                       std::string(1, '\0'));
+    EXPECT_FALSE(HttpUtil::IsValidHeaderValue(value)) << value;
+  }

[mmenke, 2016/07/12 18:24:49]

nit:  Suggest a blank line here

[Adam Rice, 2016/07/13 01:47:00]

Done.

+  // Check that all characters permitted by RFC7230 3.2.6 are allowed.
+  std::string allowed = "\t";
+  for (char c = '\x20'; c < '\x7F'; ++c) {
+    allowed.append(1, c);
+  }
+  for (int c = 0x80; c <= 0xFF; ++c) {
+    allowed.append(1, static_cast<char>(c));
+  }
+  EXPECT_TRUE(HttpUtil::IsValidHeaderValue(allowed));
+}
+
 }  // namespace net