Avoid adding invalid headers in AddHeaderFromString

net/http/http_util.h

Index: net/http/http_util.h
diff --git a/net/http/http_util.h b/net/http/http_util.h
index 77706f1afea39a9f15695247298b4243468c4061..7d313a7b098acf44d6ed8578bc51abf80dea636e 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -12,6 +12,7 @@
 
 #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"
@@ -77,11 +78,11 @@ class NET_EXPORT HttpUtil {
   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
@@ -108,16 +109,17 @@ class NET_EXPORT HttpUtil {
   // 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,