Revert 54381 - Revert 54378 - Add content-length to spdy post request header.

I was rolled back due to this failure:
http://build.chromium.org/buildbot/waterfall/builders/Modules%20Mac10.6%20(dbg)/builds/9160
I think that failure is not related to this CL (there might be a hidden problem with the WriteError test). I will disable that test and fix it if needed.

TEST=spdy_network_transaction_unittest.cc
BUG=50545
Review URL: http://codereview.chromium.org/3023029

TBR=mbelshe@google.com
Review URL: http://codereview.chromium.org/3041035

TBR=lzheng@chromium.org
Review URL: http://codereview.chromium.org/3082009

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

4 files changed, 115 insertions, 24 deletions

diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index b723b4e9..2d9fe39 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -112,6 +112,22 @@ void CreateSpdyHeadersFromHttpRequest(
   // TODO(mbelshe): Add authentication headers here.
 
   (*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"] =
+        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)["url"] = net::HttpUtil::PathForRequest(info.url);
   (*headers)["host"] = net::GetHostAndOptionalPort(info.url);
   (*headers)["scheme"] = info.url.scheme();
diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 8da5389..66feee0 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <vector>
+
 #include "net/base/net_log_unittest.h"
 #include "net/http/http_transaction_unittest.h"
 #include "net/spdy/spdy_http_stream.h"
@@ -60,7 +62,7 @@ class SpdyNetworkTransactionTest
           session_(SpdySessionDependencies::SpdyCreateSession(&session_deps_)),
           log_(log),
           test_type_(test_type) {
-            switch(test_type_) {
+            switch (test_type_) {
               case SPDYNOSSL:
               case SPDYSSL:
                 port_ = 80;
@@ -77,7 +79,7 @@ class SpdyNetworkTransactionTest
       HttpNetworkTransaction::SetUseAlternateProtocols(false);
       HttpNetworkTransaction::SetUseSSLOverSpdyWithoutNPN(false);
       HttpNetworkTransaction::SetUseSpdyWithoutNPN(false);
-      switch(test_type_) {
+      switch (test_type_) {
         case SPDYNPN:
           session_->mutable_alternate_protocols()->SetAlternateProtocolFor(
               HostPortPair("www.google.com", 80), 443,
@@ -122,11 +124,10 @@ class SpdyNetworkTransactionTest
       ASSERT_TRUE(response->headers != NULL);
       EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
       EXPECT_TRUE(response->was_fetched_via_spdy);
-      if(test_type_ == SPDYNPN) {
+      if (test_type_ == SPDYNPN) {
         EXPECT_TRUE(response->was_npn_negotiated);
         EXPECT_TRUE(response->was_alternate_protocol_available);
-      }
-      else {
+      } else {
         EXPECT_TRUE(!response->was_npn_negotiated);
         EXPECT_TRUE(!response->was_alternate_protocol_available);
       }
@@ -141,7 +142,7 @@ class SpdyNetworkTransactionTest
     // should end with an empty read, and that read needs to be processed to
     // ensure proper deletion of the spdy_session_pool.
     void VerifyDataConsumed() {
-      for(DataVector::iterator it = data_vector_.begin();
+      for (DataVector::iterator it = data_vector_.begin();
           it != data_vector_.end(); ++it) {
         EXPECT_TRUE((*it)->at_read_eof()) << "Read count: "
                                           << (*it)->read_count()
@@ -158,7 +159,7 @@ class SpdyNetworkTransactionTest
     // processed. In that case we want to explicitly ensure that the reads were
     // not processed.
     void VerifyDataNotConsumed() {
-      for(DataVector::iterator it = data_vector_.begin();
+      for (DataVector::iterator it = data_vector_.begin();
           it != data_vector_.end(); ++it) {
         EXPECT_TRUE(!(*it)->at_read_eof()) << "Read count: "
                                            << (*it)->read_count()
@@ -168,7 +169,6 @@ class SpdyNetworkTransactionTest
                                             << (*it)->write_count()
                                             << " Write index: "
                                             << (*it)->write_index();
-
       }
     }
 
@@ -183,13 +183,13 @@ class SpdyNetworkTransactionTest
       data_vector_.push_back(data);
       linked_ptr<SSLSocketDataProvider> ssl_(
           new SSLSocketDataProvider(true, OK));
-      if(test_type_ == SPDYNPN) {
+      if (test_type_ == SPDYNPN) {
         ssl_->next_proto_status = SSLClientSocket::kNextProtoNegotiated;
         ssl_->next_proto = "spdy/2";
         ssl_->was_npn_negotiated = true;
       }
       ssl_vector_.push_back(ssl_);
-      if(test_type_ == SPDYNPN || test_type_ == SPDYSSL)
+      if (test_type_ == SPDYNPN || test_type_ == SPDYSSL)
         session_deps_.socket_factory.AddSSLSocketDataProvider(ssl_.get());
       session_deps_.socket_factory.AddSocketDataProvider(data);
     }
@@ -200,7 +200,7 @@ class SpdyNetworkTransactionTest
       session_deps_.socket_factory.AddSocketDataProvider(data);
     }
 
-    void SetSession(scoped_refptr<HttpNetworkSession>& session) {
+    void SetSession(const scoped_refptr<HttpNetworkSession>& session) {
       session_ = session;
     }
     HttpNetworkTransaction* trans() { return trans_.get(); }
@@ -394,7 +394,6 @@ TEST_P(SpdyNetworkTransactionTest, ThreeGets) {
   EXPECT_EQ("hello!hello!", out.response_data);
 }
 
-
 // Similar to ThreeGets above, however this test adds a SETTINGS
 // frame.  The SETTINGS frame is read during the IO loop waiting on
 // the first transaction completion, and sets a maximum concurrent
@@ -798,7 +797,13 @@ TEST_P(SpdyNetworkTransactionTest, Post) {
   request.upload_data = new UploadData();
   request.upload_data->AppendBytes(upload, strlen(upload));
 
-  scoped_ptr<spdy::SpdyFrame> req(ConstructSpdyPost(NULL, 0));
+  // Http POST Content-Length is using UploadDataStream::size().
+  // It is the same as request.upload_data->GetContentLength().
+  ASSERT_EQ(request.upload_data->GetContentLength(),
+            UploadDataStream::Create(request.upload_data, NULL)->size());
+
+  scoped_ptr<spdy::SpdyFrame>
+      req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0));
   scoped_ptr<spdy::SpdyFrame> body(ConstructSpdyBodyFrame(1, true));
   MockWrite writes[] = {
     CreateMockWrite(*req),
@@ -824,6 +829,46 @@ TEST_P(SpdyNetworkTransactionTest, Post) {
   EXPECT_EQ("hello!", out.response_data);
 }
 
+// Test that a POST without any post data works.
+TEST_P(SpdyNetworkTransactionTest, NullPost) {
+  // Setup the request
+  HttpRequestInfo request;
+  request.method = "POST";
+  request.url = GURL("http://www.google.com/");
+  // Create an empty UploadData.
+  request.upload_data = NULL;
+
+  // When request.upload_data is NULL for post, content-length is
+  // expected to be 0.
+  scoped_ptr<spdy::SpdyFrame> req(ConstructSpdyPost(0, NULL, 0));
+  // Set the FIN bit since there will be no body.
+  req->set_flags(spdy::CONTROL_FLAG_FIN);
+  MockWrite writes[] = {
+    CreateMockWrite(*req),
+  };
+
+  scoped_ptr<spdy::SpdyFrame> resp(ConstructSpdyPostSynReply(NULL, 0));
+  scoped_ptr<spdy::SpdyFrame> body(ConstructSpdyBodyFrame(1, true));
+  MockRead reads[] = {
+    CreateMockRead(*resp),
+    CreateMockRead(*body),
+    MockRead(true, 0, 0)  // EOF
+  };
+
+  scoped_refptr<DelayedSocketData> data(
+      new DelayedSocketData(1, reads, arraysize(reads),
+                            writes, arraysize(writes)));
+
+  NormalSpdyTransactionHelper helper(request,
+                                     BoundNetLog(), GetParam());
+  helper.RunToCompletion(data.get());
+  TransactionHelperResult out = helper.output();
+  EXPECT_EQ(OK, out.rv);
+  EXPECT_EQ("HTTP/1.1 200 OK", out.status_line);
+  EXPECT_EQ("hello!", out.response_data);
+}
+
+
 // Test that a simple POST works.
 TEST_P(SpdyNetworkTransactionTest, EmptyPost) {
   // Setup the request
@@ -833,7 +878,13 @@ TEST_P(SpdyNetworkTransactionTest, EmptyPost) {
   // Create an empty UploadData.
   request.upload_data = new UploadData();
 
-  scoped_ptr<spdy::SpdyFrame> req(ConstructSpdyPost(NULL, 0));
+  // Http POST Content-Length is using UploadDataStream::size().
+  // It is the same as request.upload_data->GetContentLength().
+  ASSERT_EQ(request.upload_data->GetContentLength(),
+            UploadDataStream::Create(request.upload_data, NULL)->size());
+
+  scoped_ptr<spdy::SpdyFrame>
+      req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0));
   // Set the FIN bit since there will be no body.
   req->set_flags(spdy::CONTROL_FLAG_FIN);
   MockWrite writes[] = {
@@ -872,7 +923,13 @@ TEST_P(SpdyNetworkTransactionTest, PostWithEarlySynReply) {
   request.upload_data = new UploadData();
   request.upload_data->AppendBytes(upload, sizeof(upload));
 
-  scoped_ptr<spdy::SpdyFrame> req(ConstructSpdyPost(NULL, 0));
+  // Http POST Content-Length is using UploadDataStream::size().
+  // It is the same as request.upload_data->GetContentLength().
+  ASSERT_EQ(request.upload_data->GetContentLength(),
+            UploadDataStream::Create(request.upload_data, NULL)->size());
+
+  scoped_ptr<spdy::SpdyFrame>
+      req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0));
   scoped_ptr<spdy::SpdyFrame> body(ConstructSpdyBodyFrame(1, true));
     MockWrite writes[] = {
     CreateMockWrite(*req.get(), 2),
@@ -977,7 +1034,13 @@ TEST_P(SpdyNetworkTransactionTest, WindowUpdate) {
   request.upload_data = new UploadData();
   request.upload_data->AppendBytes(kUploadData, kUploadDataSize);
 
-  scoped_ptr<spdy::SpdyFrame> req(ConstructSpdyPost(NULL, 0));
+  // Http POST Content-Length is using UploadDataStream::size().
+  // It is the same as request.upload_data->GetContentLength().
+  ASSERT_EQ(request.upload_data->GetContentLength(),
+            UploadDataStream::Create(request.upload_data, NULL)->size());
+
+  scoped_ptr<spdy::SpdyFrame>
+      req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0));
   scoped_ptr<spdy::SpdyFrame> body(ConstructSpdyBodyFrame(1, true));
   MockWrite writes[] = {
     CreateMockWrite(*req),
@@ -1043,7 +1106,13 @@ TEST_P(SpdyNetworkTransactionTest, WindowUpdateOverflow) {
   request.upload_data = new UploadData();
   request.upload_data->AppendBytes(kUploadData, arraysize(kUploadData)-1);
 
-  scoped_ptr<spdy::SpdyFrame> req(ConstructSpdyPost(NULL, 0));
+  // Http POST Content-Length is using UploadDataStream::size().
+  // It is the same as request.upload_data->GetContentLength().
+  ASSERT_EQ(request.upload_data->GetContentLength(),
+            UploadDataStream::Create(request.upload_data, NULL)->size());
+
+  scoped_ptr<spdy::SpdyFrame>
+      req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0));
   scoped_ptr<spdy::SpdyFrame> body(ConstructSpdyBodyFrame(1, true));
   scoped_ptr<spdy::SpdyFrame> rst(
       ConstructSpdyRstStream(1, spdy::FLOW_CONTROL_ERROR));
@@ -1054,7 +1123,7 @@ TEST_P(SpdyNetworkTransactionTest, WindowUpdateOverflow) {
   };
 
   // Response frames, send WINDOW_UPDATE first
-  static const int kDeltaWindowSize = 0x7fffffff; // cause an overflow
+  static const int kDeltaWindowSize = 0x7fffffff;  // cause an overflow
   scoped_ptr<spdy::SpdyFrame> window_update(
       ConstructSpdyWindowUpdate(1, kDeltaWindowSize));
   scoped_ptr<spdy::SpdyFrame> reply(ConstructSpdyPostSynReply(NULL, 0));
diff --git a/net/spdy/spdy_test_util.cc b/net/spdy/spdy_test_util.cc
index 4d30d0c..5d65b4a 100644
--- a/net/spdy/spdy_test_util.cc
+++ b/net/spdy/spdy_test_util.cc
@@ -385,10 +385,12 @@ spdy::SpdyFrame* ConstructSpdyGetSynReply(const char* const extra_headers[],
 }
 
 // Constructs a standard SPDY POST SYN packet.
+// |content_length| is the size of post data.
 // |extra_headers| are the extra header-value pairs, which typically
 // will vary the most between calls.
 // Returns a SpdyFrame.
-spdy::SpdyFrame* ConstructSpdyPost(const char* const extra_headers[],
+spdy::SpdyFrame* ConstructSpdyPost(int64 content_length,
+                                   const char* const extra_headers[],
                                    int extra_header_count) {
   const SpdyHeaderInfo kSynStartHeader = {
     spdy::SYN_STREAM,             // Kind = Syn
@@ -402,7 +404,8 @@ spdy::SpdyFrame* ConstructSpdyPost(const char* const extra_headers[],
     0,                            // Length
     spdy::DATA_FLAG_NONE          // Data Flags
   };
-  static const char* const kStandardGetHeaders[] = {
+  std::string length_str = Int64ToString(content_length);
+  const char* post_headers[] = {
     "method",
     "POST",
     "url",
@@ -412,14 +415,16 @@ spdy::SpdyFrame* ConstructSpdyPost(const char* const extra_headers[],
     "scheme",
     "http",
     "version",
-    "HTTP/1.1"
+    "HTTP/1.1",
+    "content-length",
+    length_str.c_str()
   };
   return ConstructSpdyPacket(
       kSynStartHeader,
       extra_headers,
       extra_header_count,
-      kStandardGetHeaders,
-      arraysize(kStandardGetHeaders) / 2);
+      post_headers,
+      arraysize(post_headers) / 2);
 }
 
 // Constructs a standard SPDY SYN_REPLY packet to match the SPDY POST.
diff --git a/net/spdy/spdy_test_util.h b/net/spdy/spdy_test_util.h
index 4114fe2..0282763 100644
--- a/net/spdy/spdy_test_util.h
+++ b/net/spdy/spdy_test_util.h
@@ -181,7 +181,8 @@ spdy::SpdyFrame* ConstructSpdyGetSynReply(const char* const extra_headers[],
 // |extra_headers| are the extra header-value pairs, which typically
 // will vary the most between calls.
 // Returns a SpdyFrame.
-spdy::SpdyFrame* ConstructSpdyPost(const char* const extra_headers[],
+spdy::SpdyFrame* ConstructSpdyPost(int64 content_length,
+                                   const char* const extra_headers[],
                                    int extra_header_count);
 
 // Constructs a standard SPDY SYN_REPLY packet to match the SPDY POST.