From 95d83a09d3d174aeda4ceaf68a5f4379e571fc44 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 10 Feb 2023 16:48:17 -0500 Subject: [PATCH 1/5] perf(http): remove allocations checking upgrade and connection header values --- cli/tests/unit/http_test.ts | 29 +++++++++++ ext/http/01_http.js | 96 +++++++++++++++++++++++++++++++++---- 2 files changed, 115 insertions(+), 10 deletions(-) diff --git a/cli/tests/unit/http_test.ts b/cli/tests/unit/http_test.ts index 73ff35e09a4e72..bb70bf9bc6f828 100644 --- a/cli/tests/unit/http_test.ts +++ b/cli/tests/unit/http_test.ts @@ -14,6 +14,11 @@ import { } from "./test_util.ts"; import { join } from "../../../test_util/std/path/mod.ts"; +const { + buildCaseInsensitiveCommaValueFinder, + // @ts-expect-error TypeScript (as of 3.7) does not support indexing namespaces by symbol +} = Deno[Deno.internal]; + async function writeRequestAndReadResponse(conn: Deno.Conn): Promise { const encoder = new TextEncoder(); const decoder = new TextDecoder(); @@ -2612,6 +2617,30 @@ Deno.test({ }, }); +Deno.test("case insensitive comma value finder", async (t) => { + const cases = /** @type {[string, boolean][]} */ ([ + ["websocket", true], + ["wEbSOcKET", true], + [",wEbSOcKET", true], + [",wEbSOcKET,", true], + [", wEbSOcKET ,", true], + ["test, wEbSOcKET ,", true], + ["test , wEbSOcKET ,", true], + ["test , wEbSOcKET", true], + ["test, asdf,web,wEbSOcKET", true], + ["test, asdf,web,wEbSOcKETs", false], + ["test, asdf,awebsocket,wEbSOcKETs", false], + ]); + + const findValue = buildCaseInsensitiveCommaValueFinder("websocket"); + for (const [input, expected] of cases) { + await t.step(input.toString(), () => { + const actual = findValue(input); + assertEquals(actual, expected); + }); + } +}); + async function httpServerWithErrorBody( listener: Deno.Listener, compression: boolean, diff --git a/ext/http/01_http.js b/ext/http/01_http.js index 1da371e8dd9f53..ad4a46cb751170 100644 --- a/ext/http/01_http.js +++ b/ext/http/01_http.js @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. const core = globalThis.Deno.core; +const internals = globalThis.__bootstrap.internals; const primordials = globalThis.__bootstrap.primordials; const { BadResourcePrototype, InterruptedPrototype, ops } = core; import * as webidl from "internal:deno_webidl/00_webidl.js"; @@ -40,6 +41,7 @@ import { } from "internal:deno_web/06_streams.js"; const { ArrayPrototypeIncludes, + ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeSome, Error, @@ -48,6 +50,7 @@ const { Set, SetPrototypeAdd, SetPrototypeDelete, + StringPrototypeCharCodeAt, StringPrototypeIncludes, StringPrototypeToLowerCase, StringPrototypeSplit, @@ -389,15 +392,13 @@ function createRespondWith( } const _ws = Symbol("[[associated_ws]]"); +const websocketCvf = buildCaseInsensitiveCommaValueFinder("websocket"); +const upgradeCvf = buildCaseInsensitiveCommaValueFinder("upgrade"); function upgradeWebSocket(request, options = {}) { const upgrade = request.headers.get("upgrade"); const upgradeHasWebSocketOption = upgrade !== null && - ArrayPrototypeSome( - StringPrototypeSplit(upgrade, ","), - (option) => - StringPrototypeToLowerCase(StringPrototypeTrim(option)) === "websocket", - ); + websocketCvf(upgrade); if (!upgradeHasWebSocketOption) { throw new TypeError( "Invalid Header: 'upgrade' header must contain 'websocket'", @@ -406,11 +407,7 @@ function upgradeWebSocket(request, options = {}) { const connection = request.headers.get("connection"); const connectionHasUpgradeOption = connection !== null && - ArrayPrototypeSome( - StringPrototypeSplit(connection, ","), - (option) => - StringPrototypeToLowerCase(StringPrototypeTrim(option)) === "upgrade", - ); + upgradeCvf(connection); if (!connectionHasUpgradeOption) { throw new TypeError( "Invalid Header: 'connection' header must contain 'Upgrade'", @@ -471,4 +468,83 @@ function upgradeHttp(req) { return req[_deferred].promise; } +// todo: what space char codes are supported? Maybe only a regular space is ok? +const spaceCharCodes = ArrayPrototypeMap( + StringPrototypeSplit( + "\u0009\u000B\u000C\u0020\u00A0\uFEFF\r\n\u2028\u2029", + "", + ), + (c) => c.charCodeAt(0), +); +const commaCharCode = StringPrototypeCharCodeAt(",", 0); + +/** Builds a case function that can be used to find a case insensitive + * value in some text that's separated by commas. + * + * This is done because it doesn't require any allocations. + * @param checkText {string} - The text to find. (ex. "websocket") + */ +function buildCaseInsensitiveCommaValueFinder(checkText) { + const charCodes = ArrayPrototypeMap( + StringPrototypeSplit( + StringPrototypeToLowerCase(checkText), + "", + ), + (c) => [c.charCodeAt(0), c.toUpperCase().charCodeAt(0)], + ); + /** @type {number} */ + let i; + /** @type {number} */ + let char; + + /** @param value {string} */ + return function (value) { + for (i = 0; i < value.length; i++) { + char = value.charCodeAt(i); + skipWhitespace(value); + + if (hasWord(value)) { + skipWhitespace(value); + if (i === value.length || char === commaCharCode) { + return true; + } + } else { + skipUntilComma(value); + } + } + + return false; + }; + + /** @param value {string} */ + function hasWord(value) { + for (const [cLower, cUpper] of charCodes) { + if (cLower === char || cUpper === char) { + char = StringPrototypeCharCodeAt(value, ++i); + } else { + return false; + } + } + return true; + } + + /** @param value {string} */ + function skipWhitespace(value) { + while (ArrayPrototypeIncludes(spaceCharCodes, char)) { + char = StringPrototypeCharCodeAt(value, ++i); + } + } + + /** @param value {string} */ + function skipUntilComma(value) { + while (char !== commaCharCode && i < value.length) { + char = StringPrototypeCharCodeAt(value, ++i); + } + } +} + +// Expose this function for unit tests +internals.buildCaseInsensitiveCommaValueFinder = + buildCaseInsensitiveCommaValueFinder; + export { _ws, HttpConn, upgradeHttp, upgradeWebSocket }; From 5d728f3d0f2b4f72606c6dd5f35f97af2865bde9 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 10 Feb 2023 20:38:06 -0500 Subject: [PATCH 2/5] Only account for " " spaces and not all whitespace since it's faster --- ext/http/01_http.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/ext/http/01_http.js b/ext/http/01_http.js index ad4a46cb751170..0963acf9caec90 100644 --- a/ext/http/01_http.js +++ b/ext/http/01_http.js @@ -468,14 +468,7 @@ function upgradeHttp(req) { return req[_deferred].promise; } -// todo: what space char codes are supported? Maybe only a regular space is ok? -const spaceCharCodes = ArrayPrototypeMap( - StringPrototypeSplit( - "\u0009\u000B\u000C\u0020\u00A0\uFEFF\r\n\u2028\u2029", - "", - ), - (c) => c.charCodeAt(0), -); +const spaceCharCode = StringPrototypeCharCodeAt(" ", 0); const commaCharCode = StringPrototypeCharCodeAt(",", 0); /** Builds a case function that can be used to find a case insensitive @@ -530,7 +523,7 @@ function buildCaseInsensitiveCommaValueFinder(checkText) { /** @param value {string} */ function skipWhitespace(value) { - while (ArrayPrototypeIncludes(spaceCharCodes, char)) { + while (char === spaceCharCode) { char = StringPrototypeCharCodeAt(value, ++i); } } From 1a2272a580ac0d435fe1108d847d1ce4da461775 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 10 Feb 2023 20:46:14 -0500 Subject: [PATCH 3/5] Lint --- ext/http/01_http.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/ext/http/01_http.js b/ext/http/01_http.js index 0963acf9caec90..84f32423f40e4f 100644 --- a/ext/http/01_http.js +++ b/ext/http/01_http.js @@ -43,7 +43,6 @@ const { ArrayPrototypeIncludes, ArrayPrototypeMap, ArrayPrototypePush, - ArrayPrototypeSome, Error, ObjectPrototypeIsPrototypeOf, SafeSetIterator, @@ -54,7 +53,6 @@ const { StringPrototypeIncludes, StringPrototypeToLowerCase, StringPrototypeSplit, - StringPrototypeTrim, Symbol, SymbolAsyncIterator, TypeError, From 9158317231f96f996c886930ca59565aef4e9d31 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 12 Feb 2023 15:08:31 -0500 Subject: [PATCH 4/5] Update to support tab --- cli/tests/unit/http_test.ts | 2 +- ext/http/01_http.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cli/tests/unit/http_test.ts b/cli/tests/unit/http_test.ts index bb70bf9bc6f828..57039984111370 100644 --- a/cli/tests/unit/http_test.ts +++ b/cli/tests/unit/http_test.ts @@ -2625,7 +2625,7 @@ Deno.test("case insensitive comma value finder", async (t) => { [",wEbSOcKET,", true], [", wEbSOcKET ,", true], ["test, wEbSOcKET ,", true], - ["test , wEbSOcKET ,", true], + ["test ,\twEbSOcKET\t\t ,", true], ["test , wEbSOcKET", true], ["test, asdf,web,wEbSOcKET", true], ["test, asdf,web,wEbSOcKETs", false], diff --git a/ext/http/01_http.js b/ext/http/01_http.js index 84f32423f40e4f..76556c0c8a5045 100644 --- a/ext/http/01_http.js +++ b/ext/http/01_http.js @@ -466,7 +466,10 @@ function upgradeHttp(req) { return req[_deferred].promise; } -const spaceCharCode = StringPrototypeCharCodeAt(" ", 0); +const spaceCharCodes = ArrayPrototypeMap( + [" ", "\t"], + (c) => c.charCodeAt(0), +); const commaCharCode = StringPrototypeCharCodeAt(",", 0); /** Builds a case function that can be used to find a case insensitive @@ -521,7 +524,7 @@ function buildCaseInsensitiveCommaValueFinder(checkText) { /** @param value {string} */ function skipWhitespace(value) { - while (char === spaceCharCode) { + while (ArrayPrototypeIncludes(spaceCharCodes, char)) { char = StringPrototypeCharCodeAt(value, ++i); } } From e46732ba248feee5b3d1f04af82e020adc727239 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 12 Feb 2023 15:22:42 -0500 Subject: [PATCH 5/5] Explicit space and tab check --- ext/http/01_http.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ext/http/01_http.js b/ext/http/01_http.js index 76556c0c8a5045..f9e15e7d59e6c1 100644 --- a/ext/http/01_http.js +++ b/ext/http/01_http.js @@ -466,10 +466,8 @@ function upgradeHttp(req) { return req[_deferred].promise; } -const spaceCharCodes = ArrayPrototypeMap( - [" ", "\t"], - (c) => c.charCodeAt(0), -); +const spaceCharCode = StringPrototypeCharCodeAt(" ", 0); +const tabCharCode = StringPrototypeCharCodeAt("\t", 0); const commaCharCode = StringPrototypeCharCodeAt(",", 0); /** Builds a case function that can be used to find a case insensitive @@ -524,7 +522,7 @@ function buildCaseInsensitiveCommaValueFinder(checkText) { /** @param value {string} */ function skipWhitespace(value) { - while (ArrayPrototypeIncludes(spaceCharCodes, char)) { + while (char === spaceCharCode || char === tabCharCode) { char = StringPrototypeCharCodeAt(value, ++i); } }