Avoid adding invalid headers in AddHeaderFromString

net/http/http_util.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.
 
 // The rules for parsing content-types were borrowed from Firefox:
 // http://lxr.mozilla.org/mozilla/source/netwerk/base/src/nsURLHelper.cpp#834
 
 #include "net/http/http_util.h"
 
 #include <algorithm>
@@ skipping 322 matching lines @@
       base::StartsWith(lower_name, "sec-", base::CompareCase::SENSITIVE))
     return false;
   for (size_t i = 0; i < arraysize(kForbiddenHeaderFields); ++i) {
     if (lower_name == kForbiddenHeaderFields[i])
       return false;
   }
   return true;
 }
 
 // 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
 std::string HttpUtil::StripHeaders(const std::string& headers,
                                    const char* const headers_to_remove[],
                                    size_t headers_to_remove_len) {
   std::string stripped_headers;
   HttpUtil::HeadersIterator it(headers.begin(), headers.end(), "\r\n");
 
   while (it.GetNext()) {
@@ skipping 40 matching lines @@
                                    kNonCoalescingHeaders[i]))
       return true;
   }
   return false;
 }
 
 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,
   // but apparently some servers rely on this.
   return c == '"' || c == '\'';
 }
 
 namespace {
 bool IsTokenChar(unsigned char c) {
   return !(c >= 0x80 || c <= 0x1F || c == 0x7F || c == '(' || c == ')' ||
            c == '<' || c == '>' || c == '@' || c == ',' || c == ';' ||
            c == ':' || c == '\\' || c == '"' || c == '/' || c == '[' ||
            c == ']' || c == '?' || c == '=' || c == '{' || c == '}' ||
            c == ' ' || c == '\t');
 }
 }  // 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;
 }
 
 // See RFC 5987 Sec 3.2.1 for the definition of |parmname|.
 bool HttpUtil::IsParmName(std::string::const_iterator begin,
                           std::string::const_iterator end) {
   if (begin == end)
     return false;
@@ skipping 597 matching lines @@
   return true;
 }
 
 bool HttpUtil::NameValuePairsIterator::IsQuote(char c) const {
   if (strict_quotes_)
     return c == '"';
   return HttpUtil::IsQuote(c);
 }
 
 }  // namespace net