diff options
author | bnc <bnc@chromium.org> | 2015-08-26 12:34:56 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-08-26 19:35:46 +0000 |
commit | 54ec34b7c174848672fc3ea08aba7289630df4a3 (patch) | |
tree | 3ad888f8d407f512616a1ff529c49c6fb4ab2e15 /net/http | |
parent | 2e31d26ba3287cba63824b0c43b0494b3afc6b61 (diff) | |
download | chromium_src-54ec34b7c174848672fc3ea08aba7289630df4a3.zip chromium_src-54ec34b7c174848672fc3ea08aba7289630df4a3.tar.gz chromium_src-54ec34b7c174848672fc3ea08aba7289630df4a3.tar.bz2 |
Change |use_alternative_service| not to disable Alternate Protocols.
Currently, HttpNetworkSession::Params::use_alternative_service controls both
Alternate Protocols and Alternative Services. However, Alternate Protocols is
fully rolled out, therefore there is not need to disable it.
This CL changes |use_alternative_service| to
* disable parsing Alt-Svc headers (but not Alternate-Protocol ones);
* disable following saved alternative service entries that point to different
hosts (but not ones that point to the same host).
The latter is necessary because a browser session might have this flag enabled
and might save alternative services to disk, which we do not really want to
follow in the next browser session if the flag is disabled. (Ones discovered
through Alt-Svc pointing to the same host will still be respected.)
BUG=524141
Review URL: https://codereview.chromium.org/1310583002
Cr-Commit-Position: refs/heads/master@{#345640}
Diffstat (limited to 'net/http')
-rw-r--r-- | net/http/http_network_transaction.cc | 3 | ||||
-rw-r--r-- | net/http/http_network_transaction_unittest.cc | 108 | ||||
-rw-r--r-- | net/http/http_stream_factory_impl.cc | 9 |
3 files changed, 115 insertions, 5 deletions
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 135d0e4..0733fdf 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -70,7 +70,8 @@ namespace { void ProcessAlternativeServices(HttpNetworkSession* session, const HttpResponseHeaders& headers, const HostPortPair& http_host_port_pair) { - if (headers.HasHeader(kAlternativeServiceHeader)) { + if (session->params().use_alternative_services && + headers.HasHeader(kAlternativeServiceHeader)) { std::string alternative_service_str; headers.GetNormalizedHeader(kAlternativeServiceHeader, &alternative_service_str); diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index f263777..03bf557 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -8781,6 +8781,66 @@ TEST_P(HttpNetworkTransactionTest, HonorAlternativeServiceHeader) { EXPECT_EQ(443, alternative_service_vector[0].port); } +// Alternative Service headers must be ignored when |use_alternative_services| +// is false. +TEST_P(HttpNetworkTransactionTest, DoNotHonorAlternativeServiceHeader) { + session_deps_.next_protos = SpdyNextProtos(); + session_deps_.use_alternative_services = false; + + std::string alternative_service_http_header = + GetAlternativeServiceHttpHeader(); + + MockRead data_reads[] = { + MockRead("HTTP/1.1 200 OK\r\n"), + MockRead(alternative_service_http_header.c_str()), + MockRead("\r\n"), + MockRead("hello world"), + MockRead(SYNCHRONOUS, OK), + }; + + HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("http://www.example.org/"); + request.load_flags = 0; + + StaticSocketDataProvider data(data_reads, arraysize(data_reads), nullptr, 0); + + session_deps_.socket_factory->AddSocketDataProvider(&data); + + TestCompletionCallback callback; + + scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_)); + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get())); + + int rv = trans->Start(&request, callback.callback(), BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + + HostPortPair http_host_port_pair("www.example.org", 80); + HttpServerProperties& http_server_properties = + *session->http_server_properties(); + AlternativeServiceVector alternative_service_vector = + http_server_properties.GetAlternativeServices(http_host_port_pair); + EXPECT_TRUE(alternative_service_vector.empty()); + + EXPECT_EQ(OK, callback.WaitForResult()); + + const HttpResponseInfo* response = trans->GetResponseInfo(); + ASSERT_TRUE(response != nullptr); + ASSERT_TRUE(response->headers.get() != nullptr); + EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); + EXPECT_FALSE(response->was_fetched_via_spdy); + EXPECT_FALSE(response->was_npn_negotiated); + + std::string response_data; + ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data)); + EXPECT_EQ("hello world", response_data); + + alternative_service_vector = + http_server_properties.GetAlternativeServices(http_host_port_pair); + EXPECT_TRUE(alternative_service_vector.empty()); +} + TEST_P(HttpNetworkTransactionTest, HonorMultipleAlternativeServiceHeader) { session_deps_.next_protos = SpdyNextProtos(); session_deps_.use_alternative_services = true; @@ -8845,9 +8905,11 @@ TEST_P(HttpNetworkTransactionTest, HonorMultipleAlternativeServiceHeader) { EXPECT_EQ(1234, alternative_service_vector[1].port); } +// Alternate Protocol headers must be honored even if |use_alternative_services| +// is false. TEST_P(HttpNetworkTransactionTest, HonorAlternateProtocolHeader) { session_deps_.next_protos = SpdyNextProtos(); - session_deps_.use_alternative_services = true; + session_deps_.use_alternative_services = false; std::string alternate_protocol_http_header = GetAlternateProtocolHttpHeader(); @@ -9031,6 +9093,50 @@ TEST_P(HttpNetworkTransactionTest, AltSvcOverwritesAlternateProtocol) { EXPECT_EQ(443, alternative_service_vector[0].port); } +// When |use_alternative_services| is false, do not observe alternative service +// entries that point to a different host. +TEST_P(HttpNetworkTransactionTest, DisableAlternativeServiceToDifferentHost) { + session_deps_.use_alternative_services = false; + + HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("http://www.example.org/"); + request.load_flags = 0; + + MockConnect mock_connect(ASYNC, ERR_CONNECTION_REFUSED); + StaticSocketDataProvider first_data; + first_data.set_connect_data(mock_connect); + session_deps_.socket_factory->AddSocketDataProvider(&first_data); + + MockRead data_reads[] = { + MockRead("HTTP/1.1 200 OK\r\n\r\n"), MockRead("hello world"), + MockRead(ASYNC, OK), + }; + StaticSocketDataProvider second_data(data_reads, arraysize(data_reads), + nullptr, 0); + session_deps_.socket_factory->AddSocketDataProvider(&second_data); + + scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_)); + + base::WeakPtr<HttpServerProperties> http_server_properties = + session->http_server_properties(); + AlternativeService alternative_service( + AlternateProtocolFromNextProto(GetParam()), "different.example.org", 80); + base::Time expiration = base::Time::Now() + base::TimeDelta::FromDays(1); + http_server_properties->SetAlternativeService( + HostPortPair::FromURL(request.url), alternative_service, 1.0, expiration); + + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get())); + TestCompletionCallback callback; + + int rv = trans->Start(&request, callback.callback(), BoundNetLog()); + // The connetion to origin was refused, and the alternative service should not + // be used (even though mock data are there), therefore the request should + // fail. + EXPECT_EQ(ERR_CONNECTION_REFUSED, callback.GetResult(rv)); +} + TEST_P(HttpNetworkTransactionTest, MarkBrokenAlternateProtocolAndFallback) { session_deps_.use_alternative_services = true; diff --git a/net/http/http_stream_factory_impl.cc b/net/http/http_stream_factory_impl.cc index 68eb6af..2a8664e 100644 --- a/net/http/http_stream_factory_impl.cc +++ b/net/http/http_stream_factory_impl.cc @@ -152,9 +152,6 @@ const HostMappingRules* HttpStreamFactoryImpl::GetHostMappingRules() const { AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( const GURL& original_url) { - if (!session_->params().use_alternative_services) - return AlternativeServiceVector(); - if (original_url.SchemeIs("ftp")) return AlternativeServiceVector(); @@ -166,6 +163,9 @@ AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( if (alternative_service_vector.empty()) return AlternativeServiceVector(); + const bool enable_different_host = + session_->params().use_alternative_services; + AlternativeServiceVector enabled_alternative_service_vector; for (const AlternativeService& alternative_service : alternative_service_vector) { @@ -176,6 +176,9 @@ AlternativeServiceVector HttpStreamFactoryImpl::GetAlternativeServicesFor( continue; } + if (origin.host() != alternative_service.host && !enable_different_host) + continue; + // Some shared unix systems may have user home directories (like // http://foo.com/~mike) which allow users to emit headers. This is a bad // idea already, but with Alternate-Protocol, it provides the ability for a |