WebSocket: Split Hixie76 length-prefixed frames aren't handled correctly
authoryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Nov 2011 06:21:35 +0000 (06:21 +0000)
committeryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Nov 2011 06:21:35 +0000 (06:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=68522

Reviewed by Kent Tamura.

Source/WebCore:

Test: http/tests/websocket/tests/hixie76/split-binary-frame-header.html

* websockets/WebSocketChannel.cpp:
(WebCore::WebSocketChannel::processFrameHixie76):
Do not consume the data if the length field is not finished.

LayoutTests:

* http/tests/websocket/tests/hixie76/split-binary-frame-header-expected.txt: Added.
* http/tests/websocket/tests/hixie76/split-binary-frame-header.html: Added.
* http/tests/websocket/tests/hixie76/split-binary-frame-header_wsh.py:
Added. Send WebSocket frames that older WebSocketChannel implementation fails to parse.
The client should not receive an empty text message.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@101326 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header.html [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header_wsh.py [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/websockets/WebSocketChannel.cpp

index 2c0c5e8..efa69fc 100644 (file)
@@ -1,3 +1,16 @@
+2011-11-28  Yuta Kitamura  <yutak@chromium.org>
+
+        WebSocket: Split Hixie76 length-prefixed frames aren't handled correctly
+        https://bugs.webkit.org/show_bug.cgi?id=68522
+
+        Reviewed by Kent Tamura.
+
+        * http/tests/websocket/tests/hixie76/split-binary-frame-header-expected.txt: Added.
+        * http/tests/websocket/tests/hixie76/split-binary-frame-header.html: Added.
+        * http/tests/websocket/tests/hixie76/split-binary-frame-header_wsh.py:
+        Added. Send WebSocket frames that older WebSocketChannel implementation fails to parse.
+        The client should not receive an empty text message.
+
 2011-11-28  Stephen White  <senorblanco@chromium.org>
 
         [chromium] Fix SVG filters when running in accelerated drawing mode.
diff --git a/LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header-expected.txt b/LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header-expected.txt
new file mode 100644 (file)
index 0000000..7c86f90
--- /dev/null
@@ -0,0 +1,13 @@
+WebSocket: Hixie76 binary frames should be handled correctly even if frame header is split into multiple TCP packets.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+Connected.
+Error occurred.
+Closed.
+PASS closeEvent.wasClean is true
+PASS errorCount is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header.html b/LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header.html
new file mode 100644 (file)
index 0000000..8b47839
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../../js-test-resources/js-test-pre.js"></script>
+</head>
+<body>
+<div id="description"></div>
+<div id="console"></div>
+<script type="text/javascript">
+description("WebSocket: Hixie76 binary frames should be handled correctly even if frame header is split into multiple TCP packets.");
+
+window.jsTestIsAsync = true;
+
+var ws = new WebSocket("ws://localhost:8880/websocket/tests/hixie76/split-binary-frame-header");
+var errorCount = 0;
+
+ws.onopen = function()
+{
+    debug("Connected.");
+};
+
+ws.onmessage = function(messageEvent)
+{
+    testFailed("Should not receive message but received: \"" + messageEvent.data + "\"");
+};
+
+ws.onerror = function()
+{
+    debug("Error occurred.");
+    errorCount += 1;
+};
+
+var closeEvent;
+
+ws.onclose = function(event)
+{
+    debug("Closed.");
+    closeEvent = event;
+    shouldBeTrue("closeEvent.wasClean");
+    shouldEvaluateTo("errorCount", 1);
+
+    finishJSTest();
+};
+</script>
+<script src="../../../../js-test-resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header_wsh.py b/LayoutTests/http/tests/websocket/tests/hixie76/split-binary-frame-header_wsh.py
new file mode 100644 (file)
index 0000000..bbbf83d
--- /dev/null
@@ -0,0 +1,22 @@
+import time
+
+
+def web_socket_do_extra_handshake(request):
+    pass
+
+
+def web_socket_transfer_data(request):
+    # First frame (1 - 2 bytes): Zero-length binary frame with frame type 0x80.
+    # Second frame (3 - 4 bytes): Close frame.
+    data = '\x80\x00\xff\x00'
+
+    # There is a bug that WebSocketChannel incorrectly consumes the first byte of a binary frame header
+    # when it is split from the entire frame data. When this happens, the client recognizes the second and
+    # third bytes ('\x00\xff') as a valid text frame and invokes a message event (which should not happen
+    # in a valid client).
+
+    request.connection.write(data[:1])
+    time.sleep(0.5) # Wait for 500ms to make sure the first octet is sent out.
+    request.connection.write(data[1:])
+
+    raise handshake.AbortedByUserException('Abort the connection')
index 5c12a6d..9a2a4f4 100644 (file)
@@ -1,3 +1,16 @@
+2011-11-28  Yuta Kitamura  <yutak@chromium.org>
+
+        WebSocket: Split Hixie76 length-prefixed frames aren't handled correctly
+        https://bugs.webkit.org/show_bug.cgi?id=68522
+
+        Reviewed by Kent Tamura.
+
+        Test: http/tests/websocket/tests/hixie76/split-binary-frame-header.html
+
+        * websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::processFrameHixie76):
+        Do not consume the data if the length field is not finished.
+
 2011-11-28  Stephen White  <senorblanco@chromium.org>
 
         [chromium] Fix SVG filters when running in accelerated drawing mode.
index 96226b7..c70ccc3 100644 (file)
@@ -759,6 +759,7 @@ bool WebSocketChannel::processFrameHixie76()
     if ((frameByte & 0x80) == 0x80) {
         size_t length = 0;
         bool errorFrame = false;
+        bool lengthFinished = false;
         while (p < end) {
             if (length > numeric_limits<size_t>::max() / 128) {
                 LOG(Network, "frame length overflow %lu", static_cast<unsigned long>(length));
@@ -781,9 +782,13 @@ bool WebSocketChannel::processFrameHixie76()
             }
             length = newLength;
             ++p;
-            if (!(msgByte & 0x80))
+            if (!(msgByte & 0x80)) {
+                lengthFinished = true;
                 break;
+            }
         }
+        if (!errorFrame && !lengthFinished)
+            return false;
         if (p + length < p) {
             LOG(Network, "frame buffer pointer wrap %p+%lu->%p", p, static_cast<unsigned long>(length), p + length);
             errorFrame = true;