- Revision
- 187556
- Author
- [email protected]
- Date
- 2015-07-29 14:08:30 -0700 (Wed, 29 Jul 2015)
Log Message
Crash calling webSocket.close() from onError handler for blocked web socket.
<rdar://problem/21771620> and https://bugs.webkit.org/show_bug.cgi?id=147411
Reviewed by Tim Horton.
Source/WebCore:
Tests: http/tests/security/mixedContent/websocket/insecure-websocket-in-iframe.html
http/tests/security/mixedContent/websocket/insecure-websocket-in-main-frame.html
This was introduced with http://trac.webkit.org/changeset/185848
* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::connect): When blocked because of mixedContent, call dispatchOrQueueErrorEvent().
(WebCore::WebSocket::didReceiveMessageError): Use dispatchOrQueueErrorEvent() instead.
(WebCore::WebSocket::dispatchOrQueueErrorEvent): Dispatch the error event, but don't dispatch one twice!
* Modules/websockets/WebSocket.h:
* Modules/websockets/WebSocketChannel.cpp:
(WebCore::WebSocketChannel::fail): Null-check m_handshake before creating a console message from it.
LayoutTests:
* http/tests/security/mixedContent/resources/frame-with-insecure-websocket.html: Add a call to webSocket.close() inside the onError handler.
* http/tests/security/mixedContent/websocket/insecure-websocket-in-iframe-expected.txt:
* http/tests/security/mixedContent/websocket/insecure-websocket-in-main-frame-expected.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (187555 => 187556)
--- trunk/LayoutTests/ChangeLog 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/LayoutTests/ChangeLog 2015-07-29 21:08:30 UTC (rev 187556)
@@ -1,3 +1,14 @@
+2015-07-29 Brady Eidson <[email protected]>
+
+ Crash calling webSocket.close() from onError handler for blocked web socket.
+ <rdar://problem/21771620> and https://bugs.webkit.org/show_bug.cgi?id=147411
+
+ Reviewed by Tim Horton.
+
+ * http/tests/security/mixedContent/resources/frame-with-insecure-websocket.html: Add a call to webSocket.close() inside the onError handler.
+ * http/tests/security/mixedContent/websocket/insecure-websocket-in-iframe-expected.txt:
+ * http/tests/security/mixedContent/websocket/insecure-websocket-in-main-frame-expected.txt:
+
2015-07-19 Matt Rajca <[email protected]>
Media Session: test 'Transient Solo' interruptions
Modified: trunk/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-websocket.html (187555 => 187556)
--- trunk/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-websocket.html 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-websocket.html 2015-07-29 21:08:30 UTC (rev 187556)
@@ -2,6 +2,8 @@
<script>
window.jsTestIsAsync = true;
+var ws;
+
function onSocketOpened() {
alert("WebSocket connection opened.");
finishJSTest();
@@ -9,6 +11,7 @@
function onSocketError() {
alert("WebSocket connection failed.");
+ ws.close();
finishJSTest();
}
@@ -18,7 +21,7 @@
}
try {
- var ws = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/echo");
+ ws = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/echo");
ws._onopen_ = onSocketOpened;
ws._onerror_ = onSocketError;
ws._onclose_ = onSocketClosed;
Modified: trunk/LayoutTests/http/tests/security/mixedContent/websocket/insecure-websocket-in-iframe-expected.txt (187555 => 187556)
--- trunk/LayoutTests/http/tests/security/mixedContent/websocket/insecure-websocket-in-iframe-expected.txt 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/LayoutTests/http/tests/security/mixedContent/websocket/insecure-websocket-in-iframe-expected.txt 2015-07-29 21:08:30 UTC (rev 187556)
@@ -1,6 +1,7 @@
-CONSOLE MESSAGE: line 21: [blocked] The page at https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-websocket.html was not allowed to run insecure content from ws://127.0.0.1:8880/websocket/tests/hybi/echo.
+CONSOLE MESSAGE: line 24: [blocked] The page at https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-websocket.html was not allowed to run insecure content from ws://127.0.0.1:8880/websocket/tests/hybi/echo.
ALERT: WebSocket connection failed.
+CONSOLE MESSAGE: line 14: WebSocket connection failed: WebSocket is closed before the connection is established.
This test loads an iframe that creates an insecure WebSocket connection. We should block the connection and trigger a mixed content callback because the main frame is HTTPS, but the data sent over the socket could be recorded or controlled by an attacker.
Modified: trunk/LayoutTests/http/tests/security/mixedContent/websocket/insecure-websocket-in-main-frame-expected.txt (187555 => 187556)
--- trunk/LayoutTests/http/tests/security/mixedContent/websocket/insecure-websocket-in-main-frame-expected.txt 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/LayoutTests/http/tests/security/mixedContent/websocket/insecure-websocket-in-main-frame-expected.txt 2015-07-29 21:08:30 UTC (rev 187556)
@@ -1,4 +1,5 @@
-CONSOLE MESSAGE: line 21: [blocked] The page at https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-websocket.html was not allowed to run insecure content from ws://127.0.0.1:8880/websocket/tests/hybi/echo.
+CONSOLE MESSAGE: line 24: [blocked] The page at https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-websocket.html was not allowed to run insecure content from ws://127.0.0.1:8880/websocket/tests/hybi/echo.
ALERT: WebSocket connection failed.
+CONSOLE MESSAGE: line 14: WebSocket connection failed: WebSocket is closed before the connection is established.
This test opens a window that connects to an insecure ws:// WebSocket. We should block the connection and trigger a mixed content callback because the main frame is HTTPS, but the data sent over the socket could be recorded or controlled by an attacker.
Modified: trunk/Source/WebCore/ChangeLog (187555 => 187556)
--- trunk/Source/WebCore/ChangeLog 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/Source/WebCore/ChangeLog 2015-07-29 21:08:30 UTC (rev 187556)
@@ -1,3 +1,24 @@
+2015-07-29 Brady Eidson <[email protected]>
+
+ Crash calling webSocket.close() from onError handler for blocked web socket.
+ <rdar://problem/21771620> and https://bugs.webkit.org/show_bug.cgi?id=147411
+
+ Reviewed by Tim Horton.
+
+ Tests: http/tests/security/mixedContent/websocket/insecure-websocket-in-iframe.html
+ http/tests/security/mixedContent/websocket/insecure-websocket-in-main-frame.html
+
+ This was introduced with http://trac.webkit.org/changeset/185848
+
+ * Modules/websockets/WebSocket.cpp:
+ (WebCore::WebSocket::connect): When blocked because of mixedContent, call dispatchOrQueueErrorEvent().
+ (WebCore::WebSocket::didReceiveMessageError): Use dispatchOrQueueErrorEvent() instead.
+ (WebCore::WebSocket::dispatchOrQueueErrorEvent): Dispatch the error event, but don't dispatch one twice!
+ * Modules/websockets/WebSocket.h:
+
+ * Modules/websockets/WebSocketChannel.cpp:
+ (WebCore::WebSocketChannel::fail): Null-check m_handshake before creating a console message from it.
+
2015-07-29 Michael Catanzaro <[email protected]>
Clean up RefPtrCairo.cpp
Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.cpp (187555 => 187556)
--- trunk/Source/WebCore/Modules/websockets/WebSocket.cpp 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.cpp 2015-07-29 21:08:30 UTC (rev 187556)
@@ -284,12 +284,13 @@
if (!document.frame()->loader().mixedContentChecker().canRunInsecureContent(document.securityOrigin(), m_url)) {
// Balanced by the call to ActiveDOMObject::unsetPendingActivity() in WebSocket::stop().
ActiveDOMObject::setPendingActivity(this);
+
// We must block this connection. Instead of throwing an exception, we indicate this
// using the error event. But since this code executes as part of the WebSocket's
// constructor, we have to wait until the constructor has completed before firing the
// event; otherwise, users can't connect to the event.
RunLoop::main().dispatch([this]() {
- dispatchEvent(Event::create(eventNames().errorEvent, false, false));
+ dispatchOrQueueErrorEvent();
stop();
});
return;
@@ -587,7 +588,7 @@
LOG(Network, "WebSocket %p didReceiveErrorMessage()", this);
m_state = CLOSED;
ASSERT(scriptExecutionContext());
- dispatchOrQueueEvent(Event::create(eventNames().errorEvent, false, false));
+ dispatchOrQueueErrorEvent();
}
void WebSocket::didUpdateBufferedAmount(unsigned long bufferedAmount)
@@ -638,6 +639,15 @@
return overhead;
}
+void WebSocket::dispatchOrQueueErrorEvent()
+{
+ if (m_dispatchedErrorEvent)
+ return;
+
+ m_dispatchedErrorEvent = true;
+ dispatchOrQueueEvent(Event::create(eventNames().errorEvent, false, false));
+}
+
void WebSocket::dispatchOrQueueEvent(Ref<Event>&& event)
{
if (m_shouldDelayEventFiring)
Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.h (187555 => 187556)
--- trunk/Source/WebCore/Modules/websockets/WebSocket.h 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.h 2015-07-29 21:08:30 UTC (rev 187556)
@@ -110,6 +110,7 @@
explicit WebSocket(ScriptExecutionContext&);
void resumeTimerFired();
+ void dispatchOrQueueErrorEvent();
void dispatchOrQueueEvent(Ref<Event>&&);
// ActiveDOMObject API.
@@ -143,6 +144,7 @@
Timer m_resumeTimer;
bool m_shouldDelayEventFiring { false };
Deque<Ref<Event>> m_pendingEvents;
+ bool m_dispatchedErrorEvent { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp (187555 => 187556)
--- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp 2015-07-29 20:30:08 UTC (rev 187555)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp 2015-07-29 21:08:30 UTC (rev 187556)
@@ -201,7 +201,14 @@
ASSERT(!m_suspended);
if (m_document) {
InspectorInstrumentation::didReceiveWebSocketFrameError(m_document, m_identifier, reason);
- m_document->addConsoleMessage(MessageSource::Network, MessageLevel::Error, "WebSocket connection to '" + m_handshake->url().stringCenterEllipsizedToLength() + "' failed: " + reason);
+
+ String consoleMessage;
+ if (m_handshake)
+ consoleMessage = makeString("WebSocket connection to '", m_handshake->url().stringCenterEllipsizedToLength(), "' failed: ", reason);
+ else
+ consoleMessage = makeString("WebSocket connection failed: ", reason);
+
+ m_document->addConsoleMessage(MessageSource::Network, MessageLevel::Error, consoleMessage);
}
// Hybi-10 specification explicitly states we must not continue to handle incoming data
@@ -218,7 +225,8 @@
if (m_handle && !m_closed)
m_handle->disconnect(); // Will call didClose().
- ASSERT(m_closed);
+ // We should be closed by now, but if we never got a handshake then we never even opened.
+ ASSERT(m_closed || !m_handshake);
}
void WebSocketChannel::disconnect()