Skip to content

Commit

Permalink
Disable HTTP/2 Alternative Services.
Browse files Browse the repository at this point in the history
* Temporarily disable HTTP/2 Alternative Services,
  see https://crbug.com/706974#c5 for motivation.
* Rename test-only enable_http2_alternative_service_with_different_host
  to enable_http2_alternative_service and change function accordingly.

BUG=706974

Review-Url: https://codereview.chromium.org/2821463002
Cr-Commit-Position: refs/heads/master@{#464916}
  • Loading branch information
bnc authored and Commit bot committed Apr 17, 2017
1 parent 6648e7b commit a86731e
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 19 deletions.
4 changes: 2 additions & 2 deletions jingle/glue/proxy_resolving_client_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ ProxyResolvingClientSocket::ProxyResolvingClientSocket(
session_params.testing_fixed_https_port =
reference_params->testing_fixed_https_port;
session_params.enable_http2 = reference_params->enable_http2;
session_params.enable_http2_alternative_service_with_different_host =
reference_params->enable_http2_alternative_service_with_different_host;
session_params.enable_http2_alternative_service =
reference_params->enable_http2_alternative_service;
session_params.enable_quic_alternative_service_with_different_host =
reference_params->enable_quic_alternative_service_with_different_host;
}
Expand Down
2 changes: 1 addition & 1 deletion net/http/http_network_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ HttpNetworkSession::Params::Params()
enable_http2(true),
spdy_session_max_recv_window_size(kSpdySessionMaxRecvWindowSize),
time_func(&base::TimeTicks::Now),
enable_http2_alternative_service_with_different_host(false),
enable_http2_alternative_service(false),
enable_quic_alternative_service_with_different_host(true),
enable_quic(false),
mark_quic_broken_when_network_blackholes(false),
Expand Down
5 changes: 2 additions & 3 deletions net/http/http_network_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ class NET_EXPORT HttpNetworkSession
SettingsMap http2_settings;
// Source of time for SPDY connections.
SpdySessionPool::TimeFunc time_func;
// Whether to enable HTTP/2 Alt-Svc entries with hostname different than
// that of the origin.
bool enable_http2_alternative_service_with_different_host;
// Whether to enable HTTP/2 Alt-Svc entries.
bool enable_http2_alternative_service;
// Whether to enable QUIC Alt-Svc entries with hostname different than that
// of the origin.
bool enable_quic_alternative_service_with_different_host;
Expand Down
7 changes: 3 additions & 4 deletions net/http/http_network_transaction_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ class HttpNetworkTransactionTest : public PlatformTest {
HttpNetworkSession::NORMAL_SOCKET_POOL)),
old_max_pool_sockets_(ClientSocketPoolManager::max_sockets_per_pool(
HttpNetworkSession::NORMAL_SOCKET_POOL)) {
session_deps_.enable_http2_alternative_service_with_different_host = true;
session_deps_.enable_http2_alternative_service = true;
}

struct SimpleGetHelperResult {
Expand Down Expand Up @@ -10162,12 +10162,11 @@ TEST_F(HttpNetworkTransactionTest,
EXPECT_TRUE(alternative_service_vector.empty());
}

// HTTP/2 Alternative Services should be disabled if alternative service
// hostname is different from that of origin.
// HTTP/2 Alternative Services should be disabled by default.
// TODO(bnc): Remove when https://crbug.com/615413 is fixed.
TEST_F(HttpNetworkTransactionTest,
DisableHTTP2AlternativeServicesWithDifferentHost) {
session_deps_.enable_http2_alternative_service_with_different_host = false;
session_deps_.enable_http2_alternative_service = false;

HttpRequestInfo request;
request.method = "GET";
Expand Down
6 changes: 1 addition & 5 deletions net/http/http_stream_factory_impl_job_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,6 @@ HttpStreamFactoryImpl::JobController::GetAlternativeServiceForInternal(
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
Expand All @@ -1009,11 +1008,8 @@ HttpStreamFactoryImpl::JobController::GetAlternativeServiceForInternal(
continue;

if (alternative_service.protocol == kProtoHTTP2) {
if (origin.host() != alternative_service.host &&
!session_->params()
.enable_http2_alternative_service_with_different_host) {
if (!session_->params().enable_http2_alternative_service)
continue;
}

// Cache this entry if we don't have a non-broken Alt-Svc yet.
if (first_alternative_service.protocol == kProtoUnknown)
Expand Down
6 changes: 3 additions & 3 deletions net/spdy/spdy_test_util_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ SpdySessionDependencies::SpdySessionDependencies(
enable_server_push_cancellation(false),
session_max_recv_window_size(kDefaultInitialWindowSize),
time_func(&base::TimeTicks::Now),
enable_http2_alternative_service_with_different_host(false),
enable_http2_alternative_service(false),
net_log(nullptr),
http_09_on_non_default_ports_enabled(false),
restrict_to_one_preconnect_for_proxies(false),
Expand Down Expand Up @@ -413,8 +413,8 @@ HttpNetworkSession::Params SpdySessionDependencies::CreateSessionParams(
params.http2_settings = session_deps->http2_settings;
params.time_func = session_deps->time_func;
params.proxy_delegate = session_deps->proxy_delegate.get();
params.enable_http2_alternative_service_with_different_host =
session_deps->enable_http2_alternative_service_with_different_host;
params.enable_http2_alternative_service =
session_deps->enable_http2_alternative_service;
params.net_log = session_deps->net_log;
params.http_09_on_non_default_ports_enabled =
session_deps->http_09_on_non_default_ports_enabled;
Expand Down
2 changes: 1 addition & 1 deletion net/spdy/spdy_test_util_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ struct SpdySessionDependencies {
SettingsMap http2_settings;
SpdySession::TimeFunc time_func;
std::unique_ptr<ProxyDelegate> proxy_delegate;
bool enable_http2_alternative_service_with_different_host;
bool enable_http2_alternative_service;
NetLog* net_log;
bool http_09_on_non_default_ports_enabled;
bool restrict_to_one_preconnect_for_proxies;
Expand Down

0 comments on commit a86731e

Please sign in to comment.