Avoid adding invalid headers in AddHeaderFromString

net/http/http_util.cc

Index: net/http/http_util.cc
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index 313da11a66f118b55043627976c6e7164f527c8b..69d72b5aad849509287541ce521740d52faf2654 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -340,15 +340,19 @@ bool HttpUtil::IsSafeHeader(const std::string& name) {
 }
 
 // static
-bool HttpUtil::IsValidHeaderName(const std::string& name) {
+bool HttpUtil::IsValidHeaderName(const base::StringPiece& name) {
   // Check whether the header name is RFC 2616-compliant.
   return HttpUtil::IsToken(name);
 }
 
 // static
-bool HttpUtil::IsValidHeaderValue(const std::string& value) {
+bool HttpUtil::IsValidHeaderValue(const base::StringPiece& value) {
   // Just a sanity check: disallow NUL, CR and LF.
-  return value.find_first_of("\0\r\n", 0, 3) == std::string::npos;
+  for (char c : value) {
+    if (c == '\0' || c == '\r' || c == '\n')
+      return false;
+  }
+  return true;
 }
 
 // static
@@ -409,16 +413,32 @@ bool HttpUtil::IsLWS(char c) {
   return strchr(HTTP_LWS, c) != NULL;
 }
 
-void HttpUtil::TrimLWS(std::string::const_iterator* begin,
-                       std::string::const_iterator* end) {
+namespace {
+template <typename ConstIterator>
+void TrimLWSImplementation(ConstIterator* begin, ConstIterator* end) {
   // leading whitespace
-  while (*begin < *end && IsLWS((*begin)[0]))
+  while (*begin < *end && HttpUtil::IsLWS((*begin)[0]))
     ++(*begin);
 
   // trailing whitespace
-  while (*begin < *end && IsLWS((*end)[-1]))
+  while (*begin < *end && HttpUtil::IsLWS((*end)[-1]))
     --(*end);
 }
+}  // namespace

[mmenke, 2016/08/09 16:54:47]

The more common pattern in net/ is to use a single anonymous namespace at the
top of the file.

[Adam Rice, 2016/08/10 03:19:07]

Done. I can move the rest of the anonymous namespaces in this file in a separate
CL if you'd like.

+
+// static
+void HttpUtil::TrimLWS(std::string::const_iterator* begin,
+                       std::string::const_iterator* end) {
+  TrimLWSImplementation(begin, end);
+}
+
+// static
+base::StringPiece HttpUtil::TrimLWS(const base::StringPiece& string) {
+  const char* begin = string.data();
+  const char* end = string.data() + string.size();
+  TrimLWSImplementation(&begin, &end);
+  return base::StringPiece(begin, end - begin);
+}
 
 bool HttpUtil::IsQuote(char c) {
   // Single quote mark isn't actually part of quoted-text production,
@@ -437,12 +457,11 @@ bool IsTokenChar(unsigned char c) {
 }  // anonymous namespace
 
 // See RFC 2616 Sec 2.2 for the definition of |token|.
-bool HttpUtil::IsToken(std::string::const_iterator begin,
-                       std::string::const_iterator end) {
-  if (begin == end)
+bool HttpUtil::IsToken(const base::StringPiece& string) {
+  if (string.size() == 0)

[mmenke, 2016/08/09 16:54:47]

!string.empty() is preferred, as it doesn't require computing the length of the
string.

[Adam Rice, 2016/08/10 03:19:07]

I can't believe I did that. Fixed.

     return false;
-  for (std::string::const_iterator iter = begin; iter != end; ++iter) {
-    if (!IsTokenChar(*iter))
+  for (char c : string) {
+    if (!IsTokenChar(c))
       return false;
   }
   return true;