From e08514e33712f78bf7951d0c4df35f7df18118e7 Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Thu, 23 Feb 2023 03:54:33 -0600
Subject: [PATCH] http: fix validation of "Link" header

Updated regex for "Link" header validation to better match the
specification in RFC 8288 section 3. Does not check for valid URI
format but handles the rest of the header more permissively than
before. Alternative to another outstanding PR that disables validation
entirely.

Fixes: https://github.com/nodejs/node/issues/46453
Refs: https://www.rfc-editor.org/rfc/rfc8288.html#section-3
Refs: https://github.com/nodejs/node/pull/46464
PR-URL: https://github.com/nodejs/node/pull/46466
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
---
 lib/internal/validators.js                    | 10 ++-
 .../test-http-early-hints-invalid-argument.js | 66 ++++++++++++-------
 test/parallel/test-http-early-hints.js        |  6 +-
 3 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/lib/internal/validators.js b/lib/internal/validators.js
index 8127bd66fe3609..d86370176a8aa8 100644
--- a/lib/internal/validators.js
+++ b/lib/internal/validators.js
@@ -459,7 +459,15 @@ function validateUnion(value, name, union) {
   }
 }
 
-const linkValueRegExp = /^(?:<[^>]*>;)\s*(?:rel=(")?[^;"]*\1;?)\s*(?:(?:as|anchor|title|crossorigin|disabled|fetchpriority|rel|referrerpolicy)=(")?[^;"]*\2)?$/;
+/*
+  The rules for the Link header field are described here:
+  https://www.rfc-editor.org/rfc/rfc8288.html#section-3
+
+  This regex validates any string surrounded by angle brackets
+  (not necessarily a valid URI reference) followed by zero or more
+  link-params separated by semicolons.
+*/
+const linkValueRegExp = /^(?:<[^>]*>)(?:\s*;\s*[^;"\s]+(?:=(")?[^;"\s]*\1)?)*$/;
 
 /**
  * @param {any} value
diff --git a/test/parallel/test-http-early-hints-invalid-argument.js b/test/parallel/test-http-early-hints-invalid-argument.js
index 2cf29666bef11d..f776bcafa40ed3 100644
--- a/test/parallel/test-http-early-hints-invalid-argument.js
+++ b/test/parallel/test-http-early-hints-invalid-argument.js
@@ -6,28 +6,44 @@ const debug = require('node:util').debuglog('test');
 
 const testResBody = 'response content\n';
 
-const server = http.createServer(common.mustCall((req, res) => {
-  debug('Server sending early hints...');
-  res.writeEarlyHints('bad argument type');
-
-  debug('Server sending full response...');
-  res.end(testResBody);
-}));
-
-server.listen(0, common.mustCall(() => {
-  const req = http.request({
-    port: server.address().port, path: '/'
-  });
-
-  req.end();
-  debug('Client sending request...');
-
-  req.on('information', common.mustNotCall());
-
-  process.on('uncaughtException', (err) => {
-    debug(`Caught an exception: ${JSON.stringify(err)}`);
-    if (err.name === 'AssertionError') throw err;
-    assert.strictEqual(err.code, 'ERR_INVALID_ARG_TYPE');
-    process.exit(0);
-  });
-}));
+{
+  const server = http.createServer(common.mustCall((req, res) => {
+    debug('Server sending early hints...');
+    assert.throws(() => {
+      res.writeEarlyHints('bad argument type');
+    }, (err) => err.code === 'ERR_INVALID_ARG_TYPE');
+
+    assert.throws(() => {
+      res.writeEarlyHints({
+        link: '</>; '
+      });
+    }, (err) => err.code === 'ERR_INVALID_ARG_VALUE');
+
+    assert.throws(() => {
+      res.writeEarlyHints({
+        link: 'rel=preload; </scripts.js>'
+      });
+    }, (err) => err.code === 'ERR_INVALID_ARG_VALUE');
+
+    assert.throws(() => {
+      res.writeEarlyHints({
+        link: 'invalid string'
+      });
+    }, (err) => err.code === 'ERR_INVALID_ARG_VALUE');
+
+    debug('Server sending full response...');
+    res.end(testResBody);
+    server.close();
+  }));
+
+  server.listen(0, common.mustCall(() => {
+    const req = http.request({
+      port: server.address().port, path: '/'
+    });
+
+    req.end();
+    debug('Client sending request...');
+
+    req.on('information', common.mustNotCall());
+  }));
+}
diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js
index 7183d9516f6dda..67affdffddd95e 100644
--- a/test/parallel/test-http-early-hints.js
+++ b/test/parallel/test-http-early-hints.js
@@ -58,7 +58,8 @@ const testResBody = 'response content\n';
     res.writeEarlyHints({
       link: [
         '</styles.css>; rel=preload; as=style',
-        '</scripts.js>; rel=preload; as=script',
+        '</scripts.js>; crossorigin; rel=preload; as=script',
+        '</scripts.js>; rel=preload; as=script; crossorigin',
       ]
     });
 
@@ -75,7 +76,8 @@ const testResBody = 'response content\n';
     req.on('information', common.mustCall((res) => {
       assert.strictEqual(
         res.headers.link,
-        '</styles.css>; rel=preload; as=style, </scripts.js>; rel=preload; as=script'
+        '</styles.css>; rel=preload; as=style, </scripts.js>; crossorigin; ' +
+        'rel=preload; as=script, </scripts.js>; rel=preload; as=script; crossorigin'
       );
     }));