From 7146587b95ba66756e252528a12bcc96e47a05fa Mon Sep 17 00:00:00 2001
From: Aras Abbasi <aras.abbasi@googlemail.com>
Date: Tue, 27 Feb 2024 17:37:07 +0100
Subject: [PATCH] perf: improve performance of isValidSubprotocol (#2861)

---
 benchmarks/websocketIsValidSubprotocol.mjs | 17 +++++++++
 lib/web/websocket/util.js                  | 42 +++++++++++-----------
 test/websocket/util.js                     | 31 ++++++++++++++++
 3 files changed, 68 insertions(+), 22 deletions(-)
 create mode 100644 benchmarks/websocketIsValidSubprotocol.mjs
 create mode 100644 test/websocket/util.js

diff --git a/benchmarks/websocketIsValidSubprotocol.mjs b/benchmarks/websocketIsValidSubprotocol.mjs
new file mode 100644
index 00000000000..d4ab287ba41
--- /dev/null
+++ b/benchmarks/websocketIsValidSubprotocol.mjs
@@ -0,0 +1,17 @@
+import { bench, group, run } from 'mitata'
+import { isValidSubprotocol } from '../lib/web/websocket/util.js'
+
+const valid = 'valid'
+const invalid = 'invalid '
+
+group('isValidSubprotocol', () => {
+  bench(`valid: ${valid}`, () => {
+    return isValidSubprotocol(valid)
+  })
+
+  bench(`invalid: ${invalid}`, () => {
+    return isValidSubprotocol(invalid)
+  })
+})
+
+await run()
diff --git a/lib/web/websocket/util.js b/lib/web/websocket/util.js
index dc3e1ad7c3d..abe91be734d 100644
--- a/lib/web/websocket/util.js
+++ b/lib/web/websocket/util.js
@@ -119,31 +119,29 @@ function isValidSubprotocol (protocol) {
     return false
   }
 
-  for (const char of protocol) {
-    const code = char.charCodeAt(0)
+  for (let i = 0; i < protocol.length; ++i) {
+    const code = protocol.charCodeAt(i)
 
     if (
-      code < 0x21 ||
+      code < 0x21 || // CTL, contains SP (0x20) and HT (0x09)
       code > 0x7E ||
-      char === '(' ||
-      char === ')' ||
-      char === '<' ||
-      char === '>' ||
-      char === '@' ||
-      char === ',' ||
-      char === ';' ||
-      char === ':' ||
-      char === '\\' ||
-      char === '"' ||
-      char === '/' ||
-      char === '[' ||
-      char === ']' ||
-      char === '?' ||
-      char === '=' ||
-      char === '{' ||
-      char === '}' ||
-      code === 32 || // SP
-      code === 9 // HT
+      code === 0x22 || // "
+      code === 0x28 || // (
+      code === 0x29 || // )
+      code === 0x2C || // ,
+      code === 0x2F || // /
+      code === 0x3A || // :
+      code === 0x3B || // ;
+      code === 0x3C || // <
+      code === 0x3D || // =
+      code === 0x3E || // >
+      code === 0x3F || // ?
+      code === 0x40 || // @
+      code === 0x5B || // [
+      code === 0x5C || // \
+      code === 0x5D || // ]
+      code === 0x7B || // {
+      code === 0x7D // }
     ) {
       return false
     }
diff --git a/test/websocket/util.js b/test/websocket/util.js
new file mode 100644
index 00000000000..8dd0062c5cf
--- /dev/null
+++ b/test/websocket/util.js
@@ -0,0 +1,31 @@
+'use strict'
+
+const { tspl } = require('@matteo.collina/tspl')
+const { describe, test } = require('node:test')
+const { isValidSubprotocol } = require('../../lib/web/websocket/util')
+
+describe('isValidSubprotocol', () => {
+  test('empty string returns false', t => {
+    t = tspl(t, { plan: 1 })
+    t.strictEqual(isValidSubprotocol(''), false)
+  })
+
+  test('simple valid value returns false', t => {
+    t = tspl(t, { plan: 1 })
+    t.strictEqual(isValidSubprotocol('chat'), true)
+  })
+
+  test('empty string returns false', t => {
+    t = tspl(t, { plan: 1 })
+    t.strictEqual(isValidSubprotocol(''), false)
+  })
+
+  test('value with "(),/:;<=>?@[\\]{} returns false', t => {
+    const chars = '"(),/:;<=>?@[\\]{}'
+    t = tspl(t, { plan: 17 })
+
+    for (let i = 0; i < chars.length; ++i) {
+      t.strictEqual(isValidSubprotocol('valid' + chars[i]), false)
+    }
+  })
+})