From d3772280b808f5e0954809df754c270b73069b53 Mon Sep 17 00:00:00 2001 From: Jason Duell Date: Wed, 11 May 2016 08:50:13 -0700 Subject: [PATCH] Bug 711886. Fail Websocket if server replies with non-matching subprotocol r=mcmanus --- dom/base/test/file_websocket_wsh.py | 3 +++ dom/base/test/test_websocket5.html | 1 + dom/base/test/websocket_tests.js | 26 +++++++++++++++++++ dom/workers/test/websocket_worker5.js | 1 + .../protocol/websocket/WebSocketChannel.cpp | 15 ++++++----- 5 files changed, 40 insertions(+), 6 deletions(-) diff --git a/dom/base/test/file_websocket_wsh.py b/dom/base/test/file_websocket_wsh.py index ef440580ef2f..2d5e62e324a4 100644 --- a/dom/base/test/file_websocket_wsh.py +++ b/dom/base/test/file_websocket_wsh.py @@ -28,6 +28,9 @@ def web_socket_do_extra_handshake(request): time.sleep(13) elif request.ws_protocol == "test-41b": request.sts = "max-age=100" + elif request.ws_protocol == "test-49": + # subprotocols are compared case-sensitively, so this should fail + request.ws_protocol = "teST-49" else: pass diff --git a/dom/base/test/test_websocket5.html b/dom/base/test/test_websocket5.html index 70a948821150..ff85337cd2d2 100644 --- a/dom/base/test/test_websocket5.html +++ b/dom/base/test/test_websocket5.html @@ -21,6 +21,7 @@ var tests = [ test47, // Make sure onerror/onclose aren't called during close() test48, // see bug 1227136 - client calls close() from onopen() and waits // until WebSocketChannel::mSocketIn is nulled out on socket thread + test49, // Test that we fail if subprotocol returned from server doesn't match ]; function testWebSocket() { diff --git a/dom/base/test/websocket_tests.js b/dom/base/test/websocket_tests.js index e62896507178..6cf3d3b46aa6 100644 --- a/dom/base/test/websocket_tests.js +++ b/dom/base/test/websocket_tests.js @@ -1242,3 +1242,29 @@ function test48() { SpecialPowers.clearUserPref(pref_close); }); } + +function test49() +{ + return new Promise(function(resolve, reject) { + var ws = CreateTestWS("ws://mochi.test:8888/tests/dom/base/test/file_websocket", "test-49"); + var gotError = 0; + ok(ws.readyState == 0, "create bad readyState in test-49!"); + + ws.onopen = function() + { + ok(false, "Connection must fail in test-49") + } + + ws.onerror = function(e) + { + gotError = 1 + } + + ws.onclose = function(e) + { + ok(gotError, "Should get error in test-49!"); + resolve(); + } + }); +} + diff --git a/dom/workers/test/websocket_worker5.js b/dom/workers/test/websocket_worker5.js index 41d6e9be0053..71085a601400 100644 --- a/dom/workers/test/websocket_worker5.js +++ b/dom/workers/test/websocket_worker5.js @@ -8,6 +8,7 @@ var tests = [ test44, // Test sending/receving binary ArrayBuffer test46, // Test that we don't dispatch incoming msgs once in CLOSING state test47, // Make sure onerror/onclose aren't called during close() + test49, // Test that we fail if subprotocol returned from server doesn't match ]; doTest(); diff --git a/netwerk/protocol/websocket/WebSocketChannel.cpp b/netwerk/protocol/websocket/WebSocketChannel.cpp index 98b4613afd7f..da7c35d41eda 100644 --- a/netwerk/protocol/websocket/WebSocketChannel.cpp +++ b/netwerk/protocol/websocket/WebSocketChannel.cpp @@ -3645,9 +3645,10 @@ WebSocketChannel::OnStartRequest(nsIRequest *aRequest, return NS_ERROR_ILLEGAL_VALUE; } - // If we sent a sub protocol header, verify the response matches - // If it does not, set mProtocol to "" so the protocol attribute - // of the WebSocket JS object reflects that + // If we sent a sub protocol header, verify the response matches. + // If response contains protocol that was not in request, fail. + // If response contained no protocol header, set to "" so the protocol + // attribute of the WebSocket JS object reflects that if (!mProtocol.IsEmpty()) { nsAutoCString respProtocol; rv = mHttpChannel->GetResponseHeader( @@ -3657,7 +3658,7 @@ WebSocketChannel::OnStartRequest(nsIRequest *aRequest, rv = NS_ERROR_ILLEGAL_VALUE; val = mProtocol.BeginWriting(); while ((token = nsCRT::strtok(val, ", \t", &val))) { - if (PL_strcasecmp(token, respProtocol.get()) == 0) { + if (PL_strcmp(token, respProtocol.get()) == 0) { rv = NS_OK; break; } @@ -3669,9 +3670,11 @@ WebSocketChannel::OnStartRequest(nsIRequest *aRequest, mProtocol = respProtocol; } else { LOG(("WebsocketChannel::OnStartRequest: " - "subprotocol [%s] not found - %s returned", - mProtocol.get(), respProtocol.get())); + "Server replied with non-matching subprotocol [%s]: aborting", + respProtocol.get())); mProtocol.Truncate(); + AbortSession(NS_ERROR_ILLEGAL_VALUE); + return NS_ERROR_ILLEGAL_VALUE; } } else { LOG(("WebsocketChannel::OnStartRequest "