From 02107fee236c4310b5243e242a4d5662ede47e87 Mon Sep 17 00:00:00 2001
From: Aras Abbasi <aras.abbasi@googlemail.com>
Date: Tue, 20 Feb 2024 19:05:11 +0100
Subject: [PATCH] chore: improve getFieldValue (#2785)

* chore: improve getFieldValue

* remove

* Update benchmarks/cacheGetFieldValues.mjs
---
 benchmarks/cacheGetFieldValues.mjs | 23 +++++++++++++++++++++++
 lib/cache/cache.js                 |  2 +-
 lib/cache/util.js                  | 12 ++++--------
 lib/fetch/util.js                  |  4 +---
 test/cache/get-field-values.js     | 19 +++++++++++++++++++
 5 files changed, 48 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..50fd3185184
--- /dev/null
+++ b/benchmarks/cacheGetFieldValues.mjs
@@ -0,0 +1,23 @@
+import { bench, group, run } from 'mitata'
+import { getFieldValues } from '../lib/cache/util.js'
+
+const values = [
+  '',
+  'foo',
+  'invälid',
+  'foo, ',
+  'foo, bar',
+  'foo, bar, baz',
+  'foo, bar, baz, ',
+  'foo, bar, baz, , '
+]
+
+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..d168d45351b 100644
--- a/lib/cache/util.js
+++ b/lib/cache/util.js
@@ -23,7 +23,7 @@ function urlEquals (A, B, excludeFragment = false) {
  * @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)
 
   const values = []
@@ -31,13 +31,9 @@ function fieldValues (header) {
   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 +41,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..7a1a91d523a
--- /dev/null
+++ b/test/cache/get-field-values.js
@@ -0,0 +1,19 @@
+'use strict'
+
+const { deepStrictEqual, throws } = 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(''), [])
+  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'])
+})