Avoid WebSocket upgrade on network error.

In the event of network error, the WebSocket server response is not
validated. However, some network errors are ignored higher up in the
stack.

For example, in HttpNetworkTransaction::DoReadHeadersComplete at
http_network_transaction.cc:988, ERR_CONNECTION_CLOSED is converted to
OK.

To avoid this whole class of bugs, do not keep status code 101
(Switching Protocols) if there was a network error. It is changed to
503.

The response headers that are displayed in devtools are copied before
the change, so they will still retain the original status code.

BUG=408061
TEST=net_unittests

Review URL: https://codereview.chromium.org/889783002

Cr-Commit-Position: refs/heads/master@{#314136}

4 files changed, 68 insertions, 0 deletions

diff --git a/net/data/websocket/truncated-headers_wsh.py b/net/data/websocket/truncated-headers_wsh.py
new file mode 100644
index 0000000..70ce2f2
--- /dev/null
+++ b/net/data/websocket/truncated-headers_wsh.py
@@ -0,0 +1,19 @@
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from mod_pywebsocket import handshake
+
+
+def web_socket_do_extra_handshake(request):
+  msg = ('HTTP/1.1 101 Switching Protocols\r\n'
+         'Upgrade: websocket\r\n'
+         'Connection: Upgrade\r\n'
+         'Sec-WebSocket-Accept: 3rfd')
+  request.connection.write(msg)
+  # Prevent pywebsocket from sending its own handshake message.
+  raise handshake.AbortedByUserException('Close the connection')
+
+
+def web_socket_transfer_data(request):
+  pass
diff --git a/net/websockets/websocket_basic_handshake_stream.cc b/net/websockets/websocket_basic_handshake_stream.cc
index c0f20c7..7d99311 100644
--- a/net/websockets/websocket_basic_handshake_stream.cc
+++ b/net/websockets/websocket_basic_handshake_stream.cc
@@ -50,6 +50,8 @@ namespace net {
 
 namespace {
 
+const char kConnectionErrorStatusLine[] = "HTTP/1.1 503 Connection Error";
+
 // TODO(yhirano): Remove these functions once http://crbug.com/399535 is fixed.
 NOINLINE void RunCallbackWithOk(const CompletionCallback& callback,
                                 int result) {
@@ -654,6 +656,16 @@ int WebSocketBasicHandshakeStream::ValidateResponse(int rv,
     set_failure_message(std::string("Error during WebSocket handshake: ") +
                         ErrorToString(rv));
     OnFinishOpeningHandshake();
+    // Some error codes (for example ERR_CONNECTION_CLOSED) get changed to OK at
+    // higher levels. To prevent an unvalidated connection getting erroneously
+    // upgraded, don't pass through the status code unchanged if it is
+    // HTTP_SWITCHING_PROTOCOLS.
+    if (http_response_info_->headers &&
+        http_response_info_->headers->response_code() ==
+            HTTP_SWITCHING_PROTOCOLS) {
+      http_response_info_->headers->ReplaceStatusLine(
+          kConnectionErrorStatusLine);
+    }
     return rv;
   }
 }
diff --git a/net/websockets/websocket_end_to_end_test.cc b/net/websockets/websocket_end_to_end_test.cc
index 0b5944f..4aa2f1c 100644
--- a/net/websockets/websocket_end_to_end_test.cc
+++ b/net/websockets/websocket_end_to_end_test.cc
@@ -365,6 +365,19 @@ TEST_F(WebSocketEndToEndTest, DISABLED_ON_ANDROID(HttpsProxyUsed)) {
   EXPECT_TRUE(info.proxy_info.is_http());
 }
 
+// This is a regression test for crbug.com/408061 Crash in
+// net::WebSocketBasicHandshakeStream::Upgrade.
+TEST_F(WebSocketEndToEndTest, DISABLED_ON_ANDROID(TruncatedResponse)) {
+  SpawnedTestServer ws_server(SpawnedTestServer::TYPE_WS,
+                              SpawnedTestServer::kLocalhost,
+                              GetWebSocketTestDataDirectory());
+  ASSERT_TRUE(ws_server.Start());
+  InitialiseContext();
+
+  GURL ws_url = ws_server.GetURL("truncated-headers");
+  EXPECT_FALSE(ConnectAndWait(ws_url));
+}
+
 }  // namespace
 
 }  // namespace net
diff --git a/net/websockets/websocket_stream_test.cc b/net/websockets/websocket_stream_test.cc
index 0fdc34b..713af37 100644
--- a/net/websockets/websocket_stream_test.cc
+++ b/net/websockets/websocket_stream_test.cc
@@ -1388,5 +1388,29 @@ TEST_F(WebSocketStreamCreateUMATest, Failed) {
   EXPECT_EQ(0, samples->GetCount(FAILED));
 }
 
+TEST_F(WebSocketStreamCreateTest, HandleErrConnectionClosed) {
+  static const char kTruncatedResponse[] =
+      "HTTP/1.1 101 Switching Protocols\r\n"
+      "Upgrade: websocket\r\n"
+      "Connection: Upgrade\r\n"
+      "Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n"
+      "Cache-Control: no-sto";
+
+  std::string request =
+      WebSocketStandardRequest("/", "localhost", "http://localhost", "");
+  MockRead reads[] = {
+      MockRead(SYNCHRONOUS, 1, kTruncatedResponse),
+      MockRead(SYNCHRONOUS, ERR_CONNECTION_CLOSED, 2),
+  };
+  MockWrite writes[] = {MockWrite(SYNCHRONOUS, 0, request.c_str())};
+  scoped_ptr<DeterministicSocketData> socket_data(
+      BuildSocketData(reads, writes));
+  socket_data->set_connect_data(MockConnect(SYNCHRONOUS, OK));
+  CreateAndConnectRawExpectations("ws://localhost/", NoSubProtocols(),
+                                  "http://localhost", socket_data.Pass());
+  RunUntilIdle();
+  EXPECT_TRUE(has_failed());
+}
+
 }  // namespace
 }  // namespace net