summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--net/http/http_network_transaction_spdy2_unittest.cc89
-rw-r--r--net/http/http_network_transaction_spdy3_unittest.cc87
-rw-r--r--net/http/http_response_body_drainer.cc6
-rw-r--r--net/http/http_response_body_drainer_unittest.cc85
4 files changed, 244 insertions, 23 deletions
diff --git a/net/http/http_network_transaction_spdy2_unittest.cc b/net/http/http_network_transaction_spdy2_unittest.cc
index 400c205..bc761f9 100644
--- a/net/http/http_network_transaction_spdy2_unittest.cc
+++ b/net/http/http_network_transaction_spdy2_unittest.cc
@@ -1191,6 +1191,89 @@ TEST_F(HttpNetworkTransactionSpdy2Test, NonKeepAliveConnectionEOF) {
EXPECT_EQ(ERR_EMPTY_RESPONSE, out.rv);
}
+// Next 2 cases (KeepAliveEarlyClose and KeepAliveEarlyClose2) are regression
+// tests. There was a bug causing HttpNetworkTransaction to hang in the
+// destructor in such situations.
+// See http://crbug.com/154712 and http://crbug.com/156609.
+TEST_F(HttpNetworkTransactionSpdy2Test, KeepAliveEarlyClose) {
+ HttpRequestInfo request;
+ request.method = "GET";
+ request.url = GURL("http://www.google.com/");
+ request.load_flags = 0;
+
+ SessionDependencies session_deps;
+ scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+ scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session));
+
+ MockRead data_reads[] = {
+ MockRead("HTTP/1.0 200 OK\r\n"),
+ MockRead("Connection: keep-alive\r\n"),
+ MockRead("Content-Length: 100\r\n\r\n"),
+ MockRead("hello"),
+ MockRead(SYNCHRONOUS, 0),
+ };
+ StaticSocketDataProvider data(data_reads, arraysize(data_reads), NULL, 0);
+ session_deps.socket_factory.AddSocketDataProvider(&data);
+
+ TestCompletionCallback callback;
+
+ int rv = trans->Start(&request, callback.callback(), BoundNetLog());
+ EXPECT_EQ(ERR_IO_PENDING, rv);
+
+ rv = callback.WaitForResult();
+ EXPECT_EQ(OK, rv);
+
+ scoped_refptr<IOBufferWithSize> io_buf(new IOBufferWithSize(100));
+ rv = trans->Read(io_buf, io_buf->size(), callback.callback());
+ if (rv == ERR_IO_PENDING)
+ rv = callback.WaitForResult();
+ EXPECT_EQ(5, rv);
+ rv = trans->Read(io_buf, io_buf->size(), callback.callback());
+ EXPECT_EQ(ERR_CONTENT_LENGTH_MISMATCH, rv);
+
+ trans.reset();
+ MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(0, GetIdleSocketCountInTransportSocketPool(session.get()));
+}
+
+TEST_F(HttpNetworkTransactionSpdy2Test, KeepAliveEarlyClose2) {
+ HttpRequestInfo request;
+ request.method = "GET";
+ request.url = GURL("http://www.google.com/");
+ request.load_flags = 0;
+
+ SessionDependencies session_deps;
+ scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+ scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session));
+
+ MockRead data_reads[] = {
+ MockRead("HTTP/1.0 200 OK\r\n"),
+ MockRead("Connection: keep-alive\r\n"),
+ MockRead("Content-Length: 100\r\n\r\n"),
+ MockRead(SYNCHRONOUS, 0),
+ };
+ StaticSocketDataProvider data(data_reads, arraysize(data_reads), NULL, 0);
+ session_deps.socket_factory.AddSocketDataProvider(&data);
+
+ TestCompletionCallback callback;
+
+ int rv = trans->Start(&request, callback.callback(), BoundNetLog());
+ EXPECT_EQ(ERR_IO_PENDING, rv);
+
+ rv = callback.WaitForResult();
+ EXPECT_EQ(OK, rv);
+
+ scoped_refptr<IOBufferWithSize> io_buf(new IOBufferWithSize(100));
+ rv = trans->Read(io_buf, io_buf->size(), callback.callback());
+ if (rv == ERR_IO_PENDING)
+ rv = callback.WaitForResult();
+ EXPECT_EQ(ERR_CONTENT_LENGTH_MISMATCH, rv);
+
+ trans.reset();
+ MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(0, GetIdleSocketCountInTransportSocketPool(session.get()));
+}
+
// Test that we correctly reuse a keep-alive connection after not explicitly
// reading the body.
TEST_F(HttpNetworkTransactionSpdy2Test, KeepAliveAfterUnreadBody) {
@@ -7224,7 +7307,8 @@ TEST_F(HttpNetworkTransactionSpdy2Test, UseAlternateProtocolForNpnSpdy) {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
- MockRead(ASYNC, OK),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
+ MockRead(ASYNC, OK)
};
StaticSocketDataProvider first_transaction(
@@ -7312,6 +7396,7 @@ TEST_F(HttpNetworkTransactionSpdy2Test, AlternateProtocolWithSpdyLateBinding) {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(ASYNC, OK),
};
@@ -7427,6 +7512,7 @@ TEST_F(HttpNetworkTransactionSpdy2Test, StallAlternateProtocolForNpnSpdy) {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(ASYNC, OK),
};
@@ -7560,6 +7646,7 @@ TEST_F(HttpNetworkTransactionSpdy2Test,
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(ASYNC, OK),
};
diff --git a/net/http/http_network_transaction_spdy3_unittest.cc b/net/http/http_network_transaction_spdy3_unittest.cc
index 5875776..53c58d0 100644
--- a/net/http/http_network_transaction_spdy3_unittest.cc
+++ b/net/http/http_network_transaction_spdy3_unittest.cc
@@ -1191,6 +1191,89 @@ TEST_F(HttpNetworkTransactionSpdy3Test, NonKeepAliveConnectionEOF) {
EXPECT_EQ(ERR_EMPTY_RESPONSE, out.rv);
}
+// Next 2 cases (KeepAliveEarlyClose and KeepAliveEarlyClose2) are regression
+// tests. There was a bug causing HttpNetworkTransaction to hang in the
+// destructor in such situations.
+// See http://crbug.com/154712 and http://crbug.com/156609.
+TEST_F(HttpNetworkTransactionSpdy3Test, KeepAliveEarlyClose) {
+ HttpRequestInfo request;
+ request.method = "GET";
+ request.url = GURL("http://www.google.com/");
+ request.load_flags = 0;
+
+ SessionDependencies session_deps;
+ scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+ scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session));
+
+ MockRead data_reads[] = {
+ MockRead("HTTP/1.0 200 OK\r\n"),
+ MockRead("Connection: keep-alive\r\n"),
+ MockRead("Content-Length: 100\r\n\r\n"),
+ MockRead("hello"),
+ MockRead(SYNCHRONOUS, 0),
+ };
+ StaticSocketDataProvider data(data_reads, arraysize(data_reads), NULL, 0);
+ session_deps.socket_factory.AddSocketDataProvider(&data);
+
+ TestCompletionCallback callback;
+
+ int rv = trans->Start(&request, callback.callback(), BoundNetLog());
+ EXPECT_EQ(ERR_IO_PENDING, rv);
+
+ rv = callback.WaitForResult();
+ EXPECT_EQ(OK, rv);
+
+ scoped_refptr<IOBufferWithSize> io_buf(new IOBufferWithSize(100));
+ rv = trans->Read(io_buf, io_buf->size(), callback.callback());
+ if (rv == ERR_IO_PENDING)
+ rv = callback.WaitForResult();
+ EXPECT_EQ(5, rv);
+ rv = trans->Read(io_buf, io_buf->size(), callback.callback());
+ EXPECT_EQ(ERR_CONTENT_LENGTH_MISMATCH, rv);
+
+ trans.reset();
+ MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(0, GetIdleSocketCountInTransportSocketPool(session.get()));
+}
+
+TEST_F(HttpNetworkTransactionSpdy3Test, KeepAliveEarlyClose2) {
+ HttpRequestInfo request;
+ request.method = "GET";
+ request.url = GURL("http://www.google.com/");
+ request.load_flags = 0;
+
+ SessionDependencies session_deps;
+ scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+ scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session));
+
+ MockRead data_reads[] = {
+ MockRead("HTTP/1.0 200 OK\r\n"),
+ MockRead("Connection: keep-alive\r\n"),
+ MockRead("Content-Length: 100\r\n\r\n"),
+ MockRead(SYNCHRONOUS, 0),
+ };
+ StaticSocketDataProvider data(data_reads, arraysize(data_reads), NULL, 0);
+ session_deps.socket_factory.AddSocketDataProvider(&data);
+
+ TestCompletionCallback callback;
+
+ int rv = trans->Start(&request, callback.callback(), BoundNetLog());
+ EXPECT_EQ(ERR_IO_PENDING, rv);
+
+ rv = callback.WaitForResult();
+ EXPECT_EQ(OK, rv);
+
+ scoped_refptr<IOBufferWithSize> io_buf(new IOBufferWithSize(100));
+ rv = trans->Read(io_buf, io_buf->size(), callback.callback());
+ if (rv == ERR_IO_PENDING)
+ rv = callback.WaitForResult();
+ EXPECT_EQ(ERR_CONTENT_LENGTH_MISMATCH, rv);
+
+ trans.reset();
+ MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(0, GetIdleSocketCountInTransportSocketPool(session.get()));
+}
+
// Test that we correctly reuse a keep-alive connection after not explicitly
// reading the body.
TEST_F(HttpNetworkTransactionSpdy3Test, KeepAliveAfterUnreadBody) {
@@ -7223,6 +7306,7 @@ TEST_F(HttpNetworkTransactionSpdy3Test, UseAlternateProtocolForNpnSpdy) {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(ASYNC, OK),
};
@@ -7311,6 +7395,7 @@ TEST_F(HttpNetworkTransactionSpdy3Test, AlternateProtocolWithSpdyLateBinding) {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(ASYNC, OK),
};
@@ -7426,6 +7511,7 @@ TEST_F(HttpNetworkTransactionSpdy3Test, StallAlternateProtocolForNpnSpdy) {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(ASYNC, OK),
};
@@ -7559,6 +7645,7 @@ TEST_F(HttpNetworkTransactionSpdy3Test,
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead(kAlternateProtocolHttpHeader),
MockRead("hello world"),
+ MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
MockRead(ASYNC, OK),
};
diff --git a/net/http/http_response_body_drainer.cc b/net/http/http_response_body_drainer.cc
index 903f501..a6c9686 100644
--- a/net/http/http_response_body_drainer.cc
+++ b/net/http/http_response_body_drainer.cc
@@ -98,9 +98,6 @@ int HttpResponseBodyDrainer::DoDrainResponseBodyComplete(int result) {
if (result < 0)
return result;
- if (result == 0)
- return ERR_CONNECTION_CLOSED;
-
total_read_ += result;
if (stream_->IsResponseBodyComplete())
return OK;
@@ -109,6 +106,9 @@ int HttpResponseBodyDrainer::DoDrainResponseBodyComplete(int result) {
if (total_read_ >= kDrainBodyBufferSize)
return ERR_RESPONSE_BODY_TOO_BIG_TO_DRAIN;
+ if (result == 0)
+ return ERR_CONNECTION_CLOSED;
+
next_state_ = STATE_DRAIN_RESPONSE_BODY;
return OK;
}
diff --git a/net/http/http_response_body_drainer_unittest.cc b/net/http/http_response_body_drainer_unittest.cc
index 02b89c3..45d647f 100644
--- a/net/http/http_response_body_drainer_unittest.cc
+++ b/net/http/http_response_body_drainer_unittest.cc
@@ -37,7 +37,7 @@ class CloseResultWaiter {
waiting_for_result_(false) {}
int WaitForResult() {
- DCHECK(!waiting_for_result_);
+ CHECK(!waiting_for_result_);
while (!have_result_) {
waiting_for_result_ = true;
MessageLoop::current()->Run();
@@ -65,9 +65,12 @@ class MockHttpStream : public HttpStream {
public:
MockHttpStream(CloseResultWaiter* result_waiter)
: result_waiter_(result_waiter),
+ buf_len_(0),
closed_(false),
stall_reads_forever_(false),
num_chunks_(0),
+ is_sync_(false),
+ is_last_chunk_zero_size_(false),
is_complete_(false),
ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {}
virtual ~MockHttpStream() {}
@@ -107,7 +110,7 @@ class MockHttpStream : public HttpStream {
virtual int ReadResponseBody(IOBuffer* buf, int buf_len,
const CompletionCallback& callback) OVERRIDE;
virtual void Close(bool not_reusable) OVERRIDE {
- DCHECK(!closed_);
+ CHECK(!closed_);
closed_ = true;
result_waiter_->set_result(not_reusable);
}
@@ -125,11 +128,16 @@ class MockHttpStream : public HttpStream {
virtual void Drain(HttpNetworkSession*) OVERRIDE {}
// Methods to tweak/observer mock behavior:
- void StallReadsForever() { stall_reads_forever_ = true; }
+ void set_stall_reads_forever() { stall_reads_forever_ = true; }
void set_num_chunks(int num_chunks) { num_chunks_ = num_chunks; }
+ void set_sync() { is_sync_ = true; }
+
+ void set_is_last_chunk_zero_size() { is_last_chunk_zero_size_ = true; }
+
private:
+ int ReadResponseBodyImpl(IOBuffer* buf, int buf_len);
void CompleteRead();
bool closed() const { return closed_; }
@@ -137,34 +145,50 @@ class MockHttpStream : public HttpStream {
CloseResultWaiter* const result_waiter_;
scoped_refptr<IOBuffer> user_buf_;
CompletionCallback callback_;
+ int buf_len_;
bool closed_;
bool stall_reads_forever_;
int num_chunks_;
+ bool is_sync_;
+ bool is_last_chunk_zero_size_;
bool is_complete_;
base::WeakPtrFactory<MockHttpStream> weak_factory_;
};
-int MockHttpStream::ReadResponseBody(
- IOBuffer* buf, int buf_len, const CompletionCallback& callback) {
- DCHECK(!callback.is_null());
- DCHECK(callback_.is_null());
- DCHECK(buf);
+int MockHttpStream::ReadResponseBody(IOBuffer* buf,
+ int buf_len,
+ const CompletionCallback& callback) {
+ CHECK(!callback.is_null());
+ CHECK(callback_.is_null());
+ CHECK(buf);
if (stall_reads_forever_)
return ERR_IO_PENDING;
- if (num_chunks_ == 0)
+ if (is_complete_)
return ERR_UNEXPECTED;
- if (buf_len > kMagicChunkSize && num_chunks_ > 1) {
+ if (!is_sync_) {
user_buf_ = buf;
+ buf_len_ = buf_len;
callback_ = callback;
MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&MockHttpStream::CompleteRead, weak_factory_.GetWeakPtr()));
return ERR_IO_PENDING;
+ } else {
+ return ReadResponseBodyImpl(buf, buf_len);
}
+}
+int MockHttpStream::ReadResponseBodyImpl(IOBuffer* buf, int buf_len) {
+ if (is_last_chunk_zero_size_ && num_chunks_ == 1) {
+ buf_len = 0;
+ } else {
+ if (buf_len > kMagicChunkSize)
+ buf_len = kMagicChunkSize;
+ std::memset(buf->data(), 1, buf_len);
+ }
num_chunks_--;
if (!num_chunks_)
is_complete_ = true;
@@ -173,14 +197,11 @@ int MockHttpStream::ReadResponseBody(
}
void MockHttpStream::CompleteRead() {
- CompletionCallback callback = callback_;
- std::memset(user_buf_->data(), 1, kMagicChunkSize);
+ int result = ReadResponseBodyImpl(user_buf_, buf_len_);
user_buf_ = NULL;
+ CompletionCallback callback = callback_;
callback_.Reset();
- num_chunks_--;
- if (!num_chunks_)
- is_complete_ = true;
- callback.Run(kMagicChunkSize);
+ callback.Run(result);
}
class HttpResponseBodyDrainerTest : public testing::Test {
@@ -212,8 +233,16 @@ class HttpResponseBodyDrainerTest : public testing::Test {
HttpResponseBodyDrainer* const drainer_; // Deletes itself.
};
-TEST_F(HttpResponseBodyDrainerTest, DrainBodySyncOK) {
+TEST_F(HttpResponseBodyDrainerTest, DrainBodySyncSingleOK) {
mock_stream_->set_num_chunks(1);
+ mock_stream_->set_sync();
+ drainer_->Start(session_);
+ EXPECT_FALSE(result_waiter_.WaitForResult());
+}
+
+TEST_F(HttpResponseBodyDrainerTest, DrainBodySyncOK) {
+ mock_stream_->set_num_chunks(3);
+ mock_stream_->set_sync();
drainer_->Start(session_);
EXPECT_FALSE(result_waiter_.WaitForResult());
}
@@ -224,6 +253,24 @@ TEST_F(HttpResponseBodyDrainerTest, DrainBodyAsyncOK) {
EXPECT_FALSE(result_waiter_.WaitForResult());
}
+// Test the case when the final chunk is 0 bytes. This can happen when
+// the final 0-byte chunk of a chunk-encoded http response is read in a last
+// call to ReadResponseBody, after all data were returned from HttpStream.
+TEST_F(HttpResponseBodyDrainerTest, DrainBodyAsyncEmptyChunk) {
+ mock_stream_->set_num_chunks(4);
+ mock_stream_->set_is_last_chunk_zero_size();
+ drainer_->Start(session_);
+ EXPECT_FALSE(result_waiter_.WaitForResult());
+}
+
+TEST_F(HttpResponseBodyDrainerTest, DrainBodySyncEmptyChunk) {
+ mock_stream_->set_num_chunks(4);
+ mock_stream_->set_sync();
+ mock_stream_->set_is_last_chunk_zero_size();
+ drainer_->Start(session_);
+ EXPECT_FALSE(result_waiter_.WaitForResult());
+}
+
TEST_F(HttpResponseBodyDrainerTest, DrainBodySizeEqualsDrainBuffer) {
mock_stream_->set_num_chunks(
HttpResponseBodyDrainer::kDrainBodyBufferSize / kMagicChunkSize);
@@ -233,14 +280,14 @@ TEST_F(HttpResponseBodyDrainerTest, DrainBodySizeEqualsDrainBuffer) {
TEST_F(HttpResponseBodyDrainerTest, DrainBodyTimeOut) {
mock_stream_->set_num_chunks(2);
- mock_stream_->StallReadsForever();
+ mock_stream_->set_stall_reads_forever();
drainer_->Start(session_);
EXPECT_TRUE(result_waiter_.WaitForResult());
}
TEST_F(HttpResponseBodyDrainerTest, CancelledBySession) {
mock_stream_->set_num_chunks(2);
- mock_stream_->StallReadsForever();
+ mock_stream_->set_stall_reads_forever();
drainer_->Start(session_);
// HttpNetworkSession should delete |drainer_|.
}
generated by cgit v1.1 at 2025-02-15 04:53:15 +0000