Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(181)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: net/http/http_network_transaction_unittest.cc

Issue 1494813002: Fix HttpStreamParser::CanReuseConnection(). (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: add override Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | net/http/http_stream_parser.h » ('j') | net/socket/socket_test_util.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/http/http_network_transaction_unittest.cc
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 20ea0d411e55d66cfd97fa7c47d90991d64a1144..16e9ff7a546f0f1c4765e72ae9b25c35e7ea082d 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -1842,50 +1842,76 @@ TEST_P(HttpNetworkTransactionTest, KeepAliveAfterUnreadBody) {
session_deps_.net_log = &net_log;
scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+ const char* request_data =
+ "GET / HTTP/1.1\r\n"
+ "Host: www.foo.com\r\n"
+ "Connection: keep-alive\r\n\r\n";
+ MockWrite data_writes[] = {
+ MockWrite(ASYNC, 0, request_data), MockWrite(ASYNC, 2, request_data),
+ MockWrite(ASYNC, 4, request_data), MockWrite(ASYNC, 6, request_data),
+ MockWrite(ASYNC, 8, request_data), MockWrite(ASYNC, 10, request_data),
+ MockWrite(ASYNC, 12, request_data), MockWrite(ASYNC, 14, request_data),
+ MockWrite(ASYNC, 17, request_data), MockWrite(ASYNC, 20, request_data),
+ };
+
// Note that because all these reads happen in the same
// StaticSocketDataProvider, it shows that the same socket is being reused for
// all transactions.
- MockRead data1_reads[] = {
- MockRead("HTTP/1.1 204 No Content\r\n\r\n"),
- MockRead("HTTP/1.1 205 Reset Content\r\n\r\n"),
- MockRead("HTTP/1.1 304 Not Modified\r\n\r\n"),
- MockRead("HTTP/1.1 302 Found\r\n"
- "Content-Length: 0\r\n\r\n"),
- MockRead("HTTP/1.1 302 Found\r\n"
- "Content-Length: 5\r\n\r\n"
- "hello"),
- MockRead("HTTP/1.1 301 Moved Permanently\r\n"
- "Content-Length: 0\r\n\r\n"),
- MockRead("HTTP/1.1 301 Moved Permanently\r\n"
- "Content-Length: 5\r\n\r\n"
- "hello"),
- MockRead("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"),
- MockRead("hello"),
- };
- StaticSocketDataProvider data1(data1_reads, arraysize(data1_reads), NULL, 0);
- session_deps_.socket_factory->AddSocketDataProvider(&data1);
-
- MockRead data2_reads[] = {
- MockRead(SYNCHRONOUS, ERR_UNEXPECTED), // Should not be reached.
+ MockRead data_reads[] = {
+ MockRead(ASYNC, 1, "HTTP/1.1 204 No Content\r\n\r\n"),
+ MockRead(ASYNC, 3, "HTTP/1.1 205 Reset Content\r\n\r\n"),
+ MockRead(ASYNC, 5, "HTTP/1.1 304 Not Modified\r\n\r\n"),
+ MockRead(ASYNC, 7,
+ "HTTP/1.1 302 Found\r\n"
+ "Content-Length: 0\r\n\r\n"),
+ MockRead(ASYNC, 9,
+ "HTTP/1.1 302 Found\r\n"
+ "Content-Length: 5\r\n\r\n"
+ "hello"),
+ MockRead(ASYNC, 11,
+ "HTTP/1.1 301 Moved Permanently\r\n"
+ "Content-Length: 0\r\n\r\n"),
+ MockRead(ASYNC, 13,
+ "HTTP/1.1 301 Moved Permanently\r\n"
+ "Content-Length: 5\r\n\r\n"
+ "hello"),
+
+ // In the next two rounds, IsConnectedAndIdle returns false, due to
+ // the set_busy_before_sync_reads(true) call, while the
+ // HttpNetworkTransaction is being shut down, but the socket is still
+ // reuseable. See http://crbug.com/544255.
+ MockRead(ASYNC, 15,
+ "HTTP/1.1 200 Hunky-Dory\r\n"
+ "Content-Length: 5\r\n\r\n"),
+ MockRead(SYNCHRONOUS, 16, "hello"),
+
+ MockRead(ASYNC, 18,
+ "HTTP/1.1 200 Hunky-Dory\r\n"
+ "Content-Length: 5\r\n\r\n"
+ "he"),
+ MockRead(SYNCHRONOUS, 19, "llo"),
+
+ // The body of the final request is actually read.
+ MockRead(ASYNC, 21, "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"),
+ MockRead(ASYNC, 22, "hello"),
};
- StaticSocketDataProvider data2(data2_reads, arraysize(data2_reads), NULL, 0);
- session_deps_.socket_factory->AddSocketDataProvider(&data2);
+ SequencedSocketData data(data_reads, arraysize(data_reads), data_writes,
+ arraysize(data_writes));
+ data.set_busy_before_sync_reads(true);
+ session_deps_.socket_factory->AddSocketDataProvider(&data);
- const int kNumUnreadBodies = arraysize(data1_reads) - 2;
+ const int kNumUnreadBodies = arraysize(data_writes) - 1;
std::string response_lines[kNumUnreadBodies];
uint32 first_socket_log_id = NetLog::Source::kInvalidId;
- for (size_t i = 0; i < arraysize(data1_reads) - 2; ++i) {
+ for (size_t i = 0; i < kNumUnreadBodies; ++i) {
TestCompletionCallback callback;
scoped_ptr<HttpTransaction> trans(
new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
int rv = trans->Start(&request, callback.callback(), BoundNetLog());
- EXPECT_EQ(ERR_IO_PENDING, rv);
-
- rv = callback.WaitForResult();
- EXPECT_EQ(OK, rv);
+ EXPECT_EQ(OK, callback.GetResult(rv));
LoadTimingInfo load_timing_info;
EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info));
@@ -1898,22 +1924,27 @@ TEST_P(HttpNetworkTransactionTest, KeepAliveAfterUnreadBody) {
}
const HttpResponseInfo* response = trans->GetResponseInfo();
- ASSERT_TRUE(response != NULL);
+ ASSERT_TRUE(response);
- ASSERT_TRUE(response->headers.get() != NULL);
+ ASSERT_TRUE(response->headers);
response_lines[i] = response->headers->GetStatusLine();
- // We intentionally don't read the response bodies.
+ // Delete the transaction without reading the response bodies. Then spin
+ // the message loop, so the response bodies are drained.
+ trans.reset();
+ base::RunLoop().RunUntilIdle();
}
const char* const kStatusLines[] = {
- "HTTP/1.1 204 No Content",
- "HTTP/1.1 205 Reset Content",
- "HTTP/1.1 304 Not Modified",
- "HTTP/1.1 302 Found",
- "HTTP/1.1 302 Found",
- "HTTP/1.1 301 Moved Permanently",
- "HTTP/1.1 301 Moved Permanently",
+ "HTTP/1.1 204 No Content",
+ "HTTP/1.1 205 Reset Content",
+ "HTTP/1.1 304 Not Modified",
+ "HTTP/1.1 302 Found",
+ "HTTP/1.1 302 Found",
+ "HTTP/1.1 301 Moved Permanently",
+ "HTTP/1.1 301 Moved Permanently",
+ "HTTP/1.1 200 Hunky-Dory",
+ "HTTP/1.1 200 Hunky-Dory",
};
static_assert(kNumUnreadBodies == arraysize(kStatusLines),
@@ -1926,12 +1957,10 @@ TEST_P(HttpNetworkTransactionTest, KeepAliveAfterUnreadBody) {
scoped_ptr<HttpTransaction> trans(
new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
int rv = trans->Start(&request, callback.callback(), BoundNetLog());
- EXPECT_EQ(ERR_IO_PENDING, rv);
- rv = callback.WaitForResult();
- EXPECT_EQ(OK, rv);
+ EXPECT_EQ(OK, callback.GetResult(rv));
const HttpResponseInfo* response = trans->GetResponseInfo();
- ASSERT_TRUE(response != NULL);
- ASSERT_TRUE(response->headers.get() != NULL);
+ ASSERT_TRUE(response);
+ ASSERT_TRUE(response->headers);
EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
std::string response_data;
rv = ReadTransaction(trans.get(), &response_data);
@@ -2214,106 +2243,96 @@ TEST_P(HttpNetworkTransactionTest, DoNotSendAuth) {
// Test the request-challenge-retry sequence for basic auth, over a keep-alive
// connection.
TEST_P(HttpNetworkTransactionTest, BasicAuthKeepAlive) {
- HttpRequestInfo request;
- request.method = "GET";
- request.url = GURL("http://www.example.org/");
- request.load_flags = 0;
-
- TestNetLog log;
- session_deps_.net_log = &log;
- scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
-
- MockWrite data_writes1[] = {
- MockWrite(
- "GET / HTTP/1.1\r\n"
- "Host: www.example.org\r\n"
- "Connection: keep-alive\r\n\r\n"),
-
- // After calling trans->RestartWithAuth(), this is the request we should
- // be issuing -- the final header line contains the credentials.
- MockWrite(
- "GET / HTTP/1.1\r\n"
- "Host: www.example.org\r\n"
- "Connection: keep-alive\r\n"
- "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"),
- };
-
- MockRead data_reads1[] = {
- MockRead("HTTP/1.1 401 Unauthorized\r\n"),
- MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
- MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),
- MockRead("Content-Length: 14\r\n\r\n"),
- MockRead("Unauthorized\r\n"),
+ // On the second pass, the body read of the auth challenge is synchronous, so
+ // IsConnectedAndIdle returns false. The socket should still be drained and
+ // reused. See http://crbug.com/544255.
+ for (int i = 0; i < 2; ++i) {
+ HttpRequestInfo request;
+ request.method = "GET";
+ request.url = GURL("http://www.example.org/");
+ request.load_flags = 0;
- // Lastly, the server responds with the actual content.
- MockRead("HTTP/1.1 200 OK\r\n"),
- MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),
- MockRead("Content-Length: 5\r\n\r\n"),
- MockRead("Hello"),
- };
+ TestNetLog log;
+ session_deps_.net_log = &log;
+ scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
- // If there is a regression where we disconnect a Keep-Alive
- // connection during an auth roundtrip, we'll end up reading this.
- MockRead data_reads2[] = {
- MockRead(SYNCHRONOUS, ERR_FAILED),
- };
+ MockWrite data_writes[] = {
+ MockWrite(ASYNC, 0,
+ "GET / HTTP/1.1\r\n"
+ "Host: www.example.org\r\n"
+ "Connection: keep-alive\r\n\r\n"),
- StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
- data_writes1, arraysize(data_writes1));
- StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
- NULL, 0);
- session_deps_.socket_factory->AddSocketDataProvider(&data1);
- session_deps_.socket_factory->AddSocketDataProvider(&data2);
+ // After calling trans->RestartWithAuth(), this is the request we should
+ // be issuing -- the final header line contains the credentials.
+ MockWrite(ASYNC, 6,
+ "GET / HTTP/1.1\r\n"
+ "Host: www.example.org\r\n"
+ "Connection: keep-alive\r\n"
+ "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"),
+ };
- TestCompletionCallback callback1;
+ MockRead data_reads[] = {
+ MockRead(ASYNC, 1, "HTTP/1.1 401 Unauthorized\r\n"),
+ MockRead(ASYNC, 2, "WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
+ MockRead(ASYNC, 3, "Content-Type: text/html; charset=iso-8859-1\r\n"),
+ MockRead(ASYNC, 4, "Content-Length: 14\r\n\r\n"),
+ MockRead(i == 0 ? ASYNC : SYNCHRONOUS, 5, "Unauthorized\r\n"),
+
+ // Lastly, the server responds with the actual content.
+ MockRead(ASYNC, 7, "HTTP/1.1 200 OK\r\n"),
+ MockRead(ASYNC, 8, "Content-Type: text/html; charset=iso-8859-1\r\n"),
+ MockRead(ASYNC, 9, "Content-Length: 5\r\n\r\n"),
+ MockRead(ASYNC, 10, "Hello"),
+ };
- scoped_ptr<HttpTransaction> trans(
- new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
- int rv = trans->Start(&request, callback1.callback(), BoundNetLog());
- EXPECT_EQ(ERR_IO_PENDING, rv);
+ SequencedSocketData data(data_reads, arraysize(data_reads), data_writes,
+ arraysize(data_writes));
+ data.set_busy_before_sync_reads(true);
+ session_deps_.socket_factory->AddSocketDataProvider(&data);
- rv = callback1.WaitForResult();
- EXPECT_EQ(OK, rv);
+ TestCompletionCallback callback1;
- LoadTimingInfo load_timing_info1;
- EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info1));
- TestLoadTimingNotReused(load_timing_info1, CONNECT_TIMING_HAS_DNS_TIMES);
+ scoped_ptr<HttpTransaction> trans(
+ new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+ int rv = trans->Start(&request, callback1.callback(), BoundNetLog());
+ ASSERT_EQ(OK, callback1.GetResult(rv));
- const HttpResponseInfo* response = trans->GetResponseInfo();
- ASSERT_TRUE(response != NULL);
- EXPECT_TRUE(CheckBasicServerAuth(response->auth_challenge.get()));
+ LoadTimingInfo load_timing_info1;
+ EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info1));
+ TestLoadTimingNotReused(load_timing_info1, CONNECT_TIMING_HAS_DNS_TIMES);
- TestCompletionCallback callback2;
+ const HttpResponseInfo* response = trans->GetResponseInfo();
+ ASSERT_TRUE(response);
+ EXPECT_TRUE(CheckBasicServerAuth(response->auth_challenge.get()));
- rv = trans->RestartWithAuth(
- AuthCredentials(kFoo, kBar), callback2.callback());
- EXPECT_EQ(ERR_IO_PENDING, rv);
+ TestCompletionCallback callback2;
- rv = callback2.WaitForResult();
- EXPECT_EQ(OK, rv);
+ rv = trans->RestartWithAuth(AuthCredentials(kFoo, kBar),
+ callback2.callback());
+ ASSERT_EQ(OK, callback2.GetResult(rv));
- LoadTimingInfo load_timing_info2;
- EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info2));
- TestLoadTimingReused(load_timing_info2);
- // The load timing after restart should have the same socket ID, and times
- // those of the first load timing.
- EXPECT_LE(load_timing_info1.receive_headers_end,
- load_timing_info2.send_start);
- EXPECT_EQ(load_timing_info1.socket_log_id, load_timing_info2.socket_log_id);
+ LoadTimingInfo load_timing_info2;
+ EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info2));
+ TestLoadTimingReused(load_timing_info2);
+ // The load timing after restart should have the same socket ID, and times
+ // those of the first load timing.
+ EXPECT_LE(load_timing_info1.receive_headers_end,
+ load_timing_info2.send_start);
+ EXPECT_EQ(load_timing_info1.socket_log_id, load_timing_info2.socket_log_id);
- response = trans->GetResponseInfo();
- ASSERT_TRUE(response != NULL);
- EXPECT_TRUE(response->auth_challenge.get() == NULL);
- EXPECT_EQ(5, response->headers->GetContentLength());
+ response = trans->GetResponseInfo();
+ ASSERT_TRUE(response);
+ EXPECT_FALSE(response->auth_challenge);
+ EXPECT_EQ(5, response->headers->GetContentLength());
- std::string response_data;
- rv = ReadTransaction(trans.get(), &response_data);
- EXPECT_EQ(OK, rv);
+ std::string response_data;
+ EXPECT_EQ(OK, ReadTransaction(trans.get(), &response_data));
- int64_t writes_size1 = CountWriteBytes(data_writes1, arraysize(data_writes1));
- EXPECT_EQ(writes_size1, trans->GetTotalSentBytes());
- int64_t reads_size1 = CountReadBytes(data_reads1, arraysize(data_reads1));
- EXPECT_EQ(reads_size1, trans->GetTotalReceivedBytes());
+ int64_t writes_size = CountWriteBytes(data_writes, arraysize(data_writes));
+ EXPECT_EQ(writes_size, trans->GetTotalSentBytes());
+ int64_t reads_size = CountReadBytes(data_reads, arraysize(data_reads));
+ EXPECT_EQ(reads_size, trans->GetTotalReceivedBytes());
+ }
}
// Test the request-challenge-retry sequence for basic auth, over a keep-alive
@@ -5881,11 +5900,8 @@ TEST_P(HttpNetworkTransactionTest, RecycleDeadSSLSocket) {
};
MockRead data_reads[] = {
- MockRead("HTTP/1.1 200 OK\r\n"),
- MockRead("Content-Length: 11\r\n\r\n"),
- MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
- MockRead("hello world"),
- MockRead(ASYNC, 0, 0) // EOF
+ MockRead("HTTP/1.1 200 OK\r\n"), MockRead("Content-Length: 11\r\n\r\n"),
+ MockRead("hello world"), MockRead(ASYNC, ERR_CONNECTION_CLOSED) // EOF
};
SSLSocketDataProvider ssl(ASYNC, OK);
@@ -5972,9 +5988,6 @@ TEST_P(HttpNetworkTransactionTest, RecycleSocketAfterZeroContentLength) {
scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
- scoped_ptr<HttpTransaction> trans(
- new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
-
MockRead data_reads[] = {
MockRead("HTTP/1.1 204 No Content\r\n"
"Content-Length: 0\r\n"
@@ -5986,6 +5999,11 @@ TEST_P(HttpNetworkTransactionTest, RecycleSocketAfterZeroContentLength) {
StaticSocketDataProvider data(data_reads, arraysize(data_reads), NULL, 0);
session_deps_.socket_factory->AddSocketDataProvider(&data);
+ // Transaction must be created after the MockReads, so it's destroyed before
+ // them.
+ scoped_ptr<HttpTransaction> trans(
+ new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+
TestCompletionCallback callback;
int rv = trans->Start(&request, callback.callback(), BoundNetLog());
@@ -11155,7 +11173,6 @@ TEST_P(HttpNetworkTransactionTest, GenerateAuthToken) {
request.load_flags = 0;
scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
- HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
SSLSocketDataProvider ssl_socket_data_provider(SYNCHRONOUS, OK);
@@ -11197,6 +11214,10 @@ TEST_P(HttpNetworkTransactionTest, GenerateAuthToken) {
data_providers.back());
}
+ // Transaction must be created after DataProviders, so it's destroyed before
+ // they are as well.
+ HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
+
for (int round = 0; round < test_config.num_auth_rounds; ++round) {
const TestRound& read_write_round = test_config.rounds[round];
// Start or restart the transaction.
« no previous file with comments | « no previous file | net/http/http_stream_parser.h » ('j') | net/socket/socket_test_util.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698