Reject line terminators in HttpUtil::IsValidHeaderValue()

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 1349 matching lines @@
   EXPECT_TRUE(HttpUtil::HasValidators(v1_1, kEtagStrong, kLastModified));
   EXPECT_TRUE(HttpUtil::HasValidators(v1_1, kEtagWeak, kLastModified));
   EXPECT_TRUE(HttpUtil::HasValidators(v1_1, kEtagEmpty, kLastModified));
 
   EXPECT_FALSE(HttpUtil::HasValidators(v1_1, kMissing, kLastModifiedInvalid));
   EXPECT_TRUE(HttpUtil::HasValidators(v1_1, kEtagStrong, kLastModifiedInvalid));
   EXPECT_TRUE(HttpUtil::HasValidators(v1_1, kEtagWeak, kLastModifiedInvalid));
   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