Move BuildRequestHeaders from HttpNetworkTransaction to

HttpUtil::BuildRequestHeaders, so that it can be used
directly by SpdyHttpStream, removing the code duplication.

Strip proxy-connection header from SPDY requests to be consistent with Connection header.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3831005

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

9 files changed, 140 insertions, 134 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index fc1def9..0d9f340 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -60,75 +60,6 @@ namespace net {
 
 namespace {
 
-void BuildRequestHeaders(const HttpRequestInfo* request_info,
-                         const HttpRequestHeaders& authorization_headers,
-                         const UploadDataStream* upload_data_stream,
-                         bool using_proxy,
-                         std::string* request_line,
-                         HttpRequestHeaders* request_headers) {
-  const std::string path = using_proxy ?
-                           HttpUtil::SpecForRequest(request_info->url) :
-                           HttpUtil::PathForRequest(request_info->url);
-  *request_line = base::StringPrintf(
-      "%s %s HTTP/1.1\r\n", request_info->method.c_str(), path.c_str());
-  request_headers->SetHeader(HttpRequestHeaders::kHost,
-                             GetHostAndOptionalPort(request_info->url));
-
-  // For compat with HTTP/1.0 servers and proxies:
-  if (using_proxy) {
-    request_headers->SetHeader(HttpRequestHeaders::kProxyConnection,
-                               "keep-alive");
-  } else {
-    request_headers->SetHeader(HttpRequestHeaders::kConnection, "keep-alive");
-  }
-
-  // Our consumer should have made sure that this is a safe referrer.  See for
-  // instance WebCore::FrameLoader::HideReferrer.
-  if (request_info->referrer.is_valid()) {
-    request_headers->SetHeader(HttpRequestHeaders::kReferer,
-                               request_info->referrer.spec());
-  }
-
-  // Add a content length header?
-  if (upload_data_stream) {
-    request_headers->SetHeader(
-        HttpRequestHeaders::kContentLength,
-        base::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->SetHeader(HttpRequestHeaders::kContentLength, "0");
-  }
-
-  // Honor load flags that impact proxy caches.
-  if (request_info->load_flags & LOAD_BYPASS_CACHE) {
-    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->SetHeader(HttpRequestHeaders::kCacheControl, "max-age=0");
-  }
-
-  request_headers->MergeFrom(authorization_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 stripped_extra_headers;
-  stripped_extra_headers.CopyFrom(request_info->extra_headers);
-  for (size_t i = 0; i < arraysize(kExtraHeadersToBeStripped); ++i)
-    stripped_extra_headers.RemoveHeader(kExtraHeadersToBeStripped[i]);
-  request_headers->MergeFrom(stripped_extra_headers);
-}
-
 void ProcessAlternateProtocol(HttpStreamFactory* factory,
                               HttpAlternateProtocols* alternate_protocols,
                               const HttpResponseHeaders& headers,
@@ -695,26 +626,19 @@ int HttpNetworkTransaction::DoSendRequest() {
   // This is constructed lazily (instead of within our Start method), so that
   // we have proxy info available.
   if (request_headers_.empty() && !response_.was_fetched_via_spdy) {
-    // Figure out if we can/should add Proxy-Authentication & Authentication
-    // headers.
-    HttpRequestHeaders authorization_headers;
-    bool have_proxy_auth = (ShouldApplyProxyAuth() &&
-                            HaveAuth(HttpAuth::AUTH_PROXY));
-    bool have_server_auth = (ShouldApplyServerAuth() &&
-                             HaveAuth(HttpAuth::AUTH_SERVER));
-    if (have_proxy_auth)
-      auth_controllers_[HttpAuth::AUTH_PROXY]->AddAuthorizationHeader(
-          &authorization_headers);
-    if (have_server_auth)
-      auth_controllers_[HttpAuth::AUTH_SERVER]->AddAuthorizationHeader(
-          &authorization_headers);
-    std::string request_line;
-    HttpRequestHeaders request_headers;
+    bool using_proxy = (proxy_info_.is_http()|| proxy_info_.is_https()) &&
+                        !is_https_request();
+    const std::string path = using_proxy ?
+                             HttpUtil::SpecForRequest(request_->url) :
+                             HttpUtil::PathForRequest(request_->url);
+    std::string request_line = base::StringPrintf(
+        "%s %s HTTP/1.1\r\n", request_->method.c_str(), path.c_str());
 
-    BuildRequestHeaders(request_, authorization_headers, request_body,
-                        !is_https_request() && (proxy_info_.is_http() ||
-                                        proxy_info_.is_https()),
-                        &request_line, &request_headers);
+    HttpRequestHeaders request_headers;
+    HttpUtil::BuildRequestHeaders(request_, request_body, auth_controllers_,
+                                  ShouldApplyServerAuth(),
+                                  ShouldApplyProxyAuth(), using_proxy,
+                                  &request_headers);
 
     if (session_->network_delegate())
       session_->network_delegate()->OnSendHttpRequest(&request_headers);
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index 0f45ccc..5c7a4fc 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -14,7 +14,12 @@
 #include "base/string_number_conversions.h"
 #include "base/string_piece.h"
 #include "base/string_util.h"
+#include "net/base/load_flags.h"
 #include "net/base/net_util.h"
+#include "net/base/upload_data_stream.h"
+#include "net/http/http_request_info.h"
+#include "net/http/http_request_headers.h"
+#include "net/http/http_auth_controller.h"
 
 using std::string;
 
@@ -631,6 +636,90 @@ HttpUtil::HeadersIterator::HeadersIterator(string::const_iterator headers_begin,
     : lines_(headers_begin, headers_end, line_delimiter) {
 }
 
+namespace {
+
+bool HaveAuth(const scoped_refptr<HttpAuthController> auth_controllers[],
+              HttpAuth::Target target) {
+  return auth_controllers[target].get() &&
+      auth_controllers[target]->HaveAuth();
+}
+
+}  // namespace
+
+void HttpUtil::BuildRequestHeaders(const HttpRequestInfo* request_info,
+                                   const UploadDataStream* upload_data_stream,
+                                   const scoped_refptr<HttpAuthController>
+                                       auth_controllers[],
+                                   bool should_apply_server_auth,
+                                   bool should_apply_proxy_auth,
+                                   bool using_proxy,
+                                   HttpRequestHeaders* request_headers) {
+  request_headers->SetHeader(HttpRequestHeaders::kHost,
+                             GetHostAndOptionalPort(request_info->url));
+
+  // For compat with HTTP/1.0 servers and proxies:
+  if (using_proxy) {
+    request_headers->SetHeader(HttpRequestHeaders::kProxyConnection,
+                               "keep-alive");
+  } else {
+    request_headers->SetHeader(HttpRequestHeaders::kConnection, "keep-alive");
+  }
+
+  // Our consumer should have made sure that this is a safe referrer.  See for
+  // instance WebCore::FrameLoader::HideReferrer.
+  if (request_info->referrer.is_valid()) {
+    request_headers->SetHeader(HttpRequestHeaders::kReferer,
+                               request_info->referrer.spec());
+  }
+
+  // Add a content length header?
+  if (upload_data_stream) {
+    request_headers->SetHeader(
+        HttpRequestHeaders::kContentLength,
+        base::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->SetHeader(HttpRequestHeaders::kContentLength, "0");
+  }
+
+  // Honor load flags that impact proxy caches.
+  if (request_info->load_flags & LOAD_BYPASS_CACHE) {
+    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->SetHeader(HttpRequestHeaders::kCacheControl, "max-age=0");
+  }
+
+  if (should_apply_proxy_auth &&
+      HaveAuth(auth_controllers, HttpAuth::AUTH_PROXY))
+    auth_controllers[HttpAuth::AUTH_PROXY]->AddAuthorizationHeader(
+        request_headers);
+  if (should_apply_server_auth &&
+      HaveAuth(auth_controllers, HttpAuth::AUTH_SERVER))
+    auth_controllers[HttpAuth::AUTH_SERVER]->AddAuthorizationHeader(
+        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 stripped_extra_headers;
+  stripped_extra_headers.CopyFrom(request_info->extra_headers);
+  for (size_t i = 0; i < arraysize(kExtraHeadersToBeStripped); ++i)
+    stripped_extra_headers.RemoveHeader(kExtraHeadersToBeStripped[i]);
+  request_headers->MergeFrom(stripped_extra_headers);
+}
+
+
 HttpUtil::HeadersIterator::~HeadersIterator() {
 }
 
diff --git a/net/http/http_util.h b/net/http/http_util.h
index 631790b..f41c4e4 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/ref_counted.h"
 #include "base/string_tokenizer.h"
 #include "googleurl/src/gurl.h"
 #include "net/http/http_byte_range.h"
@@ -19,7 +20,11 @@
 
 namespace net {
 
+class HttpAuthController;
+struct HttpRequestInfo;
+class HttpRequestHeaders;
 class HttpStream;
+class UploadDataStream;
 
 class HttpUtil {
  public:
@@ -159,6 +164,19 @@ class HttpUtil {
                                     const std::string& header_value,
                                     std::string* headers);
 
+  // Constructs |request_headers| from the information contained in
+  // |request_info|.  The correct server and proxy auth headers will
+  // be populated from |auth_controllers| if |enable_server_auth| or
+  // |enable_proxy_auth| is true.
+  static void BuildRequestHeaders(const HttpRequestInfo* request_info,
+                                  const UploadDataStream* upload_data_stream,
+                                  const scoped_refptr<HttpAuthController>
+                                      auth_controllers[],
+                                  bool enable_server_auth,
+                                  bool enable_proxy_auth,
+                                  bool enable_full_url,
+                                  HttpRequestHeaders* request_headers);
+
   // Used to iterate over the name/value pairs of HTTP headers.  To iterate
   // over the values in a multi-value header, use ValuesIterator.
   // See AssembleRawHeaders for joining line continuations (this iterator
diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 0e3962e..6fede6c 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -148,8 +148,12 @@ int SpdyHttpStream::SendRequest(const std::string& /*headers_string*/,
 
   stream_->SetDelegate(this);
 
+  HttpRequestHeaders request_headers;
+  HttpUtil::BuildRequestHeaders(request_info_, request_body, NULL, false, false,
+                                !direct_, &request_headers);
   linked_ptr<spdy::SpdyHeaderBlock> headers(new spdy::SpdyHeaderBlock);
-  CreateSpdyHeadersFromHttpRequest(*request_info_, headers.get(), direct_);
+  CreateSpdyHeadersFromHttpRequest(*request_info_, request_headers,
+                                   headers.get(), direct_);
   stream_->set_spdy_headers(headers);
 
   stream_->SetRequestTime(request_time);
diff --git a/net/spdy/spdy_http_utils.cc b/net/spdy/spdy_http_utils.cc
index b2ef551..09ac79c 100644
--- a/net/spdy/spdy_http_utils.cc
+++ b/net/spdy/spdy_http_utils.cc
@@ -79,18 +79,16 @@ bool SpdyHeadersToHttpResponse(const spdy::SpdyHeaderBlock& headers,
   return true;
 }
 
-void CreateSpdyHeadersFromHttpRequest(
-    const HttpRequestInfo& info, spdy::SpdyHeaderBlock* headers,
-    bool direct) {
-  // TODO(willchan): It's not really necessary to convert from
-  // HttpRequestHeaders to spdy::SpdyHeaderBlock.
-
-  static const char kHttpProtocolVersion[] = "HTTP/1.1";
-
-  HttpRequestHeaders::Iterator it(info.extra_headers);
+void CreateSpdyHeadersFromHttpRequest(const HttpRequestInfo& info,
+                                      const HttpRequestHeaders& request_headers,
+                                      spdy::SpdyHeaderBlock* headers,
+                                      bool direct) {
 
+  HttpRequestHeaders::Iterator it(request_headers);
   while (it.GetNext()) {
     std::string name = StringToLowerASCII(it.name());
+    if (name == "connection" || name == "proxy-connection")
+      continue;
     if (headers->find(name) == headers->end()) {
       (*headers)[name] = it.value();
     } else {
@@ -100,44 +98,17 @@ void CreateSpdyHeadersFromHttpRequest(
       (*headers)[name] = new_value;
     }
   }
+  static const char kHttpProtocolVersion[] = "HTTP/1.1";
 
-  // TODO(rch): Add Proxy headers here. (See http_network_transaction.cc)
-  // TODO(rch): Add authentication headers here.
-
+  (*headers)["version"] = kHttpProtocolVersion;
   (*headers)["method"] = info.method;
-
-  // Handle content-length. This is the same as BuildRequestHeader in
-  // http_network_transaction.cc.
-  // TODO(lzheng): reduce the code duplication between spdy and http here.
-  if (info.upload_data) {
-    (*headers)["content-length"] =
-        base::Int64ToString(info.upload_data->GetContentLength());
-  } else if (info.method == "POST" || info.method == "PUT" ||
-             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.
-    (*headers)["content-length"] = "0";
-  }
-
+  (*headers)["host"] = GetHostAndOptionalPort(info.url);
+  (*headers)["scheme"] = info.url.scheme();
   if (direct)
     (*headers)["url"] = HttpUtil::PathForRequest(info.url);
   else
     (*headers)["url"] = HttpUtil::SpecForRequest(info.url);
-  (*headers)["host"] = GetHostAndOptionalPort(info.url);
-  (*headers)["scheme"] = info.url.scheme();
-  (*headers)["version"] = kHttpProtocolVersion;
-  if (!info.referrer.is_empty())
-    (*headers)["referer"] = info.referrer.spec();
 
-  // Honor load flags that impact proxy caches.
-  if (info.load_flags & LOAD_BYPASS_CACHE) {
-    (*headers)["pragma"] = "no-cache";
-    (*headers)["cache-control"] = "no-cache";
-  } else if (info.load_flags & LOAD_VALIDATE_CACHE) {
-    (*headers)["cache-control"] = "max-age=0";
-  }
 }
 
 }  // namespace net
diff --git a/net/spdy/spdy_http_utils.h b/net/spdy/spdy_http_utils.h
index 7ec29a5..c6946dd 100644
--- a/net/spdy/spdy_http_utils.h
+++ b/net/spdy/spdy_http_utils.h
@@ -12,6 +12,7 @@ namespace net {
 
 class HttpResponseInfo;
 struct HttpRequestInfo;
+class HttpRequestHeaders;
 
 // Convert a SpdyHeaderBlock into an HttpResponseInfo.
 // |headers| input parameter with the SpdyHeaderBlock.
@@ -22,8 +23,9 @@ bool SpdyHeadersToHttpResponse(const spdy::SpdyHeaderBlock& headers,
                                HttpResponseInfo* response);
 
 // Create a SpdyHeaderBlock for a Spdy SYN_STREAM Frame from
-// a HttpRequestInfo block.
+// HttpRequestInfo and HttpRequestHeaders.
 void CreateSpdyHeadersFromHttpRequest(const HttpRequestInfo& info,
+                                      const HttpRequestHeaders& request_headers,
                                       spdy::SpdyHeaderBlock* headers,
                                       bool direct);
 
diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
index c8a0153..53bcb0d 100644
--- a/net/spdy/spdy_proxy_client_socket.cc
+++ b/net/spdy/spdy_proxy_client_socket.cc
@@ -312,7 +312,8 @@ int SpdyProxyClientSocket::DoSendRequest() {
 
   request_.extra_headers.MergeFrom(request_headers);
   linked_ptr<spdy::SpdyHeaderBlock> headers(new spdy::SpdyHeaderBlock());
-  CreateSpdyHeadersFromHttpRequest(request_, headers.get(), true);
+  CreateSpdyHeadersFromHttpRequest(request_, request_headers, headers.get(),
+                                   true);
   // Reset the URL to be the endpoint of the connection
   (*headers)["url"] = endpoint_.ToString();
   headers->erase("scheme");
diff --git a/net/spdy/spdy_proxy_client_socket_unittest.cc b/net/spdy/spdy_proxy_client_socket_unittest.cc
index 9639036..236713ac 100644
--- a/net/spdy/spdy_proxy_client_socket_unittest.cc
+++ b/net/spdy/spdy_proxy_client_socket_unittest.cc
@@ -311,7 +311,6 @@ spdy::SpdyFrame* SpdyProxyClientSocketTest::ConstructConnectRequestFrame() {
     "host", kOriginHost,
     "user-agent", kUserAgent,
     "version", "HTTP/1.1",
-    "proxy-connection", "keep-alive",
   };
   return ConstructSpdyPacket(
       kSynStartHeader, NULL, 0, kConnectHeaders, arraysize(kConnectHeaders)/2);
@@ -339,7 +338,6 @@ spdy::SpdyFrame* SpdyProxyClientSocketTest::ConstructConnectAuthRequestFrame() {
     "user-agent", kUserAgent,
     "version", "HTTP/1.1",
     "proxy-authorization", "Basic Zm9vOmJhcg==",
-    "proxy-connection", "keep-alive",
   };
   return ConstructSpdyPacket(
       kSynStartHeader, NULL, 0, kConnectHeaders, arraysize(kConnectHeaders)/2);
diff --git a/net/spdy/spdy_test_util.cc b/net/spdy/spdy_test_util.cc
index 3dda1edd..d3b82c0c6 100644
--- a/net/spdy/spdy_test_util.cc
+++ b/net/spdy/spdy_test_util.cc
@@ -416,7 +416,6 @@ spdy::SpdyFrame* ConstructSpdyConnect(const char* const extra_headers[],
     "url", "www.google.com:443",
     "host", "www.google.com",
     "version", "HTTP/1.1",
-    "proxy-connection", "keep-alive",
   };
   return ConstructSpdyControlFrame(extra_headers,
                                    extra_header_count,