Title: [279069] trunk
Revision
279069
Author
commit-qu...@webkit.org
Date
2021-06-21 11:15:04 -0700 (Mon, 21 Jun 2021)

Log Message

REGRESSION (r275496): WebSocket Message too long when message is larger than 1mb
https://bugs.webkit.org/show_bug.cgi?id=227030
<rdar://problem/79370994>

Patch by Alex Christensen <achristen...@webkit.org> on 2021-06-21
Reviewed by Youenn Fablet.

Source/WebKit:

NSURLSession's WebSocket implementation currently has maximumMessageSize's default value at 1MB.
Our CFReadStream-based WebSocket implementation had no message size limitation, so set it to 0 to remove the limit.

Writing a test for this was tricky because our WebSocket LayoutTests use the deflate extension.  I wrote an API test
that implements a simple WebSocket exchange and verifies each byte is what we expect.
The test used to fail using our NSURLSession-based WebSocket implementation but now passes everywhere.

* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(WebKit::NetworkSessionCocoa::createWebSocketTask):
Set maximumMessageSize to 0, which, as documented in ws_options.h but unfortunately not yet in NSURLSession.h:
"means there is no receive limit."
This matches our behavior with the CFReadStream-based WebSocket implementation.

Tools:

* TestWebKitAPI/SourcesCocoa.txt:
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/WebSocket.mm: Added.
(TestWebKitAPI::TEST):
* TestWebKitAPI/cocoa/HTTPServer.h:
* TestWebKitAPI/cocoa/HTTPServer.mm:
(TestWebKitAPI::Connection::receiveBytes const):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (279068 => 279069)


--- trunk/Source/WebKit/ChangeLog	2021-06-21 16:04:14 UTC (rev 279068)
+++ trunk/Source/WebKit/ChangeLog	2021-06-21 18:15:04 UTC (rev 279069)
@@ -1,3 +1,24 @@
+2021-06-21  Alex Christensen  <achristen...@webkit.org>
+
+        REGRESSION (r275496): WebSocket Message too long when message is larger than 1mb
+        https://bugs.webkit.org/show_bug.cgi?id=227030
+        <rdar://problem/79370994>
+
+        Reviewed by Youenn Fablet.
+
+        NSURLSession's WebSocket implementation currently has maximumMessageSize's default value at 1MB.
+        Our CFReadStream-based WebSocket implementation had no message size limitation, so set it to 0 to remove the limit.
+
+        Writing a test for this was tricky because our WebSocket LayoutTests use the deflate extension.  I wrote an API test
+        that implements a simple WebSocket exchange and verifies each byte is what we expect.
+        The test used to fail using our NSURLSession-based WebSocket implementation but now passes everywhere.
+
+        * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+        (WebKit::NetworkSessionCocoa::createWebSocketTask):
+        Set maximumMessageSize to 0, which, as documented in ws_options.h but unfortunately not yet in NSURLSession.h:
+        "means there is no receive limit."
+        This matches our behavior with the CFReadStream-based WebSocket implementation.
+
 2021-06-20  Wenson Hsieh  <wenson_hs...@apple.com>
 
         [macOS] Rename WKVisualSearchPreviewController to WKQuickLookPreviewController

Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm (279068 => 279069)


--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm	2021-06-21 16:04:14 UTC (rev 279068)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm	2021-06-21 18:15:04 UTC (rev 279069)
@@ -1672,6 +1672,7 @@
     [nsRequest _setProperty:@NO forKey:(NSString *)_kCFURLConnectionPropertyShouldSniff];
 
     RetainPtr<NSURLSessionWebSocketTask> task = [sessionSetForPage(webPageProxyID).sessionWithCredentialStorage.session webSocketTaskWithRequest:nsRequest.get()];
+    task.get().maximumMessageSize = 0;
     return makeUnique<WebSocketTask>(channel, webPageProxyID, request, WTFMove(task));
 }
 

Modified: trunk/Tools/ChangeLog (279068 => 279069)


--- trunk/Tools/ChangeLog	2021-06-21 16:04:14 UTC (rev 279068)
+++ trunk/Tools/ChangeLog	2021-06-21 18:15:04 UTC (rev 279069)
@@ -1,3 +1,19 @@
+2021-06-21  Alex Christensen  <achristen...@webkit.org>
+
+        REGRESSION (r275496): WebSocket Message too long when message is larger than 1mb
+        https://bugs.webkit.org/show_bug.cgi?id=227030
+        <rdar://problem/79370994>
+
+        Reviewed by Youenn Fablet.
+
+        * TestWebKitAPI/SourcesCocoa.txt:
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKitCocoa/WebSocket.mm: Added.
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/cocoa/HTTPServer.h:
+        * TestWebKitAPI/cocoa/HTTPServer.mm:
+        (TestWebKitAPI::Connection::receiveBytes const):
+
 2021-06-21  Jonathan Bedard  <jbed...@apple.com>
 
         [resultsdbpy] Adopt autoinstaller (Follow-up fix)

Modified: trunk/Tools/TestWebKitAPI/SourcesCocoa.txt (279068 => 279069)


--- trunk/Tools/TestWebKitAPI/SourcesCocoa.txt	2021-06-21 16:04:14 UTC (rev 279068)
+++ trunk/Tools/TestWebKitAPI/SourcesCocoa.txt	2021-06-21 18:15:04 UTC (rev 279069)
@@ -37,3 +37,4 @@
 Tests/WebKitCocoa/EventAttribution.mm
 Tests/WebKitCocoa/TestAwakener.mm
 Tests/WebKitCocoa/TLSDeprecation.mm
+Tests/WebKitCocoa/WebSocket.mm

Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (279068 => 279069)


--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2021-06-21 16:04:14 UTC (rev 279068)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2021-06-21 18:15:04 UTC (rev 279069)
@@ -2942,6 +2942,7 @@
 		DF4B273821A47727009BD1CA /* WKNSDictionaryEmptyDictionaryCrash.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKNSDictionaryEmptyDictionaryCrash.mm; sourceTree = "<group>"; };
 		DF6BC46F2534E120008F63CC /* TestDownloadDelegate.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = TestDownloadDelegate.mm; path = cocoa/TestDownloadDelegate.mm; sourceTree = "<group>"; };
 		DF6BC4702534E120008F63CC /* TestDownloadDelegate.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = TestDownloadDelegate.h; path = cocoa/TestDownloadDelegate.h; sourceTree = "<group>"; };
+		DF8A2E41267BEFAB00ED59DF /* WebSocket.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebSocket.mm; sourceTree = "<group>"; };
 		DFB80E352615124E002D4771 /* TestAwakener.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TestAwakener.h; sourceTree = "<group>"; };
 		DFB80E362615124E002D4771 /* TestAwakener.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = TestAwakener.mm; sourceTree = "<group>"; };
 		DFB8FF312492F51A00F00B0D /* Preconnect.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = Preconnect.mm; sourceTree = "<group>"; };
@@ -3619,6 +3620,7 @@
 				C1D8EE212028E8E3008EB141 /* WebProcessTerminate.mm */,
 				5120C83C1E6750790025B250 /* WebsiteDataStoreCustomPaths.mm */,
 				5C9E56841DF9143D00C9EE33 /* WebsitePolicies.mm */,
+				DF8A2E41267BEFAB00ED59DF /* WebSocket.mm */,
 				931C281C22BC55A7001D98C4 /* WebSQLBasics.mm */,
 				44C2FBE125E7592C00ABC72F /* WKAppHighlights.mm */,
 				2E5C77061FA70136005932C3 /* WKAttachmentTests.mm */,

Added: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebSocket.mm (0 => 279069)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebSocket.mm	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebSocket.mm	2021-06-21 18:15:04 UTC (rev 279069)
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2021 Apple 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:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. 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.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS 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 APPLE INC. OR ITS 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.
+ */
+
+#import "config.h"
+
+#import "HTTPServer.h"
+#import "PlatformUtilities.h"
+#import "TestUIDelegate.h"
+#import <wtf/RetainPtr.h>
+
+namespace TestWebKitAPI {
+
+TEST(WebSocket, LongMessageNoDeflate)
+{
+    // https://datatracker.ietf.org/doc/html/rfc6455#section-5.2
+    constexpr size_t headerSizeWithLargePayloadLength = 10;
+    constexpr size_t maskingKeySize = 4;
+    constexpr uint8_t payloadLengthIndicatingLargeExtendedPayloadLength = 127;
+    constexpr uint8_t fin = 0x80;
+    constexpr uint8_t textFrame = 0x1;
+
+    constexpr uint64_t twoMegabytes = 1024 * 1024 * 2;
+    constexpr size_t expectedReceiveSize = twoMegabytes + headerSizeWithLargePayloadLength + maskingKeySize;
+
+    HTTPServer server([=](Connection connection) {
+        connection.webSocketHandshake([=] {
+            connection.receiveBytes([=] (Vector<uint8_t>&& received) {
+                constexpr size_t headerSize = headerSizeWithLargePayloadLength + maskingKeySize;
+                EXPECT_EQ(expectedReceiveSize, received.size());
+                std::array<uint8_t, headerSizeWithLargePayloadLength> expectedHeader { 0x81, 0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x20, 0x0, 0x0, };
+                for (size_t i = 0 ; i < headerSizeWithLargePayloadLength; i++)
+                    EXPECT_EQ(received[i], expectedHeader[i]);
+                std::array<uint8_t, maskingKeySize> mask { received[10], received[11], received[12], received[13] };
+                for (size_t i = headerSize; i < expectedReceiveSize; i++)
+                    EXPECT_EQ(received[i] ^ mask[(i - headerSize) % maskingKeySize], 'x');
+
+                Vector<uint8_t> bytesToSend;
+                bytesToSend.reserveInitialCapacity(twoMegabytes + headerSizeWithLargePayloadLength);
+                bytesToSend.uncheckedAppend(fin | textFrame);
+                bytesToSend.uncheckedAppend(payloadLengthIndicatingLargeExtendedPayloadLength);
+                for (size_t i = 0; i < 8; i++)
+                    bytesToSend.uncheckedAppend((twoMegabytes >> (8 * (7 - i))) & 0xFF);
+                for (size_t i = 0; i < twoMegabytes; i++)
+                    bytesToSend.uncheckedAppend('x');
+
+                connection.send(WTFMove(bytesToSend));
+            }, expectedReceiveSize);
+        });
+    });
+
+    NSString *html = [NSString stringWithFormat:@""
+        "<script>"
+        "    let twoMegabytes = 2*1024*1024;"
+        "    var ws = new WebSocket('ws://127.0.0.1:%d/');"
+        "    ws._onopen_ = function() { this.send('x'.repeat(twoMegabytes)); };"
+        "    ws._onmessage_ = function(msg) { alert(msg.data.length == twoMegabytes ? 'PASS' : 'FAIL - wrong receive length'); };"
+        "    ws._onerror_ = function(error) { alert('FAIL - error ' + error.message); }"
+        "</script>", server.port()];
+    auto webView = adoptNS([WKWebView new]);
+    [webView loadHTMLString:html baseURL:nil];
+    EXPECT_WK_STREQ([webView _test_waitForAlert], "PASS");
+}
+
+}

Modified: trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h (279068 => 279069)


--- trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h	2021-06-21 16:04:14 UTC (rev 279068)
+++ trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h	2021-06-21 18:15:04 UTC (rev 279069)
@@ -69,7 +69,7 @@
     void send(String&&, CompletionHandler<void()>&& = nullptr) const;
     void send(Vector<uint8_t>&&, CompletionHandler<void()>&& = nullptr) const;
     void send(RetainPtr<dispatch_data_t>&&, CompletionHandler<void()>&& = nullptr) const;
-    void receiveBytes(CompletionHandler<void(Vector<uint8_t>&&)>&&) const;
+    void receiveBytes(CompletionHandler<void(Vector<uint8_t>&&)>&&, size_t minimumSize = 1) const;
     void receiveHTTPRequest(CompletionHandler<void(Vector<char>&&)>&&, Vector<char>&& buffer = { }) const;
     void webSocketHandshake(CompletionHandler<void()>&& = { });
     void terminate();

Modified: trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm (279068 => 279069)


--- trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm	2021-06-21 16:04:14 UTC (rev 279068)
+++ trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm	2021-06-21 18:15:04 UTC (rev 279069)
@@ -282,9 +282,9 @@
     return [NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:format, port(), path.utf8().data()]]];
 }
 
-void Connection::receiveBytes(CompletionHandler<void(Vector<uint8_t>&&)>&& completionHandler) const
+void Connection::receiveBytes(CompletionHandler<void(Vector<uint8_t>&&)>&& completionHandler, size_t minimumSize) const
 {
-    nw_connection_receive(m_connection.get(), 1, std::numeric_limits<uint32_t>::max(), makeBlockPtr([connection = *this, completionHandler = WTFMove(completionHandler)](dispatch_data_t content, nw_content_context_t, bool, nw_error_t error) mutable {
+    nw_connection_receive(m_connection.get(), minimumSize, std::numeric_limits<uint32_t>::max(), makeBlockPtr([connection = *this, completionHandler = WTFMove(completionHandler)](dispatch_data_t content, nw_content_context_t, bool, nw_error_t error) mutable {
         if (error || !content)
             return completionHandler({ });
         completionHandler(vectorFromData(content));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to