From a33cad2b626eef69aa54943c985f9ab030a8edb9 Mon Sep 17 00:00:00 2001
From: "lzheng@chromium.org"
<lzheng@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 30 Jul 2010 22:24:39 +0000
Subject: 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
---
net/spdy/spdy_http_stream.cc | 16 ++++
net/spdy/spdy_network_transaction_unittest.cc | 105 +++++++++++++++++++++-----
net/spdy/spdy_test_util.cc | 15 ++--
net/spdy/spdy_test_util.h | 3 +-
4 files changed, 115 insertions(+), 24 deletions(-)
(limited to 'net/spdy')
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.
--
cgit v1.1