From 9947fd92329ba6bde6b01fb8497ed4c03b4e2d7d Mon Sep 17 00:00:00 2001 From: Steve Herzog Date: Fri, 17 Feb 2023 11:11:17 -0600 Subject: [PATCH 1/6] 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 --- lib/internal/validators.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 8127bd66fe3609..59b4700f1d316d 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -459,7 +459,7 @@ function validateUnion(value, name, union) { } } -const linkValueRegExp = /^(?:<[^>]*>;)\s*(?:rel=(")?[^;"]*\1;?)\s*(?:(?:as|anchor|title|crossorigin|disabled|fetchpriority|rel|referrerpolicy)=(")?[^;"]*\2)?$/; +const linkValueRegExp = /^(?:<[^>]*>)(?:\s*;\s*[^;"]+(?:=(")?[^;"]*\1)?)*$/; /** * @param {any} value From a0281cf927088b0dfcb8d6f1597b4b72ab88d660 Mon Sep 17 00:00:00 2001 From: Steve Herzog Date: Fri, 17 Feb 2023 11:12:00 -0600 Subject: [PATCH 2/6] revert error message, improve regex, add test --- lib/internal/validators.js | 2 +- ...http-early-hints-invalid-argument-value.js | 35 +++++++++++++++++++ test/parallel/test-http-early-hints.js | 4 +-- 3 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-http-early-hints-invalid-argument-value.js diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 59b4700f1d316d..4e195350ef9e7c 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -459,7 +459,7 @@ function validateUnion(value, name, union) { } } -const linkValueRegExp = /^(?:<[^>]*>)(?:\s*;\s*[^;"]+(?:=(")?[^;"]*\1)?)*$/; +const linkValueRegExp = /^(?:<[^>]*>)(?:\s*;\s*[^;"\s]+(?:=(")?[^;"\s]*\1)?)*$/; /** * @param {any} value diff --git a/test/parallel/test-http-early-hints-invalid-argument-value.js b/test/parallel/test-http-early-hints-invalid-argument-value.js new file mode 100644 index 00000000000000..aafc4e7ccfe277 --- /dev/null +++ b/test/parallel/test-http-early-hints-invalid-argument-value.js @@ -0,0 +1,35 @@ +'use strict'; +const common = require('../common'); +const assert = require('node:assert'); +const http = require('node:http'); +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({ + link: '; ' + }); + + 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_VALUE'); + process.exit(); + }); +})); diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js index 7183d9516f6dda..88a3d663b38fe8 100644 --- a/test/parallel/test-http-early-hints.js +++ b/test/parallel/test-http-early-hints.js @@ -58,7 +58,7 @@ const testResBody = 'response content\n'; res.writeEarlyHints({ link: [ '; rel=preload; as=style', - '; rel=preload; as=script', + '; rel=preload; as=script; crossorigin', ] }); @@ -75,7 +75,7 @@ const testResBody = 'response content\n'; req.on('information', common.mustCall((res) => { assert.strictEqual( res.headers.link, - '; rel=preload; as=style, ; rel=preload; as=script' + '; rel=preload; as=style, ; rel=preload; as=script; crossorigin' ); })); From 88eaafd3581ffb2ae46b62a17a0def1a6de3c47a Mon Sep 17 00:00:00 2001 From: Steve Herzog Date: Fri, 3 Feb 2023 10:08:57 -0600 Subject: [PATCH 3/6] minor test refactor --- ...http-early-hints-invalid-argument-value.js | 35 --------- .../test-http-early-hints-invalid-argument.js | 74 ++++++++++++------- 2 files changed, 49 insertions(+), 60 deletions(-) delete mode 100644 test/parallel/test-http-early-hints-invalid-argument-value.js diff --git a/test/parallel/test-http-early-hints-invalid-argument-value.js b/test/parallel/test-http-early-hints-invalid-argument-value.js deleted file mode 100644 index aafc4e7ccfe277..00000000000000 --- a/test/parallel/test-http-early-hints-invalid-argument-value.js +++ /dev/null @@ -1,35 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('node:assert'); -const http = require('node:http'); -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({ - link: '; ' - }); - - 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_VALUE'); - process.exit(); - }); -})); diff --git a/test/parallel/test-http-early-hints-invalid-argument.js b/test/parallel/test-http-early-hints-invalid-argument.js index 2cf29666bef11d..81fbf02820ff67 100644 --- a/test/parallel/test-http-early-hints-invalid-argument.js +++ b/test/parallel/test-http-early-hints-invalid-argument.js @@ -6,28 +6,52 @@ 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'); + + 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()); + })); +} + +{ + const server = http.createServer(common.mustCall((req, res) => { + debug('Server sending early hints...'); + assert.throws(() => { + res.writeEarlyHints({ + link: '; ' + }); + }, (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()); + })); +} From e24f2fd86728b09e0eeeded6d7958b341d9a5c6c Mon Sep 17 00:00:00 2001 From: Steve Herzog Date: Mon, 6 Feb 2023 11:14:16 -0600 Subject: [PATCH 4/6] update tests and comments --- lib/internal/validators.js | 8 +++++++ .../test-http-early-hints-invalid-argument.js | 23 +++++++++++++++---- test/parallel/test-http-early-hints.js | 3 ++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 4e195350ef9e7c..d86370176a8aa8 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -459,6 +459,14 @@ function validateUnion(value, name, union) { } } +/* + 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)?)*$/; /** diff --git a/test/parallel/test-http-early-hints-invalid-argument.js b/test/parallel/test-http-early-hints-invalid-argument.js index 81fbf02820ff67..ffcda11891c5c0 100644 --- a/test/parallel/test-http-early-hints-invalid-argument.js +++ b/test/parallel/test-http-early-hints-invalid-argument.js @@ -13,6 +13,24 @@ const testResBody = 'response content\n'; 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; ' + }); + }, (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(); @@ -33,11 +51,6 @@ const testResBody = 'response content\n'; { const server = http.createServer(common.mustCall((req, res) => { debug('Server sending early hints...'); - assert.throws(() => { - res.writeEarlyHints({ - link: '; ' - }); - }, (err) => err.code === 'ERR_INVALID_ARG_VALUE'); debug('Server sending full response...'); res.end(testResBody); diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js index 88a3d663b38fe8..bb631a05a6fb3f 100644 --- a/test/parallel/test-http-early-hints.js +++ b/test/parallel/test-http-early-hints.js @@ -58,6 +58,7 @@ const testResBody = 'response content\n'; res.writeEarlyHints({ link: [ '; rel=preload; as=style', + '; crossorigin; rel=preload; as=script', '; rel=preload; as=script; crossorigin', ] }); @@ -75,7 +76,7 @@ const testResBody = 'response content\n'; req.on('information', common.mustCall((res) => { assert.strictEqual( res.headers.link, - '; rel=preload; as=style, ; rel=preload; as=script; crossorigin' + '; rel=preload; as=style, ; crossorigin; rel=preload; as=script, ; rel=preload; as=script; crossorigin' ); })); From 2327a582384d94e2dd3edbb3e87e8be3af1b30b8 Mon Sep 17 00:00:00 2001 From: Steve Herzog Date: Mon, 6 Feb 2023 13:12:24 -0600 Subject: [PATCH 5/6] remove redundant test --- .../test-http-early-hints-invalid-argument.js | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/test/parallel/test-http-early-hints-invalid-argument.js b/test/parallel/test-http-early-hints-invalid-argument.js index ffcda11891c5c0..f776bcafa40ed3 100644 --- a/test/parallel/test-http-early-hints-invalid-argument.js +++ b/test/parallel/test-http-early-hints-invalid-argument.js @@ -47,24 +47,3 @@ const testResBody = 'response content\n'; req.on('information', common.mustNotCall()); })); } - -{ - const server = http.createServer(common.mustCall((req, res) => { - debug('Server sending early hints...'); - - 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()); - })); -} From 8c49924a31c79577ba8125e08711ccca9fa5cb7c Mon Sep 17 00:00:00 2001 From: Steve Herzog Date: Thu, 9 Feb 2023 08:05:33 -0600 Subject: [PATCH 6/6] fix overlong string --- test/parallel/test-http-early-hints.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js index bb631a05a6fb3f..67affdffddd95e 100644 --- a/test/parallel/test-http-early-hints.js +++ b/test/parallel/test-http-early-hints.js @@ -76,7 +76,8 @@ const testResBody = 'response content\n'; req.on('information', common.mustCall((res) => { assert.strictEqual( res.headers.link, - '; rel=preload; as=style, ; crossorigin; rel=preload; as=script, ; rel=preload; as=script; crossorigin' + '; rel=preload; as=style, ; crossorigin; ' + + 'rel=preload; as=script, ; rel=preload; as=script; crossorigin' ); }));