Avoid adding invalid headers in AddHeaderFromString

net/http/http_util.h

 // 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.
 
 #ifndef NET_HTTP_HTTP_UTIL_H_
 #define NET_HTTP_HTTP_UTIL_H_
 
 #include <stddef.h>
 
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/strings/string_piece.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/time/time.h"
 #include "net/base/net_export.h"
 #include "net/http/http_byte_range.h"
 #include "net/http/http_version.h"
 #include "url/gurl.h"
 
 // This is a macro to support extending this string literal at compile time.
 // Please excuse me polluting your global namespace!
 #define HTTP_LWS " \t"
@@ skipping 45 matching lines @@
   // Scans the '\r\n'-delimited headers for the given header name.  Returns
   // true if a match is found.  Input is assumed to be well-formed.
   // TODO(darin): kill this
   static bool HasHeader(const std::string& headers, const char* name);
 
   // Returns true if it is safe to allow users and scripts to specify the header
   // named |name|.
   static bool IsSafeHeader(const std::string& name);
 
   // Returns true if |name| is a valid HTTP header name.
-  static bool IsValidHeaderName(const std::string& name);
+  static bool IsValidHeaderName(const base::StringPiece& name);
 
   // Returns false if |value| contains NUL or CRLF. This method does not perform
   // a fully RFC-2616-compliant header value validation.
-  static bool IsValidHeaderValue(const std::string& value);
+  static bool IsValidHeaderValue(const base::StringPiece& value);
 
   // Strips all header lines from |headers| whose name matches
   // |headers_to_remove|. |headers_to_remove| is a list of null-terminated
   // lower-case header names, with array length |headers_to_remove_len|.
   // Returns the stripped header lines list, separated by "\r\n".
   static std::string StripHeaders(const std::string& headers,
                                   const char* const headers_to_remove[],
                                   size_t headers_to_remove_len);
 
   // Multiple occurances of some headers cannot be coalesced into a comma-
   // separated list since their values are (or contain) unquoted HTTP-date
   // values, which may contain a comma (see RFC 2616 section 3.3.1).
   static bool IsNonCoalescingHeader(std::string::const_iterator name_begin,
                                     std::string::const_iterator name_end);
   static bool IsNonCoalescingHeader(const std::string& name) {
     return IsNonCoalescingHeader(name.begin(), name.end());
   }
 
   // Return true if the character is HTTP "linear white space" (SP | HT).
   // This definition corresponds with the HTTP_LWS macro, and does not match
   // newlines.
   static bool IsLWS(char c);
 
   // Trim HTTP_LWS chars from the beginning and end of the string.
   static void TrimLWS(std::string::const_iterator* begin,
                       std::string::const_iterator* end);
+  static base::StringPiece TrimLWS(const base::StringPiece& string);
 
   // Whether the character is the start of a quotation mark.
   static bool IsQuote(char c);
 
   // Whether the string is a valid |token| as defined in RFC 2616 Sec 2.2.
   static bool IsToken(std::string::const_iterator begin,
-                      std::string::const_iterator end);
-  static bool IsToken(const std::string& str) {
-    return IsToken(str.begin(), str.end());
+                      std::string::const_iterator end) {
+    return IsToken(base::StringPiece(&*begin, end - begin));

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

Is this guaranteed to work?

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

Interesting question. I had to go look at the C++ standard. For the "is the type
const char *" question, the most conclusive reference I can find is in 21.4.1:

The char-like objects in a basic_string object shall be stored contiguously.
That is, for any basic_string
object s, the identity &*(s.begin() + n) == &*s.begin() + n shall hold for all
values of n such that 0
<= n < s.size().

This at least strongly implies that the type of *s.begin() is char and not some
proxy type.

I think if |begin| is off-the-end then *begin is undefined behaviour, so I have
added a check for that.

[mmenke, 2016/08/10 03:30:55]

And std::string::const_iterators are required to return const char&'s?  This
just seems very weird and obscure.

Looks like this method is only called in 3 places.  How hard would it be to just
get rid of this override?

[Adam Rice, 2016/08/10 05:22:58]

To be fair, it is widely used: https://cs.chromium.org/search/?q=%22%26*%22
Take a look at Patch Set 4. I'm not sure it's better.

   }
+  static bool IsToken(const base::StringPiece& str);
 
   // Whether the string is a valid |parmname| as defined in RFC 5987 Sec 3.2.1.
   static bool IsParmName(std::string::const_iterator begin,
                          std::string::const_iterator end);
   static bool IsParmName(const std::string& str) {
     return IsParmName(str.begin(), str.end());
   }
 
   // RFC 2616 Sec 2.2:
   // quoted-string = ( <"> *(qdtext | quoted-pair ) <"> )
@@ skipping 309 matching lines @@
     // True if quotes values are required to be properly quoted; false if
     // mismatched quotes and other problems with quoted values should be more
     // or less gracefully treated as valid.
     bool strict_quotes_;
   };
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_UTIL_H_