Fail WebSocket channel when handshake fails.

net/websockets/websocket_basic_handshake_stream.cc

Index: net/websockets/websocket_basic_handshake_stream.cc
diff --git a/net/websockets/websocket_basic_handshake_stream.cc b/net/websockets/websocket_basic_handshake_stream.cc
index 1a8154b4b8b8b0863de003985825b593e756559b..40c48da4e29dd62187dc00a2386207b5520dc152 100644
--- a/net/websockets/websocket_basic_handshake_stream.cc
+++ b/net/websockets/websocket_basic_handshake_stream.cc
@@ -22,6 +22,7 @@
 #include "net/http/http_stream_parser.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/websockets/websocket_basic_stream.h"
+#include "net/websockets/websocket_extension_parser.h"
 #include "net/websockets/websocket_handshake_constants.h"
 #include "net/websockets/websocket_handshake_handler.h"
 #include "net/websockets/websocket_stream.h"
@@ -29,6 +30,12 @@
 namespace net {
 namespace {
 
+enum GetHeaderResult {
+  GET_HEADER_OK,
+  GET_HEADER_MISSING,
+  GET_HEADER_MULTIPLE,
+};
+
 std::string GenerateHandshakeChallenge() {
   std::string raw_challenge(websockets::kRawChallengeLength, '\0');
   crypto::RandBytes(string_as_array(&raw_challenge), raw_challenge.length());
@@ -46,57 +53,155 @@ void AddVectorHeaderIfNonEmpty(const char* name,
   headers->SetHeader(name, JoinString(value, ", "));
 }
 
-// If |case_sensitive| is false, then |value| must be in lower-case.
-bool ValidateSingleTokenHeader(
-    const scoped_refptr<HttpResponseHeaders>& headers,
-    const base::StringPiece& name,
-    const std::string& value,
-    bool case_sensitive) {
+GetHeaderResult GetSingleHeaderValue(const HttpResponseHeaders* headers,
+                                     const base::StringPiece& name,
+                                     std::string* value) {
   void* state = NULL;
-  std::string token;
   int tokens = 0;
-  bool has_value = false;
+  std::string token;
   while (headers->EnumerateHeader(&state, name, &token)) {
     if (++tokens > 1)
-      return false;
-    has_value = case_sensitive ? value == token
-                               : LowerCaseEqualsASCII(token, value.c_str());
+      return GET_HEADER_MULTIPLE;
+    *value = token;
+  }
+  return tokens > 0 ? GET_HEADER_OK : GET_HEADER_MISSING;
+}
+
+bool ValidateUpgrade(const HttpResponseHeaders* headers,
+                     std::string* failure_message) {
+  std::string value;
+  GetHeaderResult result =
+      GetSingleHeaderValue(headers, websockets::kUpgrade, &value);
+  if (result == GET_HEADER_MISSING) {
+    *failure_message = "'Upgrade' header is missing";
+    return false;
+  }
+  if (result == GET_HEADER_MULTIPLE) {
+    *failure_message =
+        "'Upgrade' header must not appear more than once in a response";
+    return false;
+  }
+
+  if (!LowerCaseEqualsASCII(value, websockets::kWebSocketLowercase)) {
+    *failure_message =
+        "'Upgrade' header value is not 'WebSocket': " + value;
+    return false;
   }
-  return has_value;
+  return true;
+}
+
+bool ValidateSecWebSocketAccept(const HttpResponseHeaders* headers,
+                                const std::string& expected,
+                                std::string* failure_message) {
+  std::string actual;
+  GetHeaderResult result =
+      GetSingleHeaderValue(headers, websockets::kSecWebSocketAccept, &actual);
+  if (result == GET_HEADER_MISSING) {
+    *failure_message = "'Sec-WebSocket-Accept' header is missing";

[Adam Rice, 2013/12/06 07:02:31]

In ValidateUpgrade() and ValidateSecWebSocketAccept(), the first two error
messages are the same except for the name of the header. I wonder if it would be
cleaner to share the code for that part?

[yhirano, 2013/12/06 08:54:56]

Done.

+    return false;
+  }
+  if (result == GET_HEADER_MULTIPLE) {
+    *failure_message =
+        "'Sec-WebSocket-Accept' header must not appear "
+        "more than once in a response";
+    return false;
+  }
+
+  if (expected != actual) {
+    *failure_message = "Incorrect 'Sec-WebSocket-Accept' header value";
+    return false;
+  }
+  return true;
+}
+
+bool ValidateConnection(const HttpResponseHeaders* headers,
+                     std::string* failure_message) {

[Adam Rice, 2013/12/06 07:02:31]

Indentation.

[yhirano, 2013/12/06 08:54:56]

Done.

+  // Connection header is permitted to contain other tokens.
+  if (!headers->HasHeader(HttpRequestHeaders::kConnection)) {
+    *failure_message = "'Connection' header is missing";
+    return false;
+  }
+  if (!headers->HasHeaderValue(HttpRequestHeaders::kConnection,
+                               websockets::kUpgrade)) {
+    *failure_message = "'Connection' header value must contain 'Upgrade'";
+    return false;
+  }
+  return true;
 }
 
 bool ValidateSubProtocol(
-    const scoped_refptr<HttpResponseHeaders>& headers,
+    const HttpResponseHeaders* headers,
     const std::vector<std::string>& requested_sub_protocols,
-    std::string* sub_protocol) {
+    std::string* sub_protocol,
+    std::string* failure_message) {
   void* state = NULL;
   std::string token;
+  std::string last_token;
   base::hash_set<std::string> requested_set(requested_sub_protocols.begin(),
                                             requested_sub_protocols.end());
-  int accepted = 0;
+  int count = 0;
+  bool has_multiple_protocols = false;
+  bool has_invalid_protocol = false;
+
   while (headers->EnumerateHeader(
-      &state, websockets::kSecWebSocketProtocol, &token)) {
+      &state, websockets::kSecWebSocketProtocol, &token) &&

[Adam Rice, 2013/12/06 07:02:31]

The indentation looks weird here. I would expect it to be indented four spaces
from the '(' on the previous line. I am not sure though.

[yhirano, 2013/12/06 08:54:56]

Done.

+         !(has_multiple_protocols && has_invalid_protocol)) {
     if (requested_set.count(token) == 0)
-      return false;
+      has_invalid_protocol = true;
+    if (++count > 1)
+      has_multiple_protocols = true;
+    last_token = token;
+  }
 
-    *sub_protocol = token;
-    // The server is only allowed to accept one protocol.
-    if (++accepted > 1)
-      return false;
+  if (has_multiple_protocols) {
+    *failure_message =
+        "'Sec-WebSocket-Protocol' header must not "
+        "appear more than once in a response";
+    return false;
+  } else if (count > 0 && requested_sub_protocols.size() == 0) {
+    *failure_message =
+        std::string("Response must not include 'Sec-WebSocket-Protocol' "
+                    "header if not present in request: ")
+        + last_token;
+    return false;
+  } else if (has_invalid_protocol) {
+    *failure_message =
+        "'Sec-WebSocket-Protocol' header value '" +
+        last_token +
+        "' in response does not match any of sent values";
+    return false;
+  } else if (requested_sub_protocols.size() > 0 && count == 0) {
+    *failure_message =
+        "Sent non-empty 'Sec-WebSocket-Protocol' header "
+        "but no response is received";

[Adam Rice, 2013/12/06 07:02:31]

Grammatically, this should be "no response was received", but since we are
aiming to match the messages for the layout tests we should keep this as it is,
at least while we have two implementations.

[yhirano, 2013/12/06 08:54:56]

Done.

+    return false;
   }
-  // If the browser requested > 0 protocols, the server is required to accept
-  // one.
-  return requested_set.empty() || accepted == 1;
+  *sub_protocol = last_token;
+  return true;
 }
 
-bool ValidateExtensions(const scoped_refptr<HttpResponseHeaders>& headers,
+bool ValidateExtensions(const HttpResponseHeaders* headers,
                         const std::vector<std::string>& requested_extensions,
-                        std::string* extensions) {
+                        std::string* extensions,
+                        std::string* failure_message) {
   void* state = NULL;
   std::string token;
   while (headers->EnumerateHeader(
       &state, websockets::kSecWebSocketExtensions, &token)) {
+    WebSocketExtensionParser parser;
+    parser.Parse(token);
+    if (parser.has_error()) {
+      // TODO(yhirano) Set appropriate failure message.
+      *failure_message =
+          "'WebSocket-Extensions' header value is rejected by the parser: " +
+          token;
+      return false;
+    }
     // TODO(ricea): Accept permessage-deflate with valid parameters.
+    *failure_message =
+        "Found an unsupported extension '" +
+        parser.extension().name() +
+        "' in 'Sec-WebSocket-Extensions' header";
     return false;
   }
   return true;
@@ -264,6 +369,10 @@ void WebSocketBasicHandshakeStream::SetWebSocketKeyForTesting(
   handshake_challenge_for_testing_.reset(new std::string(key));
 }
 
+std::string WebSocketBasicHandshakeStream::FailureMessage() const {
+  return failure_message_;
+}
+
 void WebSocketBasicHandshakeStream::ReadResponseHeadersCallback(
     const CompletionCallback& callback,
     int result) {
@@ -295,20 +404,22 @@ int WebSocketBasicHandshakeStream::ValidateResponse() {
 
 int WebSocketBasicHandshakeStream::ValidateUpgradeResponse(
     const scoped_refptr<HttpResponseHeaders>& headers) {
-  if (ValidateSingleTokenHeader(headers,
-                                websockets::kUpgrade,
-                                websockets::kWebSocketLowercase,
-                                false) &&
-      ValidateSingleTokenHeader(headers,
-                                websockets::kSecWebSocketAccept,
-                                handshake_challenge_response_,
-                                true) &&
-      headers->HasHeaderValue(HttpRequestHeaders::kConnection,
-                              websockets::kUpgrade) &&
-      ValidateSubProtocol(headers, requested_sub_protocols_, &sub_protocol_) &&
-      ValidateExtensions(headers, requested_extensions_, &extensions_)) {
+  if (ValidateUpgrade(headers.get(), &failure_message_) &&
+      ValidateSecWebSocketAccept(headers.get(),
+                                 handshake_challenge_response_,
+                                 &failure_message_) &&
+      ValidateConnection(headers.get(), &failure_message_) &&
+      ValidateSubProtocol(headers.get(),
+                          requested_sub_protocols_,
+                          &sub_protocol_,
+                          &failure_message_) &&
+      ValidateExtensions(headers.get(),
+                         requested_extensions_,
+                         &extensions_,
+                         &failure_message_)) {
     return OK;
   }
+  failure_message_ = "Error during WebSocket handshake: " + failure_message_;
   return ERR_INVALID_RESPONSE;
 }