WebSocket cookie incorrectly stored
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Mar 2018 21:27:44 +0000 (21:27 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Mar 2018 21:27:44 +0000 (21:27 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/websocket/tests/hybi/cookie_wsh.py [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hybi/websocket-cookie-overwrite-behavior.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/websockets/WebSocketChannel.cpp
Source/WebCore/Modules/websockets/WebSocketHandshake.h

index 1b79bb7..7629a3c 100644 (file)
@@ -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-28  Matt Lewis  <jlewis3@apple.com>
 
         Skipped imported/mozilla/css-animations/test_keyframeeffect-getkeyframes.html.
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/cookie_wsh.py b/LayoutTests/http/tests/websocket/tests/hybi/cookie_wsh.py
new file mode 100644 (file)
index 0000000..69b6a0e
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..e9b1ad7
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..ac4f749
--- /dev/null
@@ -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>
index 8d9fe0a..e0dc146 100644 (file)
@@ -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-28  Chris Dumez  <cdumez@apple.com>
 
         Do process swap when opening a cross-origin URL via window.open(url, '_blank', 'noopener')
index 1d92e77..e9a4f6d 100644 (file)
@@ -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();
index 0b9030e..b66c10f 100644 (file)
@@ -52,6 +52,7 @@ public:
 
     const URL& url() const;
     void setURL(const URL&);
+    URL httpURLForAuthenticationAndCookies() const;
     const String host() const;
 
     const String& clientProtocol() const;
@@ -86,7 +87,6 @@ public:
     static String getExpectedWebSocketAccept(const String& secWebSocketKey);
 
 private:
-    URL httpURLForAuthenticationAndCookies() const;
 
     int readStatusLine(const char* header, size_t headerLength, int& statusCode, String& statusText);