Fail WebSocket channel when handshake fails.
Call WebSocketMsg_NotifyFailure instead of
WebSocketMsg_AddChannelResponse(true, ...) when the opening handshake fails.
BUG=310405
[email protected]
Review URL: https://codereview.chromium.org/105833003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243835 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/content/browser/renderer_host/websocket_dispatcher_host.cc b/content/browser/renderer_host/websocket_dispatcher_host.cc
index d64a8a4f..5ce0798 100644
--- a/content/browser/renderer_host/websocket_dispatcher_host.cc
+++ b/content/browser/renderer_host/websocket_dispatcher_host.cc
@@ -138,6 +138,17 @@
routing_id, response));
}
+WebSocketHostState WebSocketDispatcherHost::NotifyFailure(
+ int routing_id,
+ const std::string& message) {
+ if (SendOrDrop(new WebSocketMsg_NotifyFailure(
+ routing_id, message)) == WEBSOCKET_HOST_DELETED) {
+ return WEBSOCKET_HOST_DELETED;
+ }
+ DeleteWebSocketHost(routing_id);
+ return WEBSOCKET_HOST_DELETED;
+}
+
WebSocketHostState WebSocketDispatcherHost::DoDropChannel(
int routing_id,
uint16 code,
diff --git a/content/browser/renderer_host/websocket_dispatcher_host.h b/content/browser/renderer_host/websocket_dispatcher_host.h
index 638178e..0ce6012 100644
--- a/content/browser/renderer_host/websocket_dispatcher_host.h
+++ b/content/browser/renderer_host/websocket_dispatcher_host.h
@@ -94,6 +94,12 @@
int routing_id,
const WebSocketHandshakeResponse& response) WARN_UNUSED_RESULT;
+ // Sends a WebSocketMsg_NotifyFailure IPC and deletes and unregisters the
+ // channel.
+ WebSocketHostState NotifyFailure(
+ int routing_id,
+ const std::string& message) WARN_UNUSED_RESULT;
+
// Sends a WebSocketMsg_DropChannel IPC and deletes and unregisters the
// channel.
WebSocketHostState DoDropChannel(
diff --git a/content/browser/renderer_host/websocket_host.cc b/content/browser/renderer_host/websocket_host.cc
index aff172c..b1a0bab3 100644
--- a/content/browser/renderer_host/websocket_host.cc
+++ b/content/browser/renderer_host/websocket_host.cc
@@ -92,6 +92,7 @@
virtual ChannelState OnFlowControl(int64 quota) OVERRIDE;
virtual ChannelState OnDropChannel(uint16 code,
const std::string& reason) OVERRIDE;
+ virtual ChannelState OnFailChannel(const std::string& message) OVERRIDE;
private:
WebSocketDispatcherHost* const dispatcher_;
@@ -150,6 +151,13 @@
return StateCast(dispatcher_->DoDropChannel(routing_id_, code, reason));
}
+ChannelState WebSocketEventHandler::OnFailChannel(const std::string& message) {
+ DVLOG(3) << "WebSocketEventHandler::OnFailChannel"
+ << " routing_id=" << routing_id_
+ << " message=\"" << message << "\"";
+ return StateCast(dispatcher_->NotifyFailure(routing_id_, message));
+}
+
} // namespace
WebSocketHost::WebSocketHost(int routing_id,
diff --git a/net/http/http_stream_factory_impl_unittest.cc b/net/http/http_stream_factory_impl_unittest.cc
index e3169b6..c326b8a 100644
--- a/net/http/http_stream_factory_impl_unittest.cc
+++ b/net/http/http_stream_factory_impl_unittest.cc
@@ -117,6 +117,10 @@
return scoped_ptr<WebSocketStream>();
}
+ virtual std::string GetFailureMessage() const OVERRIDE {
+ return std::string();
+ }
+
private:
const StreamType type_;
};
diff --git a/net/url_request/url_request_http_job_unittest.cc b/net/url_request/url_request_http_job_unittest.cc
index 9637917..e7f0bf7e 100644
--- a/net/url_request/url_request_http_job_unittest.cc
+++ b/net/url_request/url_request_http_job_unittest.cc
@@ -257,6 +257,10 @@
return scoped_ptr<WebSocketStream>();
}
+ virtual std::string GetFailureMessage() const OVERRIDE {
+ return std::string();
+ }
+
private:
bool initialize_stream_was_called_;
};
diff --git a/net/websockets/websocket_basic_handshake_stream.cc b/net/websockets/websocket_basic_handshake_stream.cc
index 73a1045..e30cf9e 100644
--- a/net/websockets/websocket_basic_handshake_stream.cc
+++ b/net/websockets/websocket_basic_handshake_stream.cc
@@ -6,6 +6,7 @@
#include <algorithm>
#include <iterator>
+#include <string>
#include "base/base64.h"
#include "base/basictypes.h"
@@ -13,6 +14,7 @@
#include "base/containers/hash_tables.h"
#include "base/stl_util.h"
#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
#include "crypto/random.h"
#include "net/http/http_request_headers.h"
#include "net/http/http_request_info.h"
@@ -22,6 +24,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 +32,23 @@
namespace net {
namespace {
+enum GetHeaderResult {
+ GET_HEADER_OK,
+ GET_HEADER_MISSING,
+ GET_HEADER_MULTIPLE,
+};
+
+std::string MissingHeaderMessage(const std::string& header_name) {
+ return std::string("'") + header_name + "' header is missing";
+}
+
+std::string MultipleHeaderValuesMessage(const std::string& header_name) {
+ return
+ std::string("'") +
+ header_name +
+ "' header must not appear more than once in a response";
+}
+
std::string GenerateHandshakeChallenge() {
std::string raw_challenge(websockets::kRawChallengeLength, '\0');
crypto::RandBytes(string_as_array(&raw_challenge), raw_challenge.length());
@@ -45,57 +65,162 @@
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;
- while (headers->EnumerateHeader(&state, name, &token)) {
- if (++tokens > 1)
- return false;
- has_value = case_sensitive ? value == token
- : LowerCaseEqualsASCII(token, value.c_str());
+ size_t num_values = 0;
+ std::string temp_value;
+ while (headers->EnumerateHeader(&state, name, &temp_value)) {
+ if (++num_values > 1)
+ return GET_HEADER_MULTIPLE;
+ *value = temp_value;
}
- return has_value;
+ return num_values > 0 ? GET_HEADER_OK : GET_HEADER_MISSING;
+}
+
+bool ValidateHeaderHasSingleValue(GetHeaderResult result,
+ const std::string& header_name,
+ std::string* failure_message) {
+ if (result == GET_HEADER_MISSING) {
+ *failure_message = MissingHeaderMessage(header_name);
+ return false;
+ }
+ if (result == GET_HEADER_MULTIPLE) {
+ *failure_message = MultipleHeaderValuesMessage(header_name);
+ return false;
+ }
+ DCHECK_EQ(result, GET_HEADER_OK);
+ return true;
+}
+
+bool ValidateUpgrade(const HttpResponseHeaders* headers,
+ std::string* failure_message) {
+ std::string value;
+ GetHeaderResult result =
+ GetSingleHeaderValue(headers, websockets::kUpgrade, &value);
+ if (!ValidateHeaderHasSingleValue(result,
+ websockets::kUpgrade,
+ failure_message)) {
+ return false;
+ }
+
+ if (!LowerCaseEqualsASCII(value, websockets::kWebSocketLowercase)) {
+ *failure_message =
+ "'Upgrade' header value is not 'WebSocket': " + value;
+ return false;
+ }
+ 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 (!ValidateHeaderHasSingleValue(result,
+ websockets::kSecWebSocketAccept,
+ failure_message)) {
+ 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) {
+ // Connection header is permitted to contain other tokens.
+ if (!headers->HasHeader(HttpRequestHeaders::kConnection)) {
+ *failure_message = MissingHeaderMessage(HttpRequestHeaders::kConnection);
+ 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 value;
base::hash_set<std::string> requested_set(requested_sub_protocols.begin(),
requested_sub_protocols.end());
- int accepted = 0;
- while (headers->EnumerateHeader(
- &state, websockets::kSecWebSocketProtocol, &token)) {
- if (requested_set.count(token) == 0)
- return false;
+ int count = 0;
+ bool has_multiple_protocols = false;
+ bool has_invalid_protocol = false;
- *sub_protocol = token;
- // The server is only allowed to accept one protocol.
- if (++accepted > 1)
- return false;
+ while (!has_invalid_protocol || !has_multiple_protocols) {
+ std::string temp_value;
+ if (!headers->EnumerateHeader(
+ &state, websockets::kSecWebSocketProtocol, &temp_value))
+ break;
+ value = temp_value;
+ if (requested_set.count(value) == 0)
+ has_invalid_protocol = true;
+ if (++count > 1)
+ has_multiple_protocols = true;
}
- // If the browser requested > 0 protocols, the server is required to accept
- // one.
- return requested_set.empty() || accepted == 1;
+
+ if (has_multiple_protocols) {
+ *failure_message =
+ MultipleHeaderValuesMessage(websockets::kSecWebSocketProtocol);
+ 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: ")
+ + value;
+ return false;
+ } else if (has_invalid_protocol) {
+ *failure_message =
+ "'Sec-WebSocket-Protocol' header value '" +
+ value +
+ "' 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 was received";
+ return false;
+ }
+ *sub_protocol = value;
+ 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;
+ std::string value;
while (headers->EnumerateHeader(
- &state, websockets::kSecWebSocketExtensions, &token)) {
+ &state, websockets::kSecWebSocketExtensions, &value)) {
+ WebSocketExtensionParser parser;
+ parser.Parse(value);
+ if (parser.has_error()) {
+ // TODO(yhirano) Set appropriate failure message.
+ *failure_message =
+ "'Sec-WebSocket-Extensions' header value is "
+ "rejected by the parser: " +
+ value;
+ 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;
@@ -267,6 +392,10 @@
handshake_challenge_for_testing_.reset(new std::string(key));
}
+std::string WebSocketBasicHandshakeStream::GetFailureMessage() const {
+ return failure_message_;
+}
+
void WebSocketBasicHandshakeStream::ReadResponseHeadersCallback(
const CompletionCallback& callback,
int result) {
@@ -292,26 +421,30 @@
// Other status codes are potentially risky (see the warnings in the
// WHATWG WebSocket API spec) and so are dropped by default.
default:
+ failure_message_ = base::StringPrintf("Unexpected status code: %d",
+ headers->response_code());
return ERR_INVALID_RESPONSE;
}
}
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;
}
diff --git a/net/websockets/websocket_basic_handshake_stream.h b/net/websockets/websocket_basic_handshake_stream.h
index 2e5b628..8f3cdf5 100644
--- a/net/websockets/websocket_basic_handshake_stream.h
+++ b/net/websockets/websocket_basic_handshake_stream.h
@@ -72,6 +72,8 @@
// For tests only.
void SetWebSocketKeyForTesting(const std::string& key);
+ virtual std::string GetFailureMessage() const OVERRIDE;
+
private:
// A wrapper for the ReadResponseHeaders callback that checks whether or not
// the connection has been accepted.
@@ -114,6 +116,8 @@
// The extension(s) selected by the server.
std::string extensions_;
+ std::string failure_message_;
+
DISALLOW_COPY_AND_ASSIGN(WebSocketBasicHandshakeStream);
};
diff --git a/net/websockets/websocket_channel.cc b/net/websockets/websocket_channel.cc
index 61a47b71..34c58b3 100644
--- a/net/websockets/websocket_channel.cc
+++ b/net/websockets/websocket_channel.cc
@@ -112,8 +112,8 @@
// |this| may have been deleted.
}
- virtual void OnFailure(uint16 websocket_error) OVERRIDE {
- creator_->OnConnectFailure(websocket_error);
+ virtual void OnFailure(const std::string& message) OVERRIDE {
+ creator_->OnConnectFailure(message);
// |this| has been deleted.
}
@@ -312,11 +312,11 @@
// |this| may have been deleted.
}
-void WebSocketChannel::OnConnectFailure(uint16 websocket_error) {
+void WebSocketChannel::OnConnectFailure(const std::string& message) {
DCHECK_EQ(CONNECTING, state_);
state_ = CLOSED;
stream_request_.reset();
- AllowUnused(event_interface_->OnAddChannelResponse(true, ""));
+ AllowUnused(event_interface_->OnFailChannel(message));
// |this| has been deleted.
}
diff --git a/net/websockets/websocket_channel.h b/net/websockets/websocket_channel.h
index 61f191a..721f71c 100644
--- a/net/websockets/websocket_channel.h
+++ b/net/websockets/websocket_channel.h
@@ -153,7 +153,7 @@
// Failure callback from WebSocketStream::CreateAndConnectStream(). Reports
// failure to the event interface. May delete |this|.
- void OnConnectFailure(uint16 websocket_error);
+ void OnConnectFailure(const std::string& message);
// Returns true if state_ is SEND_CLOSED, CLOSE_WAIT or CLOSED.
bool InClosingState() const;
diff --git a/net/websockets/websocket_channel_test.cc b/net/websockets/websocket_channel_test.cc
index 1bd75db..2e20f30e 100644
--- a/net/websockets/websocket_channel_test.cc
+++ b/net/websockets/websocket_channel_test.cc
@@ -142,8 +142,9 @@
ChannelState(bool,
WebSocketMessageType,
const std::vector<char>&)); // NOLINT
- MOCK_METHOD1(OnFlowControl, ChannelState(int64)); // NOLINT
+ MOCK_METHOD1(OnFlowControl, ChannelState(int64)); // NOLINT
MOCK_METHOD0(OnClosingHandshake, ChannelState(void)); // NOLINT
+ MOCK_METHOD1(OnFailChannel, ChannelState(const std::string&)); // NOLINT
MOCK_METHOD2(OnDropChannel,
ChannelState(uint16, const std::string&)); // NOLINT
};
@@ -165,6 +166,9 @@
return CHANNEL_ALIVE;
}
virtual ChannelState OnClosingHandshake() OVERRIDE { return CHANNEL_ALIVE; }
+ virtual ChannelState OnFailChannel(const std::string& message) OVERRIDE {
+ return CHANNEL_DELETED;
+ }
virtual ChannelState OnDropChannel(uint16 code,
const std::string& reason) OVERRIDE {
return CHANNEL_DELETED;
@@ -761,7 +765,8 @@
EVENT_ON_DATA_FRAME = 0x2,
EVENT_ON_FLOW_CONTROL = 0x4,
EVENT_ON_CLOSING_HANDSHAKE = 0x8,
- EVENT_ON_DROP_CHANNEL = 0x10,
+ EVENT_ON_FAIL_CHANNEL = 0x10,
+ EVENT_ON_DROP_CHANNEL = 0x20,
};
class WebSocketChannelDeletingTest : public WebSocketChannelTest {
@@ -780,6 +785,7 @@
: deleting_(EVENT_ON_ADD_CHANNEL_RESPONSE | EVENT_ON_DATA_FRAME |
EVENT_ON_FLOW_CONTROL |
EVENT_ON_CLOSING_HANDSHAKE |
+ EVENT_ON_FAIL_CHANNEL |
EVENT_ON_DROP_CHANNEL) {}
// Create a ChannelDeletingFakeWebSocketEventInterface. Defined out-of-line to
// avoid circular dependency.
@@ -820,6 +826,10 @@
return fixture_->DeleteIfDeleting(EVENT_ON_CLOSING_HANDSHAKE);
}
+ virtual ChannelState OnFailChannel(const std::string& message) OVERRIDE {
+ return fixture_->DeleteIfDeleting(EVENT_ON_FAIL_CHANNEL);
+ }
+
virtual ChannelState OnDropChannel(uint16 code,
const std::string& reason) OVERRIDE {
return fixture_->DeleteIfDeleting(EVENT_ON_DROP_CHANNEL);
@@ -915,8 +925,7 @@
TEST_F(WebSocketChannelDeletingTest, OnAddChannelResponseFail) {
CreateChannelAndConnect();
EXPECT_TRUE(channel_);
- connect_data_.creator.connect_delegate->OnFailure(
- kWebSocketErrorNoStatusReceived);
+ connect_data_.creator.connect_delegate->OnFailure("bye");
EXPECT_EQ(NULL, channel_.get());
}
@@ -964,7 +973,7 @@
TEST_F(WebSocketChannelDeletingTest, OnFlowControlAfterSend) {
set_stream(make_scoped_ptr(new WriteableFakeWebSocketStream));
// Avoid deleting the channel yet.
- deleting_ = EVENT_ON_DROP_CHANNEL;
+ deleting_ = EVENT_ON_FAIL_CHANNEL | EVENT_ON_DROP_CHANNEL;
CreateChannelAndConnectSuccessfully();
ASSERT_TRUE(channel_);
deleting_ = EVENT_ON_FLOW_CONTROL;
@@ -1156,13 +1165,11 @@
}
TEST_F(WebSocketChannelEventInterfaceTest, ConnectFailureReported) {
- // true means failure.
- EXPECT_CALL(*event_interface_, OnAddChannelResponse(true, ""));
+ EXPECT_CALL(*event_interface_, OnFailChannel("hello"));
CreateChannelAndConnect();
- connect_data_.creator.connect_delegate->OnFailure(
- kWebSocketErrorNoStatusReceived);
+ connect_data_.creator.connect_delegate->OnFailure("hello");
}
TEST_F(WebSocketChannelEventInterfaceTest, NonWebSocketSchemeRejected) {
diff --git a/net/websockets/websocket_event_interface.h b/net/websockets/websocket_event_interface.h
index baba88c..9ca96f6 100644
--- a/net/websockets/websocket_event_interface.h
+++ b/net/websockets/websocket_event_interface.h
@@ -71,6 +71,16 @@
virtual ChannelState OnDropChannel(uint16 code, const std::string& reason)
WARN_UNUSED_RESULT = 0;
+ // Called when the browser fails the channel, as specified in the spec.
+ //
+ // The channel should not be used again after OnFailChannel() has been
+ // called.
+ //
+ // This method returns a ChannelState for consistency, but all implementations
+ // must delete the Channel and return CHANNEL_DELETED.
+ virtual ChannelState OnFailChannel(const std::string& message)
+ WARN_UNUSED_RESULT = 0;
+
protected:
WebSocketEventInterface() {}
diff --git a/net/websockets/websocket_handshake_stream_base.h b/net/websockets/websocket_handshake_stream_base.h
index 71d8321..b172a55e 100644
--- a/net/websockets/websocket_handshake_stream_base.h
+++ b/net/websockets/websocket_handshake_stream_base.h
@@ -9,6 +9,8 @@
// Since net/http can be built without linking net/websockets code,
// this file must not introduce any link-time dependencies on websockets.
+#include <string>
+
#include "base/basictypes.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
@@ -64,6 +66,10 @@
// been called.
virtual scoped_ptr<WebSocketStream> Upgrade() = 0;
+ // Returns a string describing the connection failure information.
+ // Returns an empty string if there is no failure.
+ virtual std::string GetFailureMessage() const = 0;
+
protected:
// As with the destructor, this must be inline.
WebSocketHandshakeStreamBase() {}
diff --git a/net/websockets/websocket_stream.cc b/net/websockets/websocket_stream.cc
index e81c24e..450ce32 100644
--- a/net/websockets/websocket_stream.cc
+++ b/net/websockets/websocket_stream.cc
@@ -70,7 +70,25 @@
}
void ReportFailure() {
- connect_delegate_->OnFailure(kWebSocketErrorAbnormalClosure);
+ std::string failure_message;
+ if (create_helper_->stream()) {
+ failure_message = create_helper_->stream()->GetFailureMessage();
+ } else {
+ switch (url_request_.status().status()) {
+ case URLRequestStatus::SUCCESS:
+ case URLRequestStatus::IO_PENDING:
+ break;
+ case URLRequestStatus::CANCELED:
+ failure_message = "WebSocket opening handshake was canceled";
+ break;
+ case URLRequestStatus::FAILED:
+ failure_message =
+ std::string("Error in connection establishment: ") +
+ ErrorToString(url_request_.status().error());
+ break;
+ }
+ }
+ connect_delegate_->OnFailure(failure_message);
}
private:
diff --git a/net/websockets/websocket_stream.h b/net/websockets/websocket_stream.h
index c08f8dc..78180fd1 100644
--- a/net/websockets/websocket_stream.h
+++ b/net/websockets/websocket_stream.h
@@ -57,10 +57,9 @@
// WebSocketStream.
virtual void OnSuccess(scoped_ptr<WebSocketStream> stream) = 0;
- // Called on failure to connect. The parameter is either one of the values
- // defined in net::WebSocketError, or an error defined by some WebSocket
- // extension protocol that we implement.
- virtual void OnFailure(unsigned short websocket_error) = 0;
+ // Called on failure to connect.
+ // |message| contains defails of the failure.
+ virtual void OnFailure(const std::string& message) = 0;
};
// Create and connect a WebSocketStream of an appropriate type. The actual
diff --git a/net/websockets/websocket_stream_test.cc b/net/websockets/websocket_stream_test.cc
index 3e11a95..f48dcbe 100644
--- a/net/websockets/websocket_stream_test.cc
+++ b/net/websockets/websocket_stream_test.cc
@@ -45,7 +45,7 @@
class WebSocketStreamCreateTest : public ::testing::Test {
protected:
- WebSocketStreamCreateTest() : websocket_error_(0) {}
+ WebSocketStreamCreateTest(): has_failed_(false) {}
void CreateAndConnectCustomResponse(
const std::string& socket_url,
@@ -110,18 +110,21 @@
return std::vector<std::string>();
}
- uint16 error() const { return websocket_error_; }
+ const std::string& failure_message() const { return failure_message_; }
+ bool has_failed() const { return has_failed_; }
class TestConnectDelegate : public WebSocketStream::ConnectDelegate {
public:
- TestConnectDelegate(WebSocketStreamCreateTest* owner) : owner_(owner) {}
+ explicit TestConnectDelegate(WebSocketStreamCreateTest* owner)
+ : owner_(owner) {}
virtual void OnSuccess(scoped_ptr<WebSocketStream> stream) OVERRIDE {
stream.swap(owner_->stream_);
}
- virtual void OnFailure(uint16 websocket_error) OVERRIDE {
- owner_->websocket_error_ = websocket_error;
+ virtual void OnFailure(const std::string& message) OVERRIDE {
+ owner_->has_failed_ = true;
+ owner_->failure_message_ = message;
}
private:
@@ -132,8 +135,9 @@
scoped_ptr<WebSocketStreamRequest> stream_request_;
// Only set if the connection succeeded.
scoped_ptr<WebSocketStream> stream_;
- // Only set if the connection failed. 0 otherwise.
- uint16 websocket_error_;
+ // Only set if the connection failed.
+ std::string failure_message_;
+ bool has_failed_;
};
// Confirm that the basic case works as expected.
@@ -141,6 +145,7 @@
CreateAndConnectStandard(
"ws://localhost/", "/", NoSubProtocols(), "http://localhost/", "", "");
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_TRUE(stream_);
}
@@ -148,6 +153,7 @@
TEST_F(WebSocketStreamCreateTest, NeedsToRunLoop) {
CreateAndConnectStandard(
"ws://localhost/", "/", NoSubProtocols(), "http://localhost/", "", "");
+ EXPECT_FALSE(has_failed());
EXPECT_FALSE(stream_);
}
@@ -160,6 +166,7 @@
"",
"");
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_TRUE(stream_);
}
@@ -172,6 +179,7 @@
"",
"");
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_TRUE(stream_);
}
@@ -189,6 +197,7 @@
"Sec-WebSocket-Protocol: chatv20.chromium.org\r\n");
RunUntilIdle();
EXPECT_TRUE(stream_);
+ EXPECT_FALSE(has_failed());
EXPECT_EQ("chatv20.chromium.org", stream_->GetSubProtocol());
}
@@ -202,20 +211,30 @@
"Sec-WebSocket-Protocol: chatv20.chromium.org\r\n");
RunUntilIdle();
EXPECT_FALSE(stream_);
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "Response must not include 'Sec-WebSocket-Protocol' header "
+ "if not present in request: chatv20.chromium.org",
+ failure_message());
}
// Missing sub-protocol response is rejected.
TEST_F(WebSocketStreamCreateTest, UnacceptedSubProtocol) {
+ std::vector<std::string> sub_protocols;
+ sub_protocols.push_back("chat.example.com");
CreateAndConnectStandard("ws://localhost/testing_path",
"/testing_path",
- std::vector<std::string>(1, "chat.example.com"),
+ sub_protocols,
"http://localhost/",
"Sec-WebSocket-Protocol: chat.example.com\r\n",
"");
RunUntilIdle();
EXPECT_FALSE(stream_);
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "Sent non-empty 'Sec-WebSocket-Protocol' header "
+ "but no response was received",
+ failure_message());
}
// Only one sub-protocol can be accepted.
@@ -233,7 +252,32 @@
"chatv20.chromium.org\r\n");
RunUntilIdle();
EXPECT_FALSE(stream_);
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Sec-WebSocket-Protocol' header must not appear "
+ "more than once in a response",
+ failure_message());
+}
+
+// Unmatched sub-protocol should be rejected.
+TEST_F(WebSocketStreamCreateTest, UnmatchedSubProtocolInResponse) {
+ std::vector<std::string> sub_protocols;
+ sub_protocols.push_back("chatv11.chromium.org");
+ sub_protocols.push_back("chatv20.chromium.org");
+ CreateAndConnectStandard("ws://localhost/testing_path",
+ "/testing_path",
+ sub_protocols,
+ "http://google.com/",
+ "Sec-WebSocket-Protocol: chatv11.chromium.org, "
+ "chatv20.chromium.org\r\n",
+ "Sec-WebSocket-Protocol: chatv21.chromium.org\r\n");
+ RunUntilIdle();
+ EXPECT_FALSE(stream_);
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Sec-WebSocket-Protocol' header value 'chatv21.chromium.org' "
+ "in response does not match any of sent values",
+ failure_message());
}
// Unknown extension in the response is rejected
@@ -246,7 +290,11 @@
"Sec-WebSocket-Extensions: x-unknown-extension\r\n");
RunUntilIdle();
EXPECT_FALSE(stream_);
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "Found an unsupported extension 'x-unknown-extension' "
+ "in 'Sec-WebSocket-Extensions' header",
+ failure_message());
}
// Additional Sec-WebSocket-Accept headers should be rejected.
@@ -260,7 +308,11 @@
"Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n");
RunUntilIdle();
EXPECT_FALSE(stream_);
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Sec-WebSocket-Accept' header must not appear "
+ "more than once in a response",
+ failure_message());
}
// Response code 200 must be rejected.
@@ -278,7 +330,8 @@
"",
kInvalidStatusCodeResponse);
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Unexpected status code: 200", failure_message());
}
// Redirects are not followed (according to the WHATWG WebSocket API, which
@@ -299,7 +352,8 @@
"",
kRedirectResponse);
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Unexpected status code: 302", failure_message());
}
// Malformed responses should be rejected. HttpStreamParser will accept just
@@ -321,7 +375,8 @@
"",
kMalformedResponse);
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Unexpected status code: 200", failure_message());
}
// Upgrade header must be present.
@@ -338,7 +393,9 @@
"",
kMissingUpgradeResponse);
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: 'Upgrade' header is missing",
+ failure_message());
}
// There must only be one upgrade header.
@@ -350,7 +407,31 @@
"http://localhost/",
"", "Upgrade: HTTP/2.0\r\n");
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Upgrade' header must not appear more than once in a response",
+ failure_message());
+}
+
+// There must only be one correct upgrade header.
+TEST_F(WebSocketStreamCreateTest, IncorrectUpgradeHeader) {
+ static const char kMissingUpgradeResponse[] =
+ "HTTP/1.1 101 Switching Protocols\r\n"
+ "Connection: Upgrade\r\n"
+ "Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n"
+ "Upgrade: hogefuga\r\n"
+ "\r\n";
+ CreateAndConnectCustomResponse("ws://localhost/",
+ "/",
+ NoSubProtocols(),
+ "http://localhost/",
+ "",
+ kMissingUpgradeResponse);
+ RunUntilIdle();
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Upgrade' header value is not 'WebSocket': hogefuga",
+ failure_message());
}
// Connection header must be present.
@@ -367,7 +448,31 @@
"",
kMissingConnectionResponse);
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Connection' header is missing",
+ failure_message());
+}
+
+// Connection header must contain "Upgrade".
+TEST_F(WebSocketStreamCreateTest, IncorrectConnectionHeader) {
+ static const char kMissingConnectionResponse[] =
+ "HTTP/1.1 101 Switching Protocols\r\n"
+ "Upgrade: websocket\r\n"
+ "Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n"
+ "Connection: hogefuga\r\n"
+ "\r\n";
+ CreateAndConnectCustomResponse("ws://localhost/",
+ "/",
+ NoSubProtocols(),
+ "http://localhost/",
+ "",
+ kMissingConnectionResponse);
+ RunUntilIdle();
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Connection' header value must contain 'Upgrade'",
+ failure_message());
}
// Connection header is permitted to contain other tokens.
@@ -385,6 +490,7 @@
"",
kAdditionalConnectionTokenResponse);
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_TRUE(stream_);
}
@@ -402,7 +508,10 @@
"",
kMissingAcceptResponse);
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "'Sec-WebSocket-Accept' header is missing",
+ failure_message());
}
// Sec-WebSocket-Accept header must match the key that was sent.
@@ -420,7 +529,10 @@
"",
kIncorrectAcceptResponse);
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error during WebSocket handshake: "
+ "Incorrect 'Sec-WebSocket-Accept' header value",
+ failure_message());
}
// Cancellation works.
@@ -429,6 +541,7 @@
"ws://localhost/", "/", NoSubProtocols(), "http://localhost/", "", "");
stream_request_.reset();
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_FALSE(stream_);
}
@@ -441,7 +554,9 @@
CreateAndConnectRawExpectations("ws://localhost/", NoSubProtocols(),
"http://localhost/", socket_data.Pass());
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error in connection establishment: net::ERR_CONNECTION_REFUSED",
+ failure_message());
}
// Connect timeout must look just like any other failure.
@@ -453,7 +568,9 @@
CreateAndConnectRawExpectations("ws://localhost/", NoSubProtocols(),
"http://localhost/", socket_data.Pass());
RunUntilIdle();
- EXPECT_EQ(1006, error());
+ EXPECT_TRUE(has_failed());
+ EXPECT_EQ("Error in connection establishment: net::ERR_CONNECTION_TIMED_OUT",
+ failure_message());
}
// Cancellation during connect works.
@@ -467,6 +584,7 @@
socket_data.Pass());
stream_request_.reset();
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_FALSE(stream_);
}
@@ -487,6 +605,7 @@
socket_data->Run();
stream_request_.reset();
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_FALSE(stream_);
}
@@ -508,6 +627,7 @@
socket_data->Run();
stream_request_.reset();
RunUntilIdle();
+ EXPECT_FALSE(has_failed());
EXPECT_FALSE(stream_);
}
