Lower per-process websocket connection limits to match other browser …

…behavior https://bugs.webkit.org/show_bug.cgi?id=247100 <rdar://90340929> Reviewed by Chris Dumez. Chrome and Firefox place an upper bound on the number of connections that a single web page may have open at the same time. We should follow this behavior to avoid a poorly written web page impacting system performance. We will use Chrome's larger limit to reduce the likelihood of web compatibility issues. Tested by a new test: http/tests/websocket/tests/hybi/multiple-connections-limit.html * LayoutTests/http/tests/websocket/tests/hybi/multiple-connections-limit-expected.txt: Added. * LayoutTests/http/tests/websocket/tests/hybi/multiple-connections-limit.html: Added. * Source/WebKit/WebProcess/Network/WebSocketChannel.cpp: (WebKit::WebSocketChannel::connect): Check open connections and fail the connection attempt if we exceed them. * Source/WebKit/WebProcess/Network/WebSocketChannelManager.h: (WebKit::WebSocketChannelManager::hasReachedSocketLimit const): Added. Canonical link: https://commits.webkit.org/258488@main
WebKit · Jan 5, 2023 · 80f4f4e · 80f4f4e
1 parent 9e2245f
commit 80f4f4e
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 1 deletion.
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/multiple-connections-limit-expected.txt b/LayoutTests/http/tests/websocket/tests/hybi/multiple-connections-limit-expected.txt
@@ -0,0 +1,3 @@
+Test that WebSocket is not subject to HTTP connection limit, but does not exceed a maximum. It should not contain any alerts about unexpected close events, and should say PASS below:
+
+PASS
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/multiple-connections-limit.html b/LayoutTests/http/tests/websocket/tests/hybi/multiple-connections-limit.html
@@ -0,0 +1,66 @@
+<!DOCTYPE html><!-- webkit-test-runner [ dumpJSConsoleLogInStdErr=true ] -->
+<html>
+<body>
+<p>Test that WebSocket is not subject to HTTP connection limit, but does not exceed a maximum. It should not contain any alerts about unexpected close events, and should say PASS below:</p>
+<p id=result>Running...</p>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+    testRunner.setAllowsAnySSLCertificate(true);
+}
+
+let maxSocketCount = 200;
+let totalAttemptedCount = 201;
+
+var result = document.getElementById("result");
+var sockets = [];
+
+function openSocket() {
+    var num = parseInt(result.innerHTML);
+    if (!num)
+        num = 1;
+    if (num != maxSocketCount - 1)
+        result.innerHTML = num + 1;
+    else {
+        result.innerHTML = "PASS";
+
+        for (j = 0; j < sockets.length; ++j) {
+            sockets[j].onclose = null;
+            sockets[j].close();
+        }
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }        
+}
+
+let firstHalf = Math.round(totalAttemptedCount / 2);
+
+// Our Python socket test server only allows 256 total open files (including sockets), and some
+// are used for log files, etc. So we can't test a 256-socket limit with a single process.
+// Therefore, split the test so that half go to the WS socket server, and half to the WSS server.
+for (i = 0; i < firstHalf; ++i) {
+    var ws = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/echo");
+    sockets[i] = ws;
+    ws.onopen = openSocket;
+    ws.onclose = function() {
+        var constructionTimeI = i;
+        if (constructionTimeI < maxSocketCount)
+            alert("FAIL: unexpected close event")
+    }
+}
+
+for (i = firstHalf; i < totalAttemptedCount; ++i) {
+    var ws = new WebSocket("wss://127.0.0.1:9323/websocket/tests/hybi/simple");
+    sockets[i] = ws;
+    ws.onopen = openSocket;
+    ws.onclose = function() {
+        var constructionTimeI = i;
+        if (constructionTimeI < maxSocketCount)
+            alert("FAIL: unexpected close event")
+    }
+}
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
@@ -3813,4 +3813,7 @@ webkit.org/b/249367 imported/w3c/web-platform-tests/html/canvas/offscreen/manual
 
 webkit.org/b/249407 media/audioSession/getUserMedia.html [ Failure ]
 
+# Per-process limit is only for WK2
+http/tests/websocket/tests/hybi/multiple-connections-limit.html [ Skip ]
+
 webkit.org/b/249486 media/audioSession/audioSessionType.html [ Failure ]
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -2594,3 +2594,6 @@ webkit.org/b/248493 http/tests/security/contentSecurityPolicy/script-src-parsing
 # https://bugs.webkit.org/show_bug.cgi?id=249544
 # Generic Permissions API is not supported in WK1
 imported/w3c/web-platform-tests/geolocation-API/permission.https.html [ WontFix ]
+
+# Per-process limit is only for WK2
+http/tests/websocket/tests/hybi/multiple-connections-limit.html [ Skip ]
diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
@@ -5244,3 +5244,6 @@ fast/frames/consume_transient_activation.html [ Skip ]
 fast/canvas/offscreen-enabled.html [ Skip ]
 http/wpt/offscreen-canvas [ Skip ]
 imported/w3c/web-platform-tests/html/canvas/offscreen [ Skip ]
+
+# Per-process limit is only for WK2
+http/tests/websocket/tests/hybi/multiple-connections-limit.html [ Skip ]
diff --git a/Source/WebKit/WebProcess/Network/WebSocketChannel.cpp b/Source/WebKit/WebProcess/Network/WebSocketChannel.cpp
@@ -113,6 +113,14 @@ WebSocketChannel::ConnectStatus WebSocketChannel::connect(const URL& url, const
     if (!m_document)
         return ConnectStatus::KO;
 
+    if (WebProcess::singleton().webSocketChannelManager().hasReachedSocketLimit()) {
+        auto reason = "Connection failed: Insufficient resources"_s;
+        logErrorMessage(reason);
+        if (m_client)
+            m_client->didReceiveMessageError(String { reason });
+        return ConnectStatus::KO;
+    }
+
     auto request = webSocketConnectRequest(*m_document, url);
     if (!request)
         return ConnectStatus::KO;

diff --git a/Source/WebKit/WebProcess/Network/WebSocketChannelManager.h b/Source/WebKit/WebProcess/Network/WebSocketChannelManager.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2019-2022 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -41,6 +41,11 @@ namespace WebKit {
 
 class WebSocketChannelManager {
 public:
+    // Choose a per-process limit that matches Firefox and Tor's global count (200),
+    // and Brave's per-process limit (50). Chrome has a global limit of 256, so
+    // any compatibility risk with Chrome should be very low.
+    static constexpr size_t maximumSocketCount = 200;
+
     WebSocketChannelManager() = default;
 
     void networkProcessCrashed();
@@ -49,6 +54,8 @@ class WebSocketChannelManager {
     void addChannel(WebSocketChannel&);
     void removeChannel(WebSocketChannel& channel) { m_channels.remove(channel.identifier() ); }
 
+    bool hasReachedSocketLimit() const { return m_channels.size() >= maximumSocketCount; }
+
 private:
     HashMap<WebCore::WebSocketIdentifier, WeakPtr<WebSocketChannel>> m_channels;
 };