Reland r42300: "HttpRequestHeaders refactor."

Adds a fix and tests for empty header values.  The particular bug happened when the value was non-empty, but was all LWS, so it was effectively empty.
BUG=22588

Review URL: http://codereview.chromium.org/1370001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43000 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 506 insertions, 68 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 57cf773..f3f4a7d 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -4,11 +4,11 @@
 
 #include "net/http/http_network_transaction.h"
 
-#include "base/format_macros.h"
-#include "base/scoped_ptr.h"
 #include "base/compiler_specific.h"
 #include "base/field_trial.h"
+#include "base/format_macros.h"
 #include "base/histogram.h"
+#include "base/scoped_ptr.h"
 #include "base/stats_counters.h"
 #include "base/string_util.h"
 #include "base/trace_event.h"
@@ -25,6 +25,7 @@
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_chunked_decoder.h"
 #include "net/http/http_network_session.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_response_info.h"
@@ -47,97 +48,112 @@ namespace {
 const std::string* g_next_protos = NULL;
 
 void BuildRequestHeaders(const HttpRequestInfo* request_info,
-                         const std::string& authorization_headers,
+                         const HttpRequestHeaders& authorization_headers,
                          const UploadDataStream* upload_data_stream,
                          bool using_proxy,
-                         std::string* request_headers) {
-  // Headers that will be stripped from request_info->extra_headers to prevent,
-  // e.g., plugins from overriding headers that are controlled using other
-  // means. Otherwise a plugin could set a referrer although sending the
-  // referrer is inhibited.
-  // TODO(jochen): check whether also other headers should be stripped.
-  static const char* const kExtraHeadersToBeStripped[] = {
-    "Referer"
-  };
-
+                         HttpRequestHeaders* request_headers) {
   const std::string path = using_proxy ?
       HttpUtil::SpecForRequest(request_info->url) :
       HttpUtil::PathForRequest(request_info->url);
-  *request_headers =
-      StringPrintf("%s %s HTTP/1.1\r\nHost: %s\r\n",
-                   request_info->method.c_str(), path.c_str(),
-                   GetHostAndOptionalPort(request_info->url).c_str());
+  request_headers->SetRequestLine(
+      request_info->method, path, "1.1");
+
+  request_headers->SetHeader(HttpRequestHeaders::kHost,
+                             GetHostAndOptionalPort(request_info->url));
 
   // For compat with HTTP/1.0 servers and proxies:
-  if (using_proxy)
-    *request_headers += "Proxy-";
-  *request_headers += "Connection: keep-alive\r\n";
+  if (using_proxy) {
+    request_headers->SetHeader(HttpRequestHeaders::kProxyConnection,
+                               "keep-alive");
+  } else {
+    request_headers->SetHeader(HttpRequestHeaders::kConnection, "keep-alive");
+  }
 
   if (!request_info->user_agent.empty()) {
-    StringAppendF(request_headers, "User-Agent: %s\r\n",
-                  request_info->user_agent.c_str());
+    request_headers->SetHeader(HttpRequestHeaders::kUserAgent,
+                               request_info->user_agent);
   }
 
   // Our consumer should have made sure that this is a safe referrer.  See for
   // instance WebCore::FrameLoader::HideReferrer.
-  if (request_info->referrer.is_valid())
-    StringAppendF(request_headers, "Referer: %s\r\n",
-                  request_info->referrer.spec().c_str());
+  if (request_info->referrer.is_valid()) {
+    request_headers->SetHeader(HttpRequestHeaders::kReferer,
+                               request_info->referrer.spec());
+  }
 
   // Add a content length header?
   if (upload_data_stream) {
-    StringAppendF(request_headers, "Content-Length: %" PRIu64 "\r\n",
-                  upload_data_stream->size());
+    request_headers->SetHeader(
+        HttpRequestHeaders::kContentLength,
+        Uint64ToString(upload_data_stream->size()));
   } else if (request_info->method == "POST" || request_info->method == "PUT" ||
              request_info->method == "HEAD") {
     // An empty POST/PUT request still needs a content length.  As for HEAD,
     // IE and Safari also add a content length header.  Presumably it is to
     // support sending a HEAD request to an URL that only expects to be sent a
     // POST or some other method that normally would have a message body.
-    *request_headers += "Content-Length: 0\r\n";
+    request_headers->SetHeader(HttpRequestHeaders::kContentLength, "0");
   }
 
   // Honor load flags that impact proxy caches.
   if (request_info->load_flags & LOAD_BYPASS_CACHE) {
-    *request_headers += "Pragma: no-cache\r\nCache-Control: no-cache\r\n";
+    request_headers->SetHeader(HttpRequestHeaders::kPragma, "no-cache");
+    request_headers->SetHeader(HttpRequestHeaders::kCacheControl, "no-cache");
   } else if (request_info->load_flags & LOAD_VALIDATE_CACHE) {
-    *request_headers += "Cache-Control: max-age=0\r\n";
+    request_headers->SetHeader(HttpRequestHeaders::kCacheControl, "max-age=0");
   }
 
-  if (!authorization_headers.empty()) {
-    *request_headers += authorization_headers;
-  }
+  request_headers->MergeFrom(authorization_headers);
 
-  // TODO(darin): Need to prune out duplicate headers.
+  // Headers that will be stripped from request_info->extra_headers to prevent,
+  // e.g., plugins from overriding headers that are controlled using other
+  // means. Otherwise a plugin could set a referrer although sending the
+  // referrer is inhibited.
+  // TODO(jochen): check whether also other headers should be stripped.
+  static const char* const kExtraHeadersToBeStripped[] = {
+    "Referer"
+  };
 
-  *request_headers += HttpUtil::StripHeaders(request_info->extra_headers,
-      kExtraHeadersToBeStripped, arraysize(kExtraHeadersToBeStripped));
-  *request_headers += "\r\n";
+  // TODO(willchan): Change HttpRequestInfo::extra_headers to be a
+  // HttpRequestHeaders.
+
+  std::vector<std::string> extra_headers_vector;
+  Tokenize(request_info->extra_headers, "\r\n", &extra_headers_vector);
+  HttpRequestHeaders extra_headers;
+  if (!extra_headers_vector.empty()) {
+    for (std::vector<std::string>::const_iterator it =
+         extra_headers_vector.begin(); it != extra_headers_vector.end(); ++it)
+      extra_headers.AddHeaderFromString(*it);
+
+    for (size_t i = 0; i < arraysize(kExtraHeadersToBeStripped); ++i)
+      extra_headers.RemoveHeader(kExtraHeadersToBeStripped[i]);
+
+    request_headers->MergeFrom(extra_headers);
+  }
 }
 
 // The HTTP CONNECT method for establishing a tunnel connection is documented
 // in draft-luotonen-web-proxy-tunneling-01.txt and RFC 2817, Sections 5.2 and
 // 5.3.
 void BuildTunnelRequest(const HttpRequestInfo* request_info,
-                        const std::string& authorization_headers,
-                        std::string* request_headers) {
+                        const HttpRequestHeaders& authorization_headers,
+                        HttpRequestHeaders* request_headers) {
   // RFC 2616 Section 9 says the Host request-header field MUST accompany all
   // HTTP/1.1 requests.  Add "Proxy-Connection: keep-alive" for compat with
   // HTTP/1.0 proxies such as Squid (required for NTLM authentication).
-  *request_headers = StringPrintf(
-      "CONNECT %s HTTP/1.1\r\nHost: %s\r\nProxy-Connection: keep-alive\r\n",
-      GetHostAndPort(request_info->url).c_str(),
-      GetHostAndOptionalPort(request_info->url).c_str());
+  request_headers->SetRequestLine(
+      "CONNECT", GetHostAndPort(request_info->url), "1.1");
+  request_headers->SetHeader(HttpRequestHeaders::kHost,
+                             GetHostAndOptionalPort(request_info->url));
+  request_headers->SetHeader(HttpRequestHeaders::kProxyConnection,
+                             "keep-alive");
 
-  if (!request_info->user_agent.empty())
-    StringAppendF(request_headers, "User-Agent: %s\r\n",
-                  request_info->user_agent.c_str());
-
-  if (!authorization_headers.empty()) {
-    *request_headers += authorization_headers;
+  if (!request_info->user_agent.empty()) {
+    request_headers->SetHeader(HttpRequestHeaders::kUserAgent,
+                               request_info->user_agent);
   }
 
-  *request_headers += "\r\n";
+  request_headers->MergeFrom(authorization_headers);
 }
 
 void ProcessAlternateProtocol(const HttpResponseHeaders& headers,
@@ -915,24 +931,25 @@ int HttpNetworkTransaction::DoSendRequest() {
         (HaveAuth(HttpAuth::AUTH_SERVER) ||
          SelectPreemptiveAuth(HttpAuth::AUTH_SERVER));
 
-    std::string authorization_headers;
+    HttpRequestHeaders request_headers;
+    HttpRequestHeaders authorization_headers;
 
     // TODO(wtc): If BuildAuthorizationHeader fails (returns an authorization
     // header with no credentials), we should return an error to prevent
     // entering an infinite auth restart loop.  See http://crbug.com/21050.
     if (have_proxy_auth)
-      authorization_headers.append(
-          BuildAuthorizationHeader(HttpAuth::AUTH_PROXY));
+      AddAuthorizationHeader(HttpAuth::AUTH_PROXY, &authorization_headers);
     if (have_server_auth)
-      authorization_headers.append(
-          BuildAuthorizationHeader(HttpAuth::AUTH_SERVER));
+      AddAuthorizationHeader(HttpAuth::AUTH_SERVER, &authorization_headers);
 
     if (establishing_tunnel_) {
-      BuildTunnelRequest(request_, authorization_headers, &request_headers_);
+      BuildTunnelRequest(request_, authorization_headers, &request_headers);
     } else {
       BuildRequestHeaders(request_, authorization_headers, request_body,
-                          proxy_mode_ == kHTTPProxy, &request_headers_);
+                          proxy_mode_ == kHTTPProxy, &request_headers);
     }
+
+    request_headers_ = request_headers.ToString();
   }
 
   headers_valid_ = false;
@@ -1611,8 +1628,8 @@ bool HttpNetworkTransaction::ShouldApplyServerAuth() const {
       !(request_->load_flags & LOAD_DO_NOT_SEND_AUTH_DATA);
 }
 
-std::string HttpNetworkTransaction::BuildAuthorizationHeader(
-    HttpAuth::Target target) const {
+void HttpNetworkTransaction::AddAuthorizationHeader(
+    HttpAuth::Target target, HttpRequestHeaders* authorization_headers) const {
   DCHECK(HaveAuth(target));
 
   // Add a Authorization/Proxy-Authorization header line.
@@ -1628,14 +1645,14 @@ std::string HttpNetworkTransaction::BuildAuthorizationHeader(
         auth_identity_[target].password,
         request_, &proxy_info_, &auth_token);
   }
-  if (rv == OK)
-    return HttpAuth::GetAuthorizationHeaderName(target) +
-      ": "  + auth_token + "\r\n";
+  if (rv == OK) {
+    authorization_headers->SetHeader(
+        HttpAuth::GetAuthorizationHeaderName(target), auth_token);
+  }
 
   // TODO(cbentzel): Evict username and password from cache on non-OK return?
   // TODO(cbentzel): Never use this scheme again if
   //                 ERR_UNSUPPORTED_AUTH_SCHEME is returned.
-  return std::string();
 }
 
 GURL HttpNetworkTransaction::AuthOrigin(HttpAuth::Target target) const {
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index 159f75d..113c0b5 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -33,6 +33,7 @@ class ClientSocketFactory;
 class ClientSocketHandle;
 class SpdyStream;
 class HttpNetworkSession;
+class HttpRequestHeaders;
 class HttpStream;
 
 class HttpNetworkTransaction : public HttpTransaction {
@@ -214,9 +215,10 @@ class HttpNetworkTransaction : public HttpTransaction {
   // Returns true if we should try to add an Authorization header.
   bool ShouldApplyServerAuth() const;
 
-  // Builds either the proxy auth header, or the origin server auth header,
+  // Adds either the proxy auth header, or the origin server auth header,
   // as specified by |target|.
-  std::string BuildAuthorizationHeader(HttpAuth::Target target) const;
+  void AddAuthorizationHeader(
+      HttpAuth::Target target, HttpRequestHeaders* authorization_headers) const;
 
   // Returns a log message for all the response headers related to the auth
   // challenge.
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index bef3237..02b784e8 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -3380,6 +3380,45 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_ExtraHeaders) {
   EXPECT_EQ(OK, rv);
 }
 
+TEST_F(HttpNetworkTransactionTest, BuildRequest_ExtraHeadersStripped) {
+  SessionDependencies session_deps;
+  scoped_ptr<HttpTransaction> trans(
+      new HttpNetworkTransaction(CreateSession(&session_deps)));
+
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("http://www.google.com/");
+  request.extra_headers = "referer: www.foo.com\nhEllo: Kitty\rFoO: bar\r\n";
+
+  MockWrite data_writes[] = {
+    MockWrite("GET / HTTP/1.1\r\n"
+              "Host: www.google.com\r\n"
+              "Connection: keep-alive\r\n"
+              "hEllo: Kitty\r\n"
+              "FoO: bar\r\n\r\n"),
+  };
+
+  // Lastly, the server responds with the actual content.
+  MockRead data_reads[] = {
+    MockRead("HTTP/1.0 200 OK\r\n"),
+    MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),
+    MockRead("Content-Length: 100\r\n\r\n"),
+    MockRead(false, OK),
+  };
+
+  StaticSocketDataProvider data(data_reads, arraysize(data_reads),
+                                data_writes, arraysize(data_writes));
+  session_deps.socket_factory.AddSocketDataProvider(&data);
+
+  TestCompletionCallback callback;
+
+  int rv = trans->Start(&request, &callback, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  rv = callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+}
+
 TEST_F(HttpNetworkTransactionTest, SOCKS4_HTTP_GET) {
   SessionDependencies session_deps(
       CreateFixedProxyService("socks4://myproxy:1080"));
diff --git a/net/http/http_request_headers.cc b/net/http/http_request_headers.cc
new file mode 100644
index 0000000..d894584
--- /dev/null
+++ b/net/http/http_request_headers.cc
@@ -0,0 +1,140 @@
+// Copyright (c) 2010 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 "net/http/http_request_headers.h"
+
+#include "base/logging.h"
+#include "base/string_util.h"
+#include "net/http/http_util.h"
+
+namespace net {
+
+const char HttpRequestHeaders::kGetMethod[] = "GET";
+
+const char HttpRequestHeaders::kCacheControl[] = "Cache-Control";
+const char HttpRequestHeaders::kConnection[] = "Connection";
+const char HttpRequestHeaders::kContentLength[] = "Content-Length";
+const char HttpRequestHeaders::kHost[] = "Host";
+const char HttpRequestHeaders::kPragma[] = "Pragma";
+const char HttpRequestHeaders::kProxyConnection[] = "Proxy-Connection";
+const char HttpRequestHeaders::kReferer[] = "Referer";
+const char HttpRequestHeaders::kUserAgent[] = "User-Agent";
+
+HttpRequestHeaders::HttpRequestHeaders() {}
+HttpRequestHeaders::~HttpRequestHeaders() {}
+
+void HttpRequestHeaders::SetRequestLine(const base::StringPiece& method,
+                                        const base::StringPiece& path,
+                                        const base::StringPiece& version) {
+  DCHECK(!method.empty());
+  DCHECK(!path.empty());
+  DCHECK(!version.empty());
+
+  method_.assign(method.data(), method.length());
+  path_.assign(path.data(), path.length());
+  version_.assign(version.data(), version.length());
+}
+
+void HttpRequestHeaders::SetHeader(const base::StringPiece& key,
+                                   const base::StringPiece& value) {
+  HeaderVector::iterator it = FindHeader(key);
+  if (it != headers_.end())
+    it->value = value.as_string();
+  else
+    headers_.push_back(HeaderKeyValuePair(key.as_string(), value.as_string()));
+}
+
+void HttpRequestHeaders::RemoveHeader(const base::StringPiece& key) {
+  HeaderVector::iterator it = FindHeader(key);
+  if (it != headers_.end())
+    headers_.erase(it);
+}
+
+void HttpRequestHeaders::AddHeaderFromString(
+    const base::StringPiece& header_line) {
+  DCHECK_EQ(std::string::npos, header_line.find("\r\n"))
+      << "\"" << header_line << "\" contains CRLF.";
+
+  const std::string::size_type key_end_index = header_line.find(":");
+  if (key_end_index == std::string::npos) {
+    LOG(DFATAL) << "\"" << header_line << "\" is missing colon delimiter.";
+    return;
+  }
+
+  if (key_end_index == 0) {
+    LOG(DFATAL) << "\"" << header_line << "\" is missing header key.";
+    return;
+  }
+
+  const base::StringPiece header_key(header_line.data(), key_end_index);
+
+  const std::string::size_type value_index = key_end_index + 1;
+
+  if (value_index < header_line.size()) {
+    std::string header_value(header_line.data() + value_index,
+                             header_line.size() - value_index);
+    std::string::const_iterator header_value_begin =
+        header_value.begin();
+    std::string::const_iterator header_value_end =
+        header_value.end();
+    HttpUtil::TrimLWS(&header_value_begin, &header_value_end);
+
+    if (header_value_begin == header_value_end) {
+      // Value was all LWS.
+      SetHeader(header_key, "");
+    } else {
+      SetHeader(header_key,
+                base::StringPiece(&*header_value_begin,
+                                  header_value_end - header_value_begin));
+    }
+  } else if (value_index == header_line.size()) {
+    SetHeader(header_key, "");
+  } else {
+    NOTREACHED();
+  }
+}
+
+void HttpRequestHeaders::MergeFrom(const HttpRequestHeaders& other) {
+  DCHECK(other.method_.empty());
+  DCHECK(other.path_.empty());
+  DCHECK(other.version_.empty());
+
+  for (HeaderVector::const_iterator it = other.headers_.begin();
+       it != other.headers_.end(); ++it ) {
+    SetHeader(it->key, it->value);
+  }
+}
+
+std::string HttpRequestHeaders::ToString() const {
+  std::string output;
+  if (!method_.empty()) {
+    DCHECK(!path_.empty());
+    DCHECK(!version_.empty());
+    output = StringPrintf(
+        "%s %s HTTP/%s\r\n", method_.c_str(), path_.c_str(), version_.c_str());
+  }
+  for (HeaderVector::const_iterator it = headers_.begin();
+       it != headers_.end(); ++it) {
+    if (!it->value.empty())
+      StringAppendF(&output, "%s: %s\r\n", it->key.c_str(), it->value.c_str());
+    else
+      StringAppendF(&output, "%s:\r\n", it->key.c_str());
+  }
+  output.append("\r\n");
+  return output;
+}
+
+HttpRequestHeaders::HeaderVector::iterator
+HttpRequestHeaders::FindHeader(const base::StringPiece& key) {
+  for (HeaderVector::iterator it = headers_.begin();
+       it != headers_.end(); ++it) {
+    if (key.length() == it->key.length() &&
+        !base::strncasecmp(key.data(), it->key.data(), key.length()))
+      return it;
+  }
+
+  return headers_.end();
+}
+
+}  // namespace net
diff --git a/net/http/http_request_headers.h b/net/http/http_request_headers.h
new file mode 100644
index 0000000..79522aa
--- /dev/null
+++ b/net/http/http_request_headers.h
@@ -0,0 +1,97 @@
+// Copyright (c) 2010 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.
+//
+// HttpRequestHeaders manages the request headers (including the request line).
+// It maintains these in a vector of header key/value pairs, thereby maintaining
+// the order of the headers.  This means that any lookups are linear time
+// operations.
+
+#ifndef NET_HTTP_HTTP_REQUEST_HEADERS_H_
+#define NET_HTTP_HTTP_REQUEST_HEADERS_H_
+
+#include <string>
+#include <vector>
+#include "base/basictypes.h"
+#include "base/string_piece.h"
+
+namespace net {
+
+class HttpRequestHeaders {
+ public:
+  static const char kGetMethod[];
+
+  static const char kCacheControl[];
+  static const char kConnection[];
+  static const char kContentLength[];
+  static const char kHost[];
+  static const char kPragma[];
+  static const char kProxyConnection[];
+  static const char kReferer[];
+  static const char kUserAgent[];
+
+  HttpRequestHeaders();
+  ~HttpRequestHeaders();
+
+  void SetRequestLine(const base::StringPiece& method,
+                      const base::StringPiece& path,
+                      const base::StringPiece& version);
+
+  // Sets the header value pair for |key| and |value|.  If |key| already exists,
+  // then the header value is modified, but the key is untouched, and the order
+  // in the vector remains the same.  When comparing |key|, case is ignored.
+  void SetHeader(const base::StringPiece& key, const base::StringPiece& value);
+
+  // Removes the first header that matches (case insensitive) |key|.
+  void RemoveHeader(const base::StringPiece& key);
+
+  // Parses the header from a string and calls SetHeader() with it.  This string
+  // should not contain any CRLF.  As per RFC2616, the format is:
+  //
+  // message-header = field-name ":" [ field-value ]
+  // field-name     = token
+  // field-value    = *( field-content | LWS )
+  // field-content  = <the OCTETs making up the field-value
+  //                  and consisting of either *TEXT or combinations
+  //                  of token, separators, and quoted-string>
+  //
+  // AddHeaderFromString() will trim any LWS surrounding the
+  // field-content.
+  void AddHeaderFromString(const base::StringPiece& header_line);
+
+  // Calls SetHeader() on each header from |other|, maintaining order.
+  void MergeFrom(const HttpRequestHeaders& other);
+
+  // Serializes HttpRequestHeaders to a string representation.  Joins all the
+  // header keys and values with ": ", and inserts "\r\n" between each header
+  // line, and adds the trailing "\r\n".
+  std::string ToString() const;
+
+ private:
+  struct HeaderKeyValuePair {
+    HeaderKeyValuePair() {}
+    HeaderKeyValuePair(const base::StringPiece& key,
+                       const base::StringPiece& value)
+        : key(key.data(), key.size()), value(value.data(), value.size()) {}
+
+    std::string key;
+    std::string value;
+  };
+
+  typedef std::vector<HeaderKeyValuePair> HeaderVector;
+
+  HeaderVector::iterator FindHeader(const base::StringPiece& key);
+  HeaderVector::const_iterator FindHeader(const base::StringPiece& key) const;
+
+  std::string method_;
+  std::string path_;
+  std::string version_;
+
+  HeaderVector headers_;
+
+  DISALLOW_COPY_AND_ASSIGN(HttpRequestHeaders);
+};
+
+}  // namespace net
+
+#endif  // NET_HTTP_HTTP_REQUEST_HEADERS_H_
diff --git a/net/http/http_request_headers_unittest.cc b/net/http/http_request_headers_unittest.cc
new file mode 100644
index 0000000..29431d9
--- /dev/null
+++ b/net/http/http_request_headers_unittest.cc
@@ -0,0 +1,140 @@
+// Copyright (c) 2010 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 "net/http/http_request_headers.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+TEST(HttpRequestHeaders, SetRequestLine) {
+  HttpRequestHeaders headers;
+  headers.SetRequestLine(
+      HttpRequestHeaders::kGetMethod, "/foo", "1.1");
+  EXPECT_EQ("GET /foo HTTP/1.1\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, SetHeader) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Foo", "bar");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, SetMultipleHeaders) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Cookie-Monster", "Nom nom nom");
+  headers.SetHeader("Domo-Kun", "Loves Chrome");
+  EXPECT_EQ("Cookie-Monster: Nom nom nom\r\nDomo-Kun: Loves Chrome\r\n\r\n",
+            headers.ToString());
+}
+
+TEST(HttpRequestHeaders, SetHeaderTwice) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Foo", "bar");
+  headers.SetHeader("Foo", "bar");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, SetHeaderTwiceCaseInsensitive) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Foo", "bar");
+  headers.SetHeader("FoO", "Bar");
+  EXPECT_EQ("Foo: Bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, SetHeaderTwiceSamePrefix) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("FooBar", "smokes");
+  headers.SetHeader("Foo", "crack");
+  EXPECT_EQ("FooBar: smokes\r\nFoo: crack\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, SetEmptyHeader) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Foo", "Bar");
+  headers.SetHeader("Bar", "");
+  EXPECT_EQ("Foo: Bar\r\nBar:\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, RemoveHeader) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Foo", "bar");
+  headers.RemoveHeader("Foo");
+  EXPECT_EQ("\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, RemoveHeaderMissingHeader) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Foo", "bar");
+  headers.RemoveHeader("Bar");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, RemoveHeaderCaseInsensitive) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("Foo", "bar");
+  headers.SetHeader("All-Your-Base", "Belongs To Chrome");
+  headers.RemoveHeader("foo");
+  EXPECT_EQ("All-Your-Base: Belongs To Chrome\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, AddHeaderFromString) {
+  HttpRequestHeaders headers;
+  headers.AddHeaderFromString("Foo: bar");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, AddHeaderFromStringNoLeadingWhitespace) {
+  HttpRequestHeaders headers;
+  headers.AddHeaderFromString("Foo:bar");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, AddHeaderFromStringMoreLeadingWhitespace) {
+  HttpRequestHeaders headers;
+  headers.AddHeaderFromString("Foo: \t  \t  bar");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, AddHeaderFromStringTrailingWhitespace) {
+  HttpRequestHeaders headers;
+  headers.AddHeaderFromString("Foo: bar  \t  \t   ");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, AddHeaderFromStringLeadingTrailingWhitespace) {
+  HttpRequestHeaders headers;
+  headers.AddHeaderFromString("Foo: \t    bar\t       ");
+  EXPECT_EQ("Foo: bar\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, AddHeaderFromStringWithEmptyValue) {
+  HttpRequestHeaders headers;
+  headers.AddHeaderFromString("Foo:");
+  EXPECT_EQ("Foo:\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, AddHeaderFromStringWithWhitespaceValue) {
+  HttpRequestHeaders headers;
+  headers.AddHeaderFromString("Foo: ");
+  EXPECT_EQ("Foo:\r\n\r\n", headers.ToString());
+}
+
+TEST(HttpRequestHeaders, MergeFrom) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("A", "A");
+  headers.SetHeader("B", "B");
+
+  HttpRequestHeaders headers2;
+  headers2.SetHeader("B", "b");
+  headers2.SetHeader("C", "c");
+  headers.MergeFrom(headers2);
+  EXPECT_EQ("A: A\r\nB: b\r\nC: c\r\n\r\n", headers.ToString());
+}
+
+}  // namespace
+
+}  // namespace net
diff --git a/net/net.gyp b/net/net.gyp
index 177f69f..74b151d 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -355,6 +355,8 @@
         'http/http_network_session.h',
         'http/http_network_transaction.cc',
         'http/http_network_transaction.h',
+        'http/http_request_headers.cc',
+        'http/http_request_headers.h',
         'http/http_request_info.h',
         'http/http_response_headers.cc',
         'http/http_response_headers.h',
@@ -673,6 +675,7 @@
         'http/http_chunked_decoder_unittest.cc',
         'http/http_network_layer_unittest.cc',
         'http/http_network_transaction_unittest.cc',
+        'http/http_request_headers_unittest.cc',
         'http/http_response_headers_unittest.cc',
         'http/http_transaction_unittest.cc',
         'http/http_transaction_unittest.h',
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index dc39667..439851b 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1092,7 +1092,7 @@ TEST_F(URLRequestTestHTTP, VaryHeader) {
     TestDelegate d;
     URLRequest req(server_->TestServerPage("echoheader?foo"), &d);
     req.set_context(context);
-    req.SetExtraRequestHeaders("foo:1");
+    req.SetExtraRequestHeaders("foo: 1");
     req.Start();
     MessageLoop::current()->Run();
   }
@@ -1102,7 +1102,7 @@ TEST_F(URLRequestTestHTTP, VaryHeader) {
     TestDelegate d;
     URLRequest req(server_->TestServerPage("echoheader?foo"), &d);
     req.set_context(context);
-    req.SetExtraRequestHeaders("foo:1");
+    req.SetExtraRequestHeaders("foo: 1");
     req.Start();
     MessageLoop::current()->Run();
 
@@ -1114,7 +1114,7 @@ TEST_F(URLRequestTestHTTP, VaryHeader) {
     TestDelegate d;
     URLRequest req(server_->TestServerPage("echoheader?foo"), &d);
     req.set_context(context);
-    req.SetExtraRequestHeaders("foo:2");
+    req.SetExtraRequestHeaders("foo: 2");
     req.Start();
     MessageLoop::current()->Run();