cookies: improve validateCookieValue

nodejs · Feb 29, 2024 · 02d8038 · 02d8038
1 parent 3b2df8e
commit 02d8038
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 10 deletions.
diff --git a/benchmarks/cookies/validate-cookie-value.mjs b/benchmarks/cookies/validate-cookie-value.mjs
@@ -0,0 +1,16 @@
+import { bench, group, run } from 'mitata'
+import { validateCookieValue } from '../../lib/web/cookies/util.js'
+
+const valid = 'Cat'
+const wrappedValid = `"${valid}"`
+
+group('validateCookieValue', () => {
+  bench(`valid: ${valid}`, () => {
+    return validateCookieValue(valid)
+  })
+  bench(`valid: ${wrappedValid}`, () => {
+    return validateCookieValue(wrappedValid)
+  })
+})
+
+await run()
diff --git a/lib/web/cookies/util.js b/lib/web/cookies/util.js
@@ -69,18 +69,30 @@ function validateCookieName (name) {
  * @param {string} value
  */
 function validateCookieValue (value) {
-  for (const char of value) {
-    const code = char.charCodeAt(0)
+  let len = value.length
+  let i = 0
+
+  // if the value is wrapped in DQUOTE
+  if (value[0] === '"') {
+    if (len === 1 || value[len - 1] !== '"') {
+      throw new Error('Invalid cookie value')
+    }
+    --len
+    ++i
+  }
+
+  while (i < len) {
+    const code = value.charCodeAt(i++)
 
     if (
       code < 0x21 || // exclude CTLs (0-31)
-      code === 0x22 ||
-      code === 0x2C ||
-      code === 0x3B ||
-      code === 0x5C ||
-      code > 0x7E // non-ascii
+      code > 0x7E || // non-ascii and DEL (127)
+      code === 0x22 || // "
+      code === 0x2C || // ,
+      code === 0x3B || // ;
+      code === 0x5C // \
     ) {
-      throw new Error('Invalid header value')
+      throw new Error('Invalid cookie value')
     }
   }
 }
@@ -286,6 +298,7 @@ function getHeadersList (headers) {
 module.exports = {
   isCTLExcludingHtab,
   validateCookiePath,
+  validateCookieValue,
   toIMFDate,
   stringify,
   getHeadersList

diff --git a/test/cookie/cookies.js b/test/cookie/cookies.js
@@ -116,7 +116,7 @@ test('Cookie Value Validation', () => {
           }
         )
       },
-      new Error('Invalid header value'),
+      new Error('Invalid cookie value'),
       "RFC2616 cookie 'Space'"
     )
   })
@@ -128,7 +128,7 @@ test('Cookie Value Validation', () => {
         value: 'United Kingdom'
       })
     },
-    new Error('Invalid header value'),
+    new Error('Invalid cookie value'),
     "RFC2616 cookie 'location' cannot contain character ' '"
   )
 })

diff --git a/test/cookie/validate-cookie-value.js b/test/cookie/validate-cookie-value.js
@@ -0,0 +1,78 @@
+'use strict'
+
+const { test, describe } = require('node:test')
+const { throws, strictEqual } = require('node:assert')
+
+const {
+  validateCookieValue
+} = require('../../lib/web/cookies/util')
+
+describe('validateCookieValue', () => {
+  test('should throw for CTLs', () => {
+    throws(() => validateCookieValue('\x00'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x01'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x02'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x03'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x04'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x05'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x06'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x07'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x08'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x09'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x0A'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x0B'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x0C'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x0D'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x0E'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x0F'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x10'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x11'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x12'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x13'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x14'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x15'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x16'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x17'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x18'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x19'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x1A'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x1B'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x1C'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x1D'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x1E'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x1F'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('\x7F'), new Error('Invalid cookie value'))
+  })
+
+  test('should throw for ; character', () => {
+    throws(() => validateCookieValue(';'), new Error('Invalid cookie value'))
+  })
+
+  test('should throw for " character', () => {
+    throws(() => validateCookieValue('"'), new Error('Invalid cookie value'))
+  })
+
+  test('should throw for " character', () => {
+    throws(() => validateCookieValue(','), new Error('Invalid cookie value'))
+  })
+
+  test('should throw for \\ character', () => {
+    throws(() => validateCookieValue('\\'), new Error('Invalid cookie value'))
+  })
+
+  test('should pass for a printable character', t => {
+    strictEqual(validateCookieValue('A'), undefined)
+    strictEqual(validateCookieValue('Z'), undefined)
+    strictEqual(validateCookieValue('a'), undefined)
+    strictEqual(validateCookieValue('z'), undefined)
+    strictEqual(validateCookieValue('!'), undefined)
+    strictEqual(validateCookieValue('='), undefined)
+  })
+
+  test('should handle strings wrapped in DQUOTE', t => {
+    strictEqual(validateCookieValue('""'), undefined)
+    strictEqual(validateCookieValue('"helloworld"'), undefined)
+    throws(() => validateCookieValue('"'), new Error('Invalid cookie value'))
+    throws(() => validateCookieValue('"""'), new Error('Invalid cookie value'))
+  })
+})