From 716cd24113ed2cb24db0a5a4a93387cadf2afd7f Mon Sep 17 00:00:00 2001
From: Aras Abbasi <aras.abbasi@googlemail.com>
Date: Fri, 23 Feb 2024 11:13:10 +0100
Subject: [PATCH] fix: isCTLExcludingHtab (#2790)

* fix: isCTLExcludingHtab

* Update benchmarks/cookiesIsCTLExcludingHtab.mjs

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

* Update lib/cookies/util.js

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>

* simplify

* fix paths

---------

Co-authored-by: tsctx <91457664+tsctx@users.noreply.github.com>
---
 benchmarks/cookiesIsCTLExcludingHtab.mjs | 17 +++++
 lib/web/cookies/util.js                  | 19 +++---
 test/cookie/is-ctl-excluding-htab.js     | 85 ++++++++++++++++++++++++
 3 files changed, 112 insertions(+), 9 deletions(-)
 create mode 100644 benchmarks/cookiesIsCTLExcludingHtab.mjs
 create mode 100644 test/cookie/is-ctl-excluding-htab.js

diff --git a/benchmarks/cookiesIsCTLExcludingHtab.mjs b/benchmarks/cookiesIsCTLExcludingHtab.mjs
new file mode 100644
index 00000000000..b0e134b22d2
--- /dev/null
+++ b/benchmarks/cookiesIsCTLExcludingHtab.mjs
@@ -0,0 +1,17 @@
+import { bench, group, run } from 'mitata'
+import { isCTLExcludingHtab } from '../lib/web/cookies/util.js'
+
+const valid = 'Space=Cat; Secure; HttpOnly; Max-Age=2'
+const invalid = 'Space=Cat; Secure; HttpOnly; Max-Age=2\x7F'
+
+group('isCTLExcludingHtab', () => {
+  bench(`valid: ${valid}`, () => {
+    return isCTLExcludingHtab(valid)
+  })
+
+  bench(`invalid: ${invalid}`, () => {
+    return isCTLExcludingHtab(invalid)
+  })
+})
+
+await run()
diff --git a/lib/web/cookies/util.js b/lib/web/cookies/util.js
index 0c1353d5ca3..6d3a79b69ad 100644
--- a/lib/web/cookies/util.js
+++ b/lib/web/cookies/util.js
@@ -3,22 +3,23 @@
 const assert = require('node:assert')
 const { kHeadersList } = require('../../core/symbols')
 
+/**
+ * @param {string} value
+ * @returns {boolean}
+ */
 function isCTLExcludingHtab (value) {
-  if (value.length === 0) {
-    return false
-  }
-
-  for (const char of value) {
-    const code = char.charCodeAt(0)
+  for (let i = 0; i < value.length; ++i) {
+    const code = value.charCodeAt(i)
 
     if (
-      (code >= 0x00 || code <= 0x08) ||
-      (code >= 0x0A || code <= 0x1F) ||
+      (code >= 0x00 && code <= 0x08) ||
+      (code >= 0x0A && code <= 0x1F) ||
       code === 0x7F
     ) {
-      return false
+      return true
     }
   }
+  return false
 }
 
 /**
diff --git a/test/cookie/is-ctl-excluding-htab.js b/test/cookie/is-ctl-excluding-htab.js
new file mode 100644
index 00000000000..a9523326553
--- /dev/null
+++ b/test/cookie/is-ctl-excluding-htab.js
@@ -0,0 +1,85 @@
+'use strict'
+
+const { test, describe } = require('node:test')
+const { strictEqual } = require('node:assert')
+
+const {
+  isCTLExcludingHtab
+} = require('../../lib/web/cookies/util')
+
+describe('isCTLExcludingHtab', () => {
+  test('should return false for 0x00 - 0x08 characters', () => {
+    strictEqual(isCTLExcludingHtab('\x00'), true)
+    strictEqual(isCTLExcludingHtab('\x01'), true)
+    strictEqual(isCTLExcludingHtab('\x02'), true)
+    strictEqual(isCTLExcludingHtab('\x03'), true)
+    strictEqual(isCTLExcludingHtab('\x04'), true)
+    strictEqual(isCTLExcludingHtab('\x05'), true)
+    strictEqual(isCTLExcludingHtab('\x06'), true)
+    strictEqual(isCTLExcludingHtab('\x07'), true)
+    strictEqual(isCTLExcludingHtab('\x08'), true)
+  })
+
+  test('should return false for 0x09 HTAB character', () => {
+    strictEqual(isCTLExcludingHtab('\x09'), false)
+  })
+
+  test('should return false for 0x0A - 0x1F characters', () => {
+    strictEqual(isCTLExcludingHtab('\x0A'), true)
+    strictEqual(isCTLExcludingHtab('\x0B'), true)
+    strictEqual(isCTLExcludingHtab('\x0C'), true)
+    strictEqual(isCTLExcludingHtab('\x0D'), true)
+    strictEqual(isCTLExcludingHtab('\x0E'), true)
+    strictEqual(isCTLExcludingHtab('\x0F'), true)
+    strictEqual(isCTLExcludingHtab('\x10'), true)
+    strictEqual(isCTLExcludingHtab('\x11'), true)
+    strictEqual(isCTLExcludingHtab('\x12'), true)
+    strictEqual(isCTLExcludingHtab('\x13'), true)
+    strictEqual(isCTLExcludingHtab('\x14'), true)
+    strictEqual(isCTLExcludingHtab('\x15'), true)
+    strictEqual(isCTLExcludingHtab('\x16'), true)
+    strictEqual(isCTLExcludingHtab('\x17'), true)
+    strictEqual(isCTLExcludingHtab('\x18'), true)
+    strictEqual(isCTLExcludingHtab('\x19'), true)
+    strictEqual(isCTLExcludingHtab('\x1A'), true)
+    strictEqual(isCTLExcludingHtab('\x1B'), true)
+    strictEqual(isCTLExcludingHtab('\x1C'), true)
+    strictEqual(isCTLExcludingHtab('\x1D'), true)
+    strictEqual(isCTLExcludingHtab('\x1E'), true)
+    strictEqual(isCTLExcludingHtab('\x1F'), true)
+  })
+
+  test('should return false for a 0x7F character', t => {
+    strictEqual(isCTLExcludingHtab('\x7F'), true)
+  })
+
+  test('should return false for a 0x20 / space character', t => {
+    strictEqual(isCTLExcludingHtab(' '), false)
+  })
+
+  test('should return false for a printable character', t => {
+    strictEqual(isCTLExcludingHtab('A'), false)
+    strictEqual(isCTLExcludingHtab('Z'), false)
+    strictEqual(isCTLExcludingHtab('a'), false)
+    strictEqual(isCTLExcludingHtab('z'), false)
+    strictEqual(isCTLExcludingHtab('!'), false)
+  })
+
+  test('should return false for an empty string', () => {
+    strictEqual(isCTLExcludingHtab(''), false)
+  })
+
+  test('all printable characters (0x20 - 0x7E)', () => {
+    for (let i = 0x20; i < 0x7F; i++) {
+      strictEqual(isCTLExcludingHtab(String.fromCharCode(i)), false)
+    }
+  })
+
+  test('valid case', () => {
+    strictEqual(isCTLExcludingHtab('Space=Cat; Secure; HttpOnly; Max-Age=2'), false)
+  })
+
+  test('invalid case', () => {
+    strictEqual(isCTLExcludingHtab('Space=Cat; Secure; HttpOnly; Max-Age=2\x7F'), true)
+  })
+})