From 007d3171aef64a2bee64cf18539e8eb3a8dfda5a Mon Sep 17 00:00:00 2001 From: uzlopak Date: Tue, 20 Feb 2024 01:59:34 +0100 Subject: [PATCH 1/3] chore: improve getFieldValue --- benchmarks/cacheGetFieldValues.mjs | 19 +++++++++++++++++++ lib/cache/cache.js | 2 +- lib/cache/util.js | 18 ++++++++++-------- lib/fetch/util.js | 4 +--- test/cache/get-field-values.js | 20 ++++++++++++++++++++ 5 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 benchmarks/cacheGetFieldValues.mjs create mode 100644 test/cache/get-field-values.js diff --git a/benchmarks/cacheGetFieldValues.mjs b/benchmarks/cacheGetFieldValues.mjs new file mode 100644 index 00000000000..2a5894bd2ad --- /dev/null +++ b/benchmarks/cacheGetFieldValues.mjs @@ -0,0 +1,19 @@ +import { bench, group, run } from 'mitata' +import { getFieldValues } from '../lib/cache/util.js' + +const values = [ + '', + 'foo', + 'invälid', + 'foo, ' +] + +group('getFieldValues', () => { + bench('getFieldValues', () => { + for (let i = 0; i < values.length; ++i) { + getFieldValues(values[i]) + } + }) +}) + +await run() diff --git a/lib/cache/cache.js b/lib/cache/cache.js index d49393d15bc..74cd802de7f 100644 --- a/lib/cache/cache.js +++ b/lib/cache/cache.js @@ -1,7 +1,7 @@ 'use strict' const { kConstruct } = require('./symbols') -const { urlEquals, fieldValues: getFieldValues } = require('./util') +const { urlEquals, getFieldValues } = require('./util') const { kEnumerableProperty, isDisturbed } = require('../core/util') const { webidl } = require('../fetch/webidl') const { Response, cloneResponse, fromInnerResponse } = require('../fetch/response') diff --git a/lib/cache/util.js b/lib/cache/util.js index eba8a60efa3..745c5b04ce7 100644 --- a/lib/cache/util.js +++ b/lib/cache/util.js @@ -19,25 +19,27 @@ function urlEquals (A, B, excludeFragment = false) { return serializedA === serializedB } +const emptyArray = Object.freeze([]) + /** * @see https://github.com/chromium/chromium/blob/694d20d134cb553d8d89e5500b9148012b1ba299/content/browser/cache_storage/cache_storage_cache.cc#L260-L262 * @param {string} header */ -function fieldValues (header) { +function getFieldValues (header) { assert(header !== null) + if (header === '') { + return emptyArray + } + const values = [] for (let value of header.split(',')) { value = value.trim() - if (!value.length) { - continue - } else if (!isValidHeaderName(value)) { - continue + if (isValidHeaderName(value)) { + values.push(value) } - - values.push(value) } return values @@ -45,5 +47,5 @@ function fieldValues (header) { module.exports = { urlEquals, - fieldValues + getFieldValues } diff --git a/lib/fetch/util.js b/lib/fetch/util.js index fdda578213e..c07db2338a4 100644 --- a/lib/fetch/util.js +++ b/lib/fetch/util.js @@ -111,9 +111,7 @@ function isValidReasonPhrase (statusText) { * @see https://fetch.spec.whatwg.org/#header-name * @param {string} potentialValue */ -function isValidHeaderName (potentialValue) { - return isValidHTTPToken(potentialValue) -} +const isValidHeaderName = isValidHTTPToken /** * @see https://fetch.spec.whatwg.org/#header-value diff --git a/test/cache/get-field-values.js b/test/cache/get-field-values.js new file mode 100644 index 00000000000..e9287066fbb --- /dev/null +++ b/test/cache/get-field-values.js @@ -0,0 +1,20 @@ +'use strict' + +const { deepStrictEqual, throws, strictEqual } = require('node:assert') +const { test } = require('node:test') +const { getFieldValues } = require('../../lib/cache/util') + +test('getFieldValues', () => { + throws(() => getFieldValues(null), { + name: 'AssertionError', + message: 'The expression evaluated to a falsy value:\n\n assert(header !== null)\n' + }) + deepStrictEqual(getFieldValues(''), []) + strictEqual(Object.isFrozen(getFieldValues('')), true) + deepStrictEqual(getFieldValues('foo'), ['foo']) + deepStrictEqual(getFieldValues('invälid'), []) + deepStrictEqual(getFieldValues('foo, bar'), ['foo', 'bar']) + deepStrictEqual(getFieldValues('foo, bar, baz'), ['foo', 'bar', 'baz']) + deepStrictEqual(getFieldValues('foo, bar, baz, '), ['foo', 'bar', 'baz']) + deepStrictEqual(getFieldValues('foo, bar, baz, , '), ['foo', 'bar', 'baz']) +}) From 0e12714846a75cc49d9e3bc623a17384d8aa0ea0 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Tue, 20 Feb 2024 03:11:58 +0100 Subject: [PATCH 2/3] remove --- benchmarks/cacheGetFieldValues.mjs | 1 - lib/cache/util.js | 6 ------ test/cache/get-field-values.js | 3 +-- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/benchmarks/cacheGetFieldValues.mjs b/benchmarks/cacheGetFieldValues.mjs index 2a5894bd2ad..a02d0987d4d 100644 --- a/benchmarks/cacheGetFieldValues.mjs +++ b/benchmarks/cacheGetFieldValues.mjs @@ -2,7 +2,6 @@ import { bench, group, run } from 'mitata' import { getFieldValues } from '../lib/cache/util.js' const values = [ - '', 'foo', 'invälid', 'foo, ' diff --git a/lib/cache/util.js b/lib/cache/util.js index 745c5b04ce7..d168d45351b 100644 --- a/lib/cache/util.js +++ b/lib/cache/util.js @@ -19,8 +19,6 @@ function urlEquals (A, B, excludeFragment = false) { return serializedA === serializedB } -const emptyArray = Object.freeze([]) - /** * @see https://github.com/chromium/chromium/blob/694d20d134cb553d8d89e5500b9148012b1ba299/content/browser/cache_storage/cache_storage_cache.cc#L260-L262 * @param {string} header @@ -28,10 +26,6 @@ const emptyArray = Object.freeze([]) function getFieldValues (header) { assert(header !== null) - if (header === '') { - return emptyArray - } - const values = [] for (let value of header.split(',')) { diff --git a/test/cache/get-field-values.js b/test/cache/get-field-values.js index e9287066fbb..7a1a91d523a 100644 --- a/test/cache/get-field-values.js +++ b/test/cache/get-field-values.js @@ -1,6 +1,6 @@ 'use strict' -const { deepStrictEqual, throws, strictEqual } = require('node:assert') +const { deepStrictEqual, throws } = require('node:assert') const { test } = require('node:test') const { getFieldValues } = require('../../lib/cache/util') @@ -10,7 +10,6 @@ test('getFieldValues', () => { message: 'The expression evaluated to a falsy value:\n\n assert(header !== null)\n' }) deepStrictEqual(getFieldValues(''), []) - strictEqual(Object.isFrozen(getFieldValues('')), true) deepStrictEqual(getFieldValues('foo'), ['foo']) deepStrictEqual(getFieldValues('invälid'), []) deepStrictEqual(getFieldValues('foo, bar'), ['foo', 'bar']) From a6474a2b42e3c28ca2e6f2396583af103e8423eb Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Tue, 20 Feb 2024 10:10:43 +0100 Subject: [PATCH 3/3] Update benchmarks/cacheGetFieldValues.mjs --- benchmarks/cacheGetFieldValues.mjs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/benchmarks/cacheGetFieldValues.mjs b/benchmarks/cacheGetFieldValues.mjs index a02d0987d4d..50fd3185184 100644 --- a/benchmarks/cacheGetFieldValues.mjs +++ b/benchmarks/cacheGetFieldValues.mjs @@ -2,9 +2,14 @@ import { bench, group, run } from 'mitata' import { getFieldValues } from '../lib/cache/util.js' const values = [ + '', 'foo', 'invälid', - 'foo, ' + 'foo, ', + 'foo, bar', + 'foo, bar, baz', + 'foo, bar, baz, ', + 'foo, bar, baz, , ' ] group('getFieldValues', () => {