From b45a9ed44dc8234f0c7871e173571f6fb8fb0266 Mon Sep 17 00:00:00 2001
From: Brian Ewins <bewins@nerdwallet.com>
Date: Wed, 17 Jul 2024 20:03:09 +0100
Subject: [PATCH 1/6] fix: narrow the validation cookies to match RFC6265

fixes #165. Previously, validation on serialization used the field-content
regexp from RFC7230 to perform all checks. However, that is the regexp
for the cookie header as a whole, but each individual part of a cookie
has a tighter restriction.

In the bug report #165 I demonstrated that whitespace in names and values
was invalid but allowed by the field-content pattern. In this code I
started by adding tests for those two cases, then added the expressions
derived from the RFC; the relevant portions of the RFC have been inlined
as comments.

Removing the field-content regexp unearthed that as well as those two
it was being used to validate domain names and paths. Paths are fairly
unrestricted (being any ascii character except control characters and
semicolon) but the code was wrong here too - it allowed all 8-bit characters
not just 7-bit characters.

Domain names have a well recognised pattern, there are only a couple of
gotchas: RFC6265 explicitly says RFC1123 applies, so the leading character
of a label is allowed to be a digit; and while I wondered for a moment
if this should allow things like [::1] the domain matching section of
https://datatracker.ietf.org/doc/html/rfc6265#section-5.1.3
teaches that domain values are _not_ ip addresses.
---
 index.js          | 61 ++++++++++++++++++++++++++++++++++++++++-------
 test/serialize.js |  4 ++++
 2 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/index.js b/index.js
index 7506a79..0eb924e 100644
--- a/index.js
+++ b/index.js
@@ -23,14 +23,57 @@ exports.serialize = serialize;
 var __toString = Object.prototype.toString
 
 /**
- * RegExp to match field-content in RFC 7230 sec 3.2
+ * RegExp to match cookie-name in RFC 6265 sec 4.1.1
+ * This refers out to the obsoleted definition of token in RFC 2616 sec 2.2
+ * which has been replaced by the token definition in RFC 7230 appendix B.
+ * 
+ * cookie-name       = token
+ * token             = 1*tchar
+ * tchar             = "!" / "#" / "$" / "%" / "&" / "'" /
+ *                     "*" / "+" / "-" / "." / "^" / "_" /
+ *                     "`" / "|" / "~" / DIGIT / ALPHA
+ */
+
+var cookieNameRegExp = /^[!#$%&'*+.^_`|~0-9A-Za-z-]+$/;
+
+/** 
+ * RegExp to match cookie-value in RFC 6265 sec 4.1.1
+ *
+ * cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
+ * cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
+ *                     ; US-ASCII characters excluding CTLs,
+ *                     ; whitespace DQUOTE, comma, semicolon,
+ *                     ; and backslash
+ */
+
+var  cookieValueRegExp = /^("?)[\u0021\u0023-\u002B\u002D-\u003A\u003C-\u005B\u005D-\u007E]*\1$/;
+
+/**
+ * RegExp to match domain-value in RFC 6265 sec 4.1.1
+ *
+ * domain-value      = <subdomain>
+ *                     ; defined in [RFC1034], Section 3.5, as
+ *                     ; enhanced by [RFC1123], Section 2.1
+ * <subdomain>       = <label> | <subdomain> "." <label>
+ * <label>           = <let-dig> [ [ <ldh-str> ] <let-dig> ]
+ *                     Labels must be 63 characters or less.
+ *                     'let-dig' not 'letter' in the first char, per RFC1123
+ * <ldh-str>         = <let-dig-hyp> | <let-dig-hyp> <ldh-str>
+ * <let-dig-hyp>     = <let-dig> | "-"
+ * <let-dig>         = <letter> | <digit>
+ * <letter>          = any one of the 52 alphabetic characters A through Z in
+ *                     upper case and a through z in lower case
+ * <digit>           = any one of the ten digits 0 through 9
+ */
+var  domainValueRegExp = /^([a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)([.][a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/;
+
+/**
+ * RegExp to match path-value in RFC 6265 sec 4.1.1
  *
- * field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ]
- * field-vchar   = VCHAR / obs-text
- * obs-text      = %x80-FF
+ * path-value        = <any CHAR except CTLs or ";">
  */
 
-var fieldContentRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/;
+var  pathValueRegExp = /^[\u0020-\u003A\u003D-\u007E]*$/;
 
 /**
  * Parse a cookie header.
@@ -116,13 +159,13 @@ function serialize(name, val, options) {
     throw new TypeError('option encode is invalid');
   }
 
-  if (!fieldContentRegExp.test(name)) {
+  if (!cookieNameRegExp.test(name)) {
     throw new TypeError('argument name is invalid');
   }
 
   var value = enc(val);
 
-  if (value && !fieldContentRegExp.test(value)) {
+  if (value && !cookieValueRegExp.test(value)) {
     throw new TypeError('argument val is invalid');
   }
 
@@ -139,7 +182,7 @@ function serialize(name, val, options) {
   }
 
   if (opt.domain) {
-    if (!fieldContentRegExp.test(opt.domain)) {
+    if (!domainValueRegExp.test(opt.domain)) {
       throw new TypeError('option domain is invalid');
     }
 
@@ -147,7 +190,7 @@ function serialize(name, val, options) {
   }
 
   if (opt.path) {
-    if (!fieldContentRegExp.test(opt.path)) {
+    if (!pathValueRegExp.test(opt.path)) {
       throw new TypeError('option path is invalid');
     }
 
diff --git a/test/serialize.js b/test/serialize.js
index 80d0c48..0f8a6df 100644
--- a/test/serialize.js
+++ b/test/serialize.js
@@ -20,6 +20,7 @@ describe('cookie.serialize(name, value)', function () {
   it('should throw for invalid name', function () {
     assert.throws(cookie.serialize.bind(cookie, 'foo\n', 'bar'), /argument name is invalid/)
     assert.throws(cookie.serialize.bind(cookie, 'foo\u280a', 'bar'), /argument name is invalid/)
+    assert.throws(cookie.serialize.bind(cookie, 'foo bar', 'bar'), /argument name is invalid/)
   })
 })
 
@@ -52,6 +53,9 @@ describe('cookie.serialize(name, value, options)', function () {
       assert.throws(cookie.serialize.bind(cookie, 'foo', '+ \n', {
         encode: function (v) { return v }
       }), /argument val is invalid/)
+      assert.throws(cookie.serialize.bind(cookie, 'foo', 'foo bar', {
+        encode: function (v) { return v }
+      }), /argument val is invalid/)
     })
   })
 

From adf50e45f941cc65a4a17e6e1b24e4da62b601f4 Mon Sep 17 00:00:00 2001
From: Brian Ewins <bewins@nerdwallet.com>
Date: Wed, 17 Jul 2024 20:23:44 +0100
Subject: [PATCH 2/6] chore: removing some extra spaces

---
 index.js | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/index.js b/index.js
index 0eb924e..2e14fb2 100644
--- a/index.js
+++ b/index.js
@@ -26,7 +26,7 @@ var __toString = Object.prototype.toString
  * RegExp to match cookie-name in RFC 6265 sec 4.1.1
  * This refers out to the obsoleted definition of token in RFC 2616 sec 2.2
  * which has been replaced by the token definition in RFC 7230 appendix B.
- * 
+ *
  * cookie-name       = token
  * token             = 1*tchar
  * tchar             = "!" / "#" / "$" / "%" / "&" / "'" /
@@ -36,7 +36,7 @@ var __toString = Object.prototype.toString
 
 var cookieNameRegExp = /^[!#$%&'*+.^_`|~0-9A-Za-z-]+$/;
 
-/** 
+/**
  * RegExp to match cookie-value in RFC 6265 sec 4.1.1
  *
  * cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
@@ -46,7 +46,7 @@ var cookieNameRegExp = /^[!#$%&'*+.^_`|~0-9A-Za-z-]+$/;
  *                     ; and backslash
  */
 
-var  cookieValueRegExp = /^("?)[\u0021\u0023-\u002B\u002D-\u003A\u003C-\u005B\u005D-\u007E]*\1$/;
+var cookieValueRegExp = /^("?)[\u0021\u0023-\u002B\u002D-\u003A\u003C-\u005B\u005D-\u007E]*\1$/;
 
 /**
  * RegExp to match domain-value in RFC 6265 sec 4.1.1
@@ -65,7 +65,7 @@ var  cookieValueRegExp = /^("?)[\u0021\u0023-\u002B\u002D-\u003A\u003C-\u005B\u0
  *                     upper case and a through z in lower case
  * <digit>           = any one of the ten digits 0 through 9
  */
-var  domainValueRegExp = /^([a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)([.][a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/;
+var domainValueRegExp = /^([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)([.][a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$/i;
 
 /**
  * RegExp to match path-value in RFC 6265 sec 4.1.1
@@ -73,7 +73,7 @@ var  domainValueRegExp = /^([a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)([.][a-z
  * path-value        = <any CHAR except CTLs or ";">
  */
 
-var  pathValueRegExp = /^[\u0020-\u003A\u003D-\u007E]*$/;
+var pathValueRegExp = /^[\u0020-\u003A\u003D-\u007E]*$/;
 
 /**
  * Parse a cookie header.

From 23187ce118dbdff718f866597dc69631ca73243b Mon Sep 17 00:00:00 2001
From: Brian Ewins <bewins@nerdwallet.com>
Date: Wed, 17 Jul 2024 22:52:50 +0100
Subject: [PATCH 3/6] chore: reorder regexp per review comment

The regexp should be in the same order as the RFC grammar, for ease
of checking.
---
 index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/index.js b/index.js
index 2e14fb2..dcda792 100644
--- a/index.js
+++ b/index.js
@@ -34,7 +34,7 @@ var __toString = Object.prototype.toString
  *                     "`" / "|" / "~" / DIGIT / ALPHA
  */
 
-var cookieNameRegExp = /^[!#$%&'*+.^_`|~0-9A-Za-z-]+$/;
+var cookieNameRegExp = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/;
 
 /**
  * RegExp to match cookie-value in RFC 6265 sec 4.1.1

From 3963475d2293fdd0a1bc019059103a06a95e2d2c Mon Sep 17 00:00:00 2001
From: Brian Ewins <119598239+bewinsnw@users.noreply.github.com>
Date: Thu, 18 Jul 2024 14:48:31 +0100
Subject: [PATCH 4/6] chore: document the meaning of CHAR

---
 index.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/index.js b/index.js
index dcda792..941e96b 100644
--- a/index.js
+++ b/index.js
@@ -71,6 +71,8 @@ var domainValueRegExp = /^([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)([.][a-z0-9]([a-z0
  * RegExp to match path-value in RFC 6265 sec 4.1.1
  *
  * path-value        = <any CHAR except CTLs or ";">
+ * CHAR              = %x01-7F
+ *                     ; defined in RFC 5234 appendix B.1
  */
 
 var pathValueRegExp = /^[\u0020-\u003A\u003D-\u007E]*$/;

From 32046d43e378b4403e163b9cfc230aa91c343436 Mon Sep 17 00:00:00 2001
From: Brian Ewins <119598239+bewinsnw@users.noreply.github.com>
Date: Thu, 18 Jul 2024 14:52:25 +0100
Subject: [PATCH 5/6] chore: missing whitespace after comment block

---
 index.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/index.js b/index.js
index 941e96b..d800cbb 100644
--- a/index.js
+++ b/index.js
@@ -65,6 +65,7 @@ var cookieValueRegExp = /^("?)[\u0021\u0023-\u002B\u002D-\u003A\u003C-\u005B\u00
  *                     upper case and a through z in lower case
  * <digit>           = any one of the ten digits 0 through 9
  */
+ 
 var domainValueRegExp = /^([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)([.][a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$/i;
 
 /**

From 22e5f22bf89ecfed48965b68be3ba00f7f7f45f4 Mon Sep 17 00:00:00 2001
From: Brian Ewins <bewins@nerdwallet.com>
Date: Sat, 27 Jul 2024 17:28:21 +0100
Subject: [PATCH 6/6] chore: trailing space

---
 index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/index.js b/index.js
index d800cbb..6fe633d 100644
--- a/index.js
+++ b/index.js
@@ -65,7 +65,7 @@ var cookieValueRegExp = /^("?)[\u0021\u0023-\u002B\u002D-\u003A\u003C-\u005B\u00
  *                     upper case and a through z in lower case
  * <digit>           = any one of the ten digits 0 through 9
  */
- 
+
 var domainValueRegExp = /^([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)([.][a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$/i;
 
 /**