From a6550f16d87527b7963e7e827c64b64bddeaaef2 Mon Sep 17 00:00:00 2001 From: "lzheng@chromium.org" Date: Fri, 30 Jul 2010 21:20:12 +0000 Subject: Revert 54378 - Add content-length to spdy post request header. http://build.chromium.org/buildbot/waterfall/builders/Modules%20Mac10.6%20(dbg)/builds/9160 TEST=spdy_network_transaction_unittest.cc BUG=50545 Review URL: http://codereview.chromium.org/3023029 TBR=lzheng@google.com Review URL: http://codereview.chromium.org/3041035 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54381 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, 24 insertions(+), 115 deletions(-) diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc index 2d9fe39..b723b4e9 100644 --- a/net/spdy/spdy_http_stream.cc +++ b/net/spdy/spdy_http_stream.cc @@ -112,22 +112,6 @@ 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 66feee0..8da5389 100644 --- a/net/spdy/spdy_network_transaction_unittest.cc +++ b/net/spdy/spdy_network_transaction_unittest.cc @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include - #include "net/base/net_log_unittest.h" #include "net/http/http_transaction_unittest.h" #include "net/spdy/spdy_http_stream.h" @@ -62,7 +60,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; @@ -79,7 +77,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, @@ -124,10 +122,11 @@ 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); } @@ -142,7 +141,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() @@ -159,7 +158,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() @@ -169,6 +168,7 @@ class SpdyNetworkTransactionTest << (*it)->write_count() << " Write index: " << (*it)->write_index(); + } } @@ -183,13 +183,13 @@ class SpdyNetworkTransactionTest data_vector_.push_back(data); linked_ptr 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(const scoped_refptr& session) { + void SetSession(scoped_refptr& session) { session_ = session; } HttpNetworkTransaction* trans() { return trans_.get(); } @@ -394,6 +394,7 @@ 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 @@ -797,13 +798,7 @@ TEST_P(SpdyNetworkTransactionTest, Post) { request.upload_data = new UploadData(); request.upload_data->AppendBytes(upload, strlen(upload)); - // 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 - req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0)); + scoped_ptr req(ConstructSpdyPost(NULL, 0)); scoped_ptr body(ConstructSpdyBodyFrame(1, true)); MockWrite writes[] = { CreateMockWrite(*req), @@ -829,46 +824,6 @@ 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 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 resp(ConstructSpdyPostSynReply(NULL, 0)); - scoped_ptr body(ConstructSpdyBodyFrame(1, true)); - MockRead reads[] = { - CreateMockRead(*resp), - CreateMockRead(*body), - MockRead(true, 0, 0) // EOF - }; - - scoped_refptr 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 @@ -878,13 +833,7 @@ TEST_P(SpdyNetworkTransactionTest, EmptyPost) { // Create an empty UploadData. request.upload_data = new UploadData(); - // 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 - req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0)); + scoped_ptr req(ConstructSpdyPost(NULL, 0)); // Set the FIN bit since there will be no body. req->set_flags(spdy::CONTROL_FLAG_FIN); MockWrite writes[] = { @@ -923,13 +872,7 @@ TEST_P(SpdyNetworkTransactionTest, PostWithEarlySynReply) { request.upload_data = new UploadData(); request.upload_data->AppendBytes(upload, sizeof(upload)); - // 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 - req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0)); + scoped_ptr req(ConstructSpdyPost(NULL, 0)); scoped_ptr body(ConstructSpdyBodyFrame(1, true)); MockWrite writes[] = { CreateMockWrite(*req.get(), 2), @@ -1034,13 +977,7 @@ TEST_P(SpdyNetworkTransactionTest, WindowUpdate) { request.upload_data = new UploadData(); request.upload_data->AppendBytes(kUploadData, kUploadDataSize); - // 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 - req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0)); + scoped_ptr req(ConstructSpdyPost(NULL, 0)); scoped_ptr body(ConstructSpdyBodyFrame(1, true)); MockWrite writes[] = { CreateMockWrite(*req), @@ -1106,13 +1043,7 @@ TEST_P(SpdyNetworkTransactionTest, WindowUpdateOverflow) { request.upload_data = new UploadData(); request.upload_data->AppendBytes(kUploadData, arraysize(kUploadData)-1); - // 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 - req(ConstructSpdyPost(request.upload_data->GetContentLength(), NULL, 0)); + scoped_ptr req(ConstructSpdyPost(NULL, 0)); scoped_ptr body(ConstructSpdyBodyFrame(1, true)); scoped_ptr rst( ConstructSpdyRstStream(1, spdy::FLOW_CONTROL_ERROR)); @@ -1123,7 +1054,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 window_update( ConstructSpdyWindowUpdate(1, kDeltaWindowSize)); scoped_ptr reply(ConstructSpdyPostSynReply(NULL, 0)); diff --git a/net/spdy/spdy_test_util.cc b/net/spdy/spdy_test_util.cc index 5d65b4a..4d30d0c 100644 --- a/net/spdy/spdy_test_util.cc +++ b/net/spdy/spdy_test_util.cc @@ -385,12 +385,10 @@ 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(int64 content_length, - const char* const extra_headers[], +spdy::SpdyFrame* ConstructSpdyPost(const char* const extra_headers[], int extra_header_count) { const SpdyHeaderInfo kSynStartHeader = { spdy::SYN_STREAM, // Kind = Syn @@ -404,8 +402,7 @@ spdy::SpdyFrame* ConstructSpdyPost(int64 content_length, 0, // Length spdy::DATA_FLAG_NONE // Data Flags }; - std::string length_str = Int64ToString(content_length); - const char* post_headers[] = { + static const char* const kStandardGetHeaders[] = { "method", "POST", "url", @@ -415,16 +412,14 @@ spdy::SpdyFrame* ConstructSpdyPost(int64 content_length, "scheme", "http", "version", - "HTTP/1.1", - "content-length", - length_str.c_str() + "HTTP/1.1" }; return ConstructSpdyPacket( kSynStartHeader, extra_headers, extra_header_count, - post_headers, - arraysize(post_headers) / 2); + kStandardGetHeaders, + arraysize(kStandardGetHeaders) / 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 0282763..4114fe2 100644 --- a/net/spdy/spdy_test_util.h +++ b/net/spdy/spdy_test_util.h @@ -181,8 +181,7 @@ 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(int64 content_length, - const char* const extra_headers[], +spdy::SpdyFrame* ConstructSpdyPost(const char* const extra_headers[], int extra_header_count); // Constructs a standard SPDY SYN_REPLY packet to match the SPDY POST. -- cgit v1.1