Changeset 109840 in webkit

Timestamp:

Mar 5, 2012 8:20:47 PM (12 years ago)

Author:

bashi@chromium.org

Message:

[WebSocket] Should raise SYNTAX_ERR when message contains unpaired surrogates
​https://bugs.webkit.org/show_bug.cgi?id=80103

Reviewed by Kent Tamura.

Source/WebCore:

Add UTF8 validation checks for WebSocket message and close reason.

Tests: http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason.html

http/tests/websocket/tests/hybi/unpaired-surrogates-in-message.html

• Modules/websockets/WebSocket.cpp:

(WebCore::WebSocket::send): Raise SYNTAX_ERR if the message is invalid.
(WebCore::WebSocket::close):Raise SYNTAX_ERR if the reason is invalid.

• Modules/websockets/WebSocketChannel.cpp:

(WebCore::WebSocketChannel::send): Check whether message is a valid UTF8 string.

LayoutTests:

Added tests for unpaired surrogates check for WebSocket message and close reason.
Updated two expectations for close() tests because further error message is added.

• http/tests/websocket/tests/hybi/close-expected.txt: Updated.
• http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason-expected.txt: Added.
• http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason.html: Added.
• http/tests/websocket/tests/hybi/unpaired-surrogates-in-message-expected.txt: Added.
• http/tests/websocket/tests/hybi/unpaired-surrogates-in-message.html: Added.
• http/tests/websocket/tests/hybi/workers/close-expected.txt: Updated.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/websocket/tests/hybi/close-expected.txt (modified) (1 diff)
• LayoutTests/http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason-expected.txt (added)
• LayoutTests/http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason.html (added)
• LayoutTests/http/tests/websocket/tests/hybi/unpaired-surrogates-in-message-expected.txt (added)
• LayoutTests/http/tests/websocket/tests/hybi/unpaired-surrogates-in-message.html (added)
• LayoutTests/http/tests/websocket/tests/hybi/workers/close-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/websockets/WebSocket.cpp (modified) (2 diffs)
• Source/WebCore/Modules/websockets/WebSocketChannel.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-03-05  Kenichi Ishibashi  <bashi@chromium.org>
+
+        [WebSocket] Should raise SYNTAX_ERR when message contains unpaired surrogates
+        https://bugs.webkit.org/show_bug.cgi?id=80103
+
+        Reviewed by Kent Tamura.
+
+        Added tests for unpaired surrogates check for WebSocket message and close reason.
+        Updated two expectations for close() tests because further error message is added.
+
+        * http/tests/websocket/tests/hybi/close-expected.txt: Updated.
+        * http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason-expected.txt: Added.
+        * http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason.html: Added.
+        * http/tests/websocket/tests/hybi/unpaired-surrogates-in-message-expected.txt: Added.
+        * http/tests/websocket/tests/hybi/unpaired-surrogates-in-message.html: Added.
+        * http/tests/websocket/tests/hybi/workers/close-expected.txt: Updated.
+
 2012-03-05  Yoshifumi Inoue  <yosin@chromium.org>
 

trunk/LayoutTests/http/tests/websocket/tests/hybi/close-expected.txt

@@ -1 +1 @@
 CONSOLE MESSAGE: WebSocket is closed before the connection is established.
+CONSOLE MESSAGE: WebSocket close message is too long.
+CONSOLE MESSAGE: WebSocket close message is too long.
 CONSOLE MESSAGE: WebSocket is closed before the connection is established.
 Verify WebSocket::close behaviors.

trunk/LayoutTests/http/tests/websocket/tests/hybi/workers/close-expected.txt

@@ -1 +1 @@
 CONSOLE MESSAGE: WebSocket is closed before the connection is established.
+CONSOLE MESSAGE: WebSocket close message is too long.
+CONSOLE MESSAGE: WebSocket close message is too long.
 CONSOLE MESSAGE: WebSocket is closed before the connection is established.
 Verify WebSocket::close behaviors in Worker.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-05  Kenichi Ishibashi  <bashi@chromium.org>
+
+        [WebSocket] Should raise SYNTAX_ERR when message contains unpaired surrogates
+        https://bugs.webkit.org/show_bug.cgi?id=80103
+
+        Reviewed by Kent Tamura.
+
+        Add UTF8 validation checks for WebSocket message and close reason.
+
+        Tests: http/tests/websocket/tests/hybi/unpaired-surrogates-in-close-reason.html
+               http/tests/websocket/tests/hybi/unpaired-surrogates-in-message.html
+
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::send): Raise SYNTAX_ERR if the message is invalid.
+        (WebCore::WebSocket::close):Raise SYNTAX_ERR if the reason is invalid.
+        * Modules/websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::send): Check whether message is a valid UTF8 string.
+
 2012-03-05  Kenneth Russell  <kbr@google.com>
 

trunk/Source/WebCore/Modules/websockets/WebSocket.cpp

@@ -289 +289 @@
         return false;
     }
-    // FIXME: check message is valid utf8.
     ASSERT(m_channel);
-    return m_channel->send(message) == ThreadableWebSocketChannel::SendSuccess;
+    ThreadableWebSocketChannel::SendResult result = m_channel->send(message);
+    if (result == ThreadableWebSocketChannel::InvalidMessage) {
+        scriptExecutionContext()->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Websocket message contains invalid character(s).");
+        ec = SYNTAX_ERR;
+        return false;
+    }
+    return result == ThreadableWebSocketChannel::SendSuccess;
 }
 
@@ -344 +349 @@
             return;
         }
-        // FIXME: if reason contains any unpaired surrogates, raise SYNTAX_ERR.
-        if (reason.utf8().length() > maxReasonSizeInBytes) {
+        CString utf8 = reason.utf8(true);
+        if (utf8.length() > maxReasonSizeInBytes) {
+            scriptExecutionContext()->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "WebSocket close message is too long.");
+            ec = SYNTAX_ERR;
+            return;
+        }
+        // Checks whether reason is valid utf8.
+        if (utf8.isNull() && reason.length()) {
+            scriptExecutionContext()->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "WebSocket close message contains invalid character(s).");
             ec = SYNTAX_ERR;
             return;

trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp

@@ -161 +161 @@
 {
     LOG(Network, "WebSocketChannel %p send %s", this, message.utf8().data());
-    CString utf8 = message.utf8();
+    CString utf8 = message.utf8(true);
+    if (utf8.isNull() && message.length())
+        return InvalidMessage;
     if (m_useHixie76Protocol) {
         return sendFrameHixie76(utf8.data(), utf8.length()) ? ThreadableWebSocketChannel::SendSuccess : ThreadableWebSocketChannel::SendFail;