Do not crash if WebSocket server negotiates HTTP/2 over proxy.

I overzealously landed https://crrev.com/c/990512 assuming that if a
TCP connection is open through an HTTP/2 proxy with an empty ALPN list,
then the server cannot negotiate HTTP/2.  Turns out that there are
already 11 crashes reported on the first day of the Canary release
that picked up this change.  This CL adds a unittest that triggers this
CHECK (verified locally), and changes Job::DoInitConnectionComplete() to
handle this case properly and avoid the crash.

Bug: 828865
Change-Id: I5bc8fb447a07d350970904b350f9aeb670e2da7d
Reviewed-on: https://chromium-review.googlesource.com/996878Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548520}

net/http/http_stream_factory_impl_job.cc

@@ -1056,11 +1056,18 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
             NetLogEventType::HTTP_STREAM_REQUEST_PROTO,
             base::Bind(&NetLogHttpStreamProtoCallback, negotiated_protocol_));
         if (negotiated_protocol_ == kProtoHTTP2) {
-          // If request is WebSocket with no proxy, then HTTP/2 must not have
-          // been advertised in the TLS handshake.  The TLS layer must not have
-          // accepted the server choosing HTTP/2.
-          // TODO(bnc): Change to DCHECK once https://crbug.com/819101 is fixed.
-          CHECK(!(is_websocket_ && proxy_info_.is_direct()));
+          if (is_websocket_) {
+            // If request is WebSocket with no proxy, then HTTP/2 must not have
+            // been advertised in the TLS handshake.  The TLS layer must not
+            // have accepted the server choosing HTTP/2.
+            // TODO(bnc): Change to DCHECK once https://crbug.com/819101 is
+            // fixed.
+            CHECK(!proxy_info_.is_direct());
+
+            // WebSocket is not supported over a fresh HTTP/2 connection.
+            return ERR_NOT_IMPLEMENTED;
+          }
+
           using_spdy_ = true;
         }
       }

net/spdy/chromium/spdy_network_transaction_unittest.cc

@@ -7350,6 +7350,8 @@ TEST_F(SpdyNetworkTransactionTest, SecureWebSocketOverHttp2Proxy) {
   SSLSocketDataProvider ssl_provider(ASYNC, OK);
   ssl_provider.ssl_info.cert =
       ImportCertFromFile(GetTestCertsDirectory(), "wildcard.pem");
+  // A WebSocket request should not advertise HTTP/2 support.
+  ssl_provider.next_protos_expected_in_ssl_config = NextProtoVector{};
   // This test uses WebSocket over HTTP/1.1.
   ssl_provider.next_proto = kProtoHTTP11;
   helper.session_deps()->socket_factory->AddSSLSocketDataProvider(
@@ -7378,6 +7380,56 @@ TEST_F(SpdyNetworkTransactionTest, SecureWebSocketOverHttp2Proxy) {
   helper.VerifyDataConsumed();
 }
 
+// Regression test for https://crbug.com/828865.
+TEST_F(SpdyNetworkTransactionTest,
+       SecureWebSocketOverHttp2ProxyNegotiatesHttp2) {
+  SpdySerializedFrame connect_request(spdy_util_.ConstructSpdyConnect(
+      nullptr, 0, 1, LOWEST, HostPortPair("www.example.org", 443)));
+  MockWrite writes[] = {CreateMockWrite(connect_request, 0)};
+  SpdySerializedFrame connect_response(
+      spdy_util_.ConstructSpdyGetReply(nullptr, 0, 1));
+  MockRead reads[] = {CreateMockRead(connect_response, 1),
+                      MockRead(ASYNC, 0, 2)};
+  SequencedSocketData data(reads, arraysize(reads), writes, arraysize(writes));
+
+  request_.url = GURL("wss://www.example.org/");
+  request_.extra_headers.SetHeader("Connection", "Upgrade");
+  request_.extra_headers.SetHeader("Upgrade", "websocket");
+  request_.extra_headers.SetHeader("Origin", "http://www.example.org");
+  request_.extra_headers.SetHeader("Sec-WebSocket-Version", "13");
+  auto session_deps = std::make_unique<SpdySessionDependencies>(
+      ProxyResolutionService::CreateFixed("https://proxy:70",
+                                          TRAFFIC_ANNOTATION_FOR_TESTS));
+  NormalSpdyTransactionHelper helper(request_, DEFAULT_PRIORITY, log_,
+                                     std::move(session_deps));
+  helper.RunPreTestSetup();
+  helper.AddData(&data);
+
+  // Add SSL data for the tunneled connection.
+  SSLSocketDataProvider ssl_provider(ASYNC, OK);
+  ssl_provider.ssl_info.cert =
+      ImportCertFromFile(GetTestCertsDirectory(), "wildcard.pem");
+  // A WebSocket request should not advertise HTTP/2 support.
+  ssl_provider.next_protos_expected_in_ssl_config = NextProtoVector{};
+  // The server should not negotiate HTTP/2 over the tunnelled connection,
+  // but it must be handled gracefully if it does.
+  ssl_provider.next_proto = kProtoHTTP2;
+  helper.session_deps()->socket_factory->AddSSLSocketDataProvider(
+      &ssl_provider);
+
+  HttpNetworkTransaction* trans = helper.trans();
+  TestWebSocketHandshakeStreamCreateHelper websocket_stream_create_helper;
+  trans->SetWebSocketHandshakeStreamCreateHelper(
+      &websocket_stream_create_helper);
+
+  EXPECT_TRUE(helper.StartDefaultTest());
+  helper.WaitForCallbackToComplete();
+  EXPECT_THAT(helper.output().rv, IsError(ERR_NOT_IMPLEMENTED));
+
+  base::RunLoop().RunUntilIdle();
+  helper.VerifyDataConsumed();
+}
+
 #endif  // BUILDFLAG(ENABLE_WEBSOCKETS)
 
 }  // namespace net