Merge r230052 - WebSocket cookie incorrectly stored

https://bugs.webkit.org/show_bug.cgi?id=184100 <rdar://problem/37928715> Reviewed by Brent Fulgham. Source/WebCore: A cookie received in a WebSocket response should be stored with respect to the origin of the WebSocket server in order for it to be sent in a subsequent request. Also removed a FIXME about implementing support for the long since deprecated Set-Cookie2 header. Test: http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html * Modules/websockets/WebSocketChannel.cpp: (WebCore::WebSocketChannel::processBuffer): * Modules/websockets/WebSocketHandshake.h: LayoutTests: * http/tests/websocket/tests/hybi/cookie_wsh.py: Added. Downloaded from <https://github.com/w3c/pywebsocket/blob/b2e1d11086fdf00b33a0d30c504f227e7d4fa86b/src/example/cookie_wsh.py>. (_add_set_cookie): (web_socket_do_extra_handshake): (web_socket_transfer_data): * http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior-expected.txt: Added. * http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html: Added.
WebKit · Jun 11, 2018 · 2bc84b6 · 2bc84b6
1 parent 2d5f589
commit 2bc84b6
Show file tree

Hide file tree

Showing 7 changed files with 170 additions and 7 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2018-03-28  Daniel Bates  <dabates@apple.com>
+
+        WebSocket cookie incorrectly stored
+        https://bugs.webkit.org/show_bug.cgi?id=184100
+        <rdar://problem/37928715>
+
+        Reviewed by Brent Fulgham.
+
+        * http/tests/websocket/tests/hybi/cookie_wsh.py: Added. Downloaded from
+        <https://github.com/w3c/pywebsocket/blob/b2e1d11086fdf00b33a0d30c504f227e7d4fa86b/src/example/cookie_wsh.py>.
+        (_add_set_cookie):
+        (web_socket_do_extra_handshake):
+        (web_socket_transfer_data):
+        * http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior-expected.txt: Added.
+        * http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html: Added.
+
 2018-03-27  Daniel Bates  <dabates@apple.com>
 
         CSS mask images should be retrieved using potentially CORS-enabled fetch

diff --git a/LayoutTests/http/tests/websocket/tests/hybi/cookie_wsh.py b/LayoutTests/http/tests/websocket/tests/hybi/cookie_wsh.py
@@ -0,0 +1,53 @@
+# Copyright (C) 2014 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+from mod_pywebsocket import msgutil
+import urlparse
+
+
+def _add_set_cookie(request, value):
+    request.extra_headers.append(('Set-Cookie', value))
+
+def web_socket_do_extra_handshake(request):
+    components = urlparse.urlparse(request.uri)
+    command = components[4]
+
+    ONE_DAY_LIFE = 'Max-Age=86400'
+
+    if command == 'set':
+        _add_set_cookie(request, '; '.join(['foo=bar', ONE_DAY_LIFE]))
+    elif command == 'set_httponly':
+        _add_set_cookie(request,
+            '; '.join(['httpOnlyFoo=bar', ONE_DAY_LIFE, 'httpOnly']))
+    elif command == 'clear':
+        _add_set_cookie(request, 'foo=0; Max-Age=0')
+        _add_set_cookie(request, 'httpOnlyFoo=0; Max-Age=0')
+
+
+def web_socket_transfer_data(request):
+    pass
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior-expected.txt b/LayoutTests/http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior-expected.txt
@@ -0,0 +1,14 @@
+Tests WebSocket Set-Cookie overwriting behavior with respect to a document cookie.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Same origin WebSocket:
+PASS cookieValue is "foo=bar"
+
+Cross origin WebSocket:
+PASS cookieValue is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html b/LayoutTests/http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../../js-test-resources/js-test.js"></script>
+<script>
+window.jsTestIsAsync = true;
+
+var cookieValue;
+
+function clearCookie()
+{
+    document.cookie = "foo=0; Max-Age=0"; // The key "foo" must match the key used in the WebSocket Set-Cookie header.
+}
+
+function setCookieFromHost(host)
+{
+    var promise = new Promise(resolve => {
+        var websocket = new WebSocket(`ws://${host}:8880/websocket/tests/hybi/cookie?set`);
+        websocket.onclose = () => resolve();
+    });
+    return promise;
+}
+
+function echoCookie()
+{
+    return document.cookie;
+}
+
+async function testSameOriginCookie()
+{
+    clearCookie();
+    document.cookie = "foo=should_be_overwritten_by_websocket_set_cookie";
+    await setCookieFromHost("127.0.0.1");
+    cookieValue = echoCookie();
+    shouldBeEqualToString("cookieValue", "foo=bar");
+}
+
+async function testCrossOriginCookie()
+{
+    clearCookie();
+    await setCookieFromHost("localhost");
+    cookieValue = echoCookie();
+    shouldBeEmptyString("cookieValue");
+}
+
+async function runTests()
+{
+    debug("Same origin WebSocket:");
+    await testSameOriginCookie();
+    debug("<br>Cross origin WebSocket:");
+    await testCrossOriginCookie();
+    finishJSTest();
+}
+</script>
+</head>
+<body>
+<script>
+description("Tests WebSocket Set-Cookie overwriting behavior with respect to a document cookie.");
+runTests();
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2018-03-28  Daniel Bates  <dabates@apple.com>
+
+        WebSocket cookie incorrectly stored
+        https://bugs.webkit.org/show_bug.cgi?id=184100
+        <rdar://problem/37928715>
+
+        Reviewed by Brent Fulgham.
+
+        A cookie received in a WebSocket response should be stored with respect to the
+        origin of the WebSocket server in order for it to be sent in a subsequent request.
+
+        Also removed a FIXME about implementing support for the long since
+        deprecated Set-Cookie2 header.
+
+        Test: http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html
+
+        * Modules/websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::processBuffer):
+        * Modules/websockets/WebSocketHandshake.h:
+
 2018-03-27  Daniel Bates  <dabates@apple.com>
 
         Attempt to fix media control layout tests after <https://trac.webkit.org/changeset/230006/>

diff --git a/Source/WebCore/Modules/websockets/WebSocketChannel.cpp b/Source/WebCore/Modules/websockets/WebSocketChannel.cpp
@@ -443,13 +443,11 @@ bool WebSocketChannel::processBuffer()
         if (m_handshake->mode() == WebSocketHandshake::Connected) {
             if (m_identifier)
                 InspectorInstrumentation::didReceiveWebSocketHandshakeResponse(m_document, m_identifier, m_handshake->serverHandshakeResponse());
-            if (!m_handshake->serverSetCookie().isEmpty()) {
-                if (m_document && cookiesEnabled(*m_document)) {
-                    // Exception (for sandboxed documents) ignored.
-                    m_document->setCookie(m_handshake->serverSetCookie());
-                }
+            String serverSetCookie = m_handshake->serverSetCookie();
+            if (!serverSetCookie.isEmpty()) {
+                if (m_document && cookiesEnabled(*m_document))
+                    setCookies(*m_document, m_handshake->httpURLForAuthenticationAndCookies(), serverSetCookie);
             }
-            // FIXME: handle set-cookie2.
             LOG(Network, "WebSocketChannel %p Connected", this);
             skipBuffer(headerLength);
             m_client->didConnect();

diff --git a/Source/WebCore/Modules/websockets/WebSocketHandshake.h b/Source/WebCore/Modules/websockets/WebSocketHandshake.h
@@ -52,6 +52,7 @@ class WebSocketHandshake {
 
     const URL& url() const;
     void setURL(const URL&);
+    URL httpURLForAuthenticationAndCookies() const;
     const String host() const;
 
     const String& clientProtocol() const;
@@ -86,7 +87,6 @@ class WebSocketHandshake {
     static String getExpectedWebSocketAccept(const String& secWebSocketKey);
 
 private:
-    URL httpURLForAuthenticationAndCookies() const;
 
     int readStatusLine(const char* header, size_t headerLength, int& statusCode, String& statusText);