From ca64b81be309c44c794066b8bf10fccb26827fd6 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 29 Sep 2022 09:57:02 -0400 Subject: [PATCH 1/7] http2: make early hints generic --- lib/_http_server.js | 47 +++++++++- lib/internal/http2/compat.js | 46 +++++++++- ...-http-early-hints-invalid-argument-type.js | 33 ------- test/parallel/test-http-early-hints.js | 86 +++++++++++++++++++ 4 files changed, 176 insertions(+), 36 deletions(-) delete mode 100644 test/parallel/test-http-early-hints-invalid-argument-type.js diff --git a/lib/_http_server.js b/lib/_http_server.js index 8bd8358b5edb67..2f5c9c0287c78d 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -320,11 +320,56 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(links, cb) { } head += '\r\n'; + } else if (typeof links === 'object') { + const keys = ObjectKeys(links); + + if (!keys.length) { + return; + } + + const link = links.link; + + if (typeof link === 'string') { + validateLinkHeaderValue(link, 'links'); + head += 'Link: ' + link + '\r\n'; + } else if (ArrayIsArray(link)) { + const linkLength = link.length; + if (!linkLength) { + return; + } + + head += 'Link: '; + + for (let i = 0; i < linkLength; i++) { + validateLinkHeaderValue(link[i], 'links'); + head += link[i]; + + if (i !== linkLength - 1) { + head += ', '; + } + } + + head += '\r\n'; + } else { + throw new ERR_INVALID_ARG_VALUE( + 'links', + links, + 'must be an array, object or string of format "; rel=preload; as=style"' + ); + } + + for (const key of keys) { + if (key === 'link') { + continue; + } + + head += key + ', ' + keys[key] + '\r\n'; + } } else { throw new ERR_INVALID_ARG_VALUE( 'links', links, - 'must be an array or string of format "; rel=preload; as=style"' + 'must be an array, object or string of format "; rel=preload; as=style"' ); } diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 727192e23fade2..7c7de7187b14bd 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -848,6 +848,7 @@ class Http2ServerResponse extends Stream { } writeEarlyHints(links) { + const headers = ObjectCreate(null); let linkHeaderValue = ''; if (typeof links === 'string') { @@ -869,11 +870,51 @@ class Http2ServerResponse extends Stream { linkHeaderValue += ', '; } } + } else if (typeof links === 'object') { + const keys = ObjectKeys(links); + + if (!keys.length) { + return; + } + + const link = links.link; + + if (typeof link === 'string') { + validateLinkHeaderValue(link, 'links'); + linkHeaderValue += link; + } else if (ArrayIsArray(link)) { + if (!link.length) { + return; + } + + for (let i = 0; i < link.length; i++) { + validateLinkHeaderValue(link[i]); + linkHeaderValue += link; + + if (i !== link.length - 1) { + linkHeaderValue += ', '; + } + } + } else { + throw new ERR_INVALID_ARG_VALUE( + 'links', + links, + 'must be an array, object or string of format "; rel=preload; as=style"' + ); + } + + for (const key of keys) { + if (key === 'link') { + continue; + } + + headers[key] = keys[key]; + } } else { throw new ERR_INVALID_ARG_VALUE( 'links', links, - 'must be an array or string of format "; rel=preload; as=style"' + 'must be an array, object or string of format "; rel=preload; as=style"' ); } @@ -884,7 +925,8 @@ class Http2ServerResponse extends Stream { stream.additionalHeaders({ [HTTP2_HEADER_STATUS]: HTTP_STATUS_EARLY_HINTS, - 'Link': linkHeaderValue + 'Link': linkHeaderValue, + ...headers }); return true; diff --git a/test/parallel/test-http-early-hints-invalid-argument-type.js b/test/parallel/test-http-early-hints-invalid-argument-type.js deleted file mode 100644 index 796ab7d6366572..00000000000000 --- a/test/parallel/test-http-early-hints-invalid-argument-type.js +++ /dev/null @@ -1,33 +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({ links: 'bad argument object' }); - - 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(0); - }); -})); diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js index 975be2e112a5b3..6f03bbf3667a04 100644 --- a/test/parallel/test-http-early-hints.js +++ b/test/parallel/test-http-early-hints.js @@ -133,3 +133,89 @@ const testResBody = 'response content\n'; req.end(); })); } + +{ + // Happy flow - object argument + + const server = http.createServer(common.mustCall((req, res) => { + debug('Server sending early hints...'); + res.writeEarlyHints({ + 'link': '; rel=preload; as=style', + 'x-trace-id': 'id for diagnostics' + }); + + debug('Server sending full response...'); + res.end(testResBody); + })); + + server.listen(0, common.mustCall(() => { + const req = http.request({ + port: server.address().port, path: '/' + }); + debug('Client sending request...'); + + req.on('information', common.mustCall((res) => { + assert.strictEqual( + res.headers.link, + '; rel=preload; as=style, ; rel=preload; as=script' + ); + })); + + req.on('response', common.mustCall((res) => { + let body = ''; + + assert.strictEqual(res.statusCode, 200, `Final status code was ${res.statusCode}, not 200.`); + + res.on('data', (chunk) => { + body += chunk; + }); + + res.on('end', common.mustCall(() => { + debug('Got full response.'); + assert.strictEqual(body, testResBody); + server.close(); + })); + })); + + req.end(); + })); +} + +{ + // Happy flow - empty object + + const server = http.createServer(common.mustCall((req, res) => { + debug('Server sending early hints...'); + res.writeEarlyHints({}); + + debug('Server sending full response...'); + res.end(testResBody); + })); + + server.listen(0, common.mustCall(() => { + const req = http.request({ + port: server.address().port, path: '/' + }); + debug('Client sending request...'); + + req.on('information', common.mustNotCall()); + + req.on('response', common.mustCall((res) => { + let body = ''; + + assert.strictEqual(res.statusCode, 200, `Final status code was ${res.statusCode}, not 200.`); + + res.on('data', (chunk) => { + body += chunk; + }); + + res.on('end', common.mustCall(() => { + debug('Got full response.'); + assert.strictEqual(body, testResBody); + server.close(); + })); + })); + + req.end(); + })); +} From 911e0276b89e27cef4d8da83f9a8917a815cdb55 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 29 Sep 2022 10:10:36 -0400 Subject: [PATCH 2/7] test: add more tests for early-hints --- test/parallel/test-http-early-hints.js | 52 +++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js index 6f03bbf3667a04..82369785f2e918 100644 --- a/test/parallel/test-http-early-hints.js +++ b/test/parallel/test-http-early-hints.js @@ -135,7 +135,7 @@ const testResBody = 'response content\n'; } { - // Happy flow - object argument + // Happy flow - object argument with string const server = http.createServer(common.mustCall((req, res) => { debug('Server sending early hints...'); @@ -148,6 +148,56 @@ const testResBody = 'response content\n'; res.end(testResBody); })); + server.listen(0, common.mustCall(() => { + const req = http.request({ + port: server.address().port, path: '/' + }); + debug('Client sending request...'); + + req.on('information', common.mustCall((res) => { + assert.strictEqual( + res.headers.link, + '; rel=preload; as=style' + ); + })); + + req.on('response', common.mustCall((res) => { + let body = ''; + + assert.strictEqual(res.statusCode, 200, `Final status code was ${res.statusCode}, not 200.`); + + res.on('data', (chunk) => { + body += chunk; + }); + + res.on('end', common.mustCall(() => { + debug('Got full response.'); + assert.strictEqual(body, testResBody); + server.close(); + })); + })); + + req.end(); + })); +} + +{ + // Happy flow - object argument with array of strings + + const server = http.createServer(common.mustCall((req, res) => { + debug('Server sending early hints...'); + res.writeEarlyHints({ + 'link': [ + '; rel=preload; as=style', + '; rel=preload; as=script', + ], + 'x-trace-id': 'id for diagnostics' + }); + + debug('Server sending full response...'); + res.end(testResBody); + })); + server.listen(0, common.mustCall(() => { const req = http.request({ port: server.address().port, path: '/' From 335008df6cf357dbf63fc9390eb968b2a26c87a1 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 29 Sep 2022 10:14:16 -0400 Subject: [PATCH 3/7] docs: update documentation --- doc/api/http.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/api/http.md b/doc/api/http.md index d844d7663464aa..755afa374c72dd 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -2143,7 +2143,7 @@ the request body should be sent. See the [`'checkContinue'`][] event on added: REPLACEME --> -* `links` {string|Array} +* `links` {string|Array|Object} * `callback` {Function} Sends an HTTP/1.1 103 Early Hints message to the client with a Link header, @@ -2152,6 +2152,9 @@ The `links` can be a string or an array of strings containing the values of the `Link` header. The optional `callback` argument will be called when the response message has been written. +When links are of type object, the `link` key will be used to set the values +of the `Link` header. Any additional attributes will be used as a header. + **Example** ```js From 257dd1e76891ef565994ebcaa8171216fbc47eed Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 29 Sep 2022 10:21:02 -0400 Subject: [PATCH 4/7] fix: properly set additional headers --- lib/_http_server.js | 2 +- lib/internal/http2/compat.js | 2 +- test/parallel/test-http-early-hints.js | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index 2f5c9c0287c78d..f781ce84f51082 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -363,7 +363,7 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(links, cb) { continue; } - head += key + ', ' + keys[key] + '\r\n'; + head += key + ': ' + links[key] + '\r\n'; } } else { throw new ERR_INVALID_ARG_VALUE( diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 7c7de7187b14bd..a4e99e91728c71 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -908,7 +908,7 @@ class Http2ServerResponse extends Stream { continue; } - headers[key] = keys[key]; + headers[key] = links[key]; } } else { throw new ERR_INVALID_ARG_VALUE( diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js index 82369785f2e918..fdbb3e7430956c 100644 --- a/test/parallel/test-http-early-hints.js +++ b/test/parallel/test-http-early-hints.js @@ -159,6 +159,7 @@ const testResBody = 'response content\n'; res.headers.link, '; rel=preload; as=style' ); + assert.strictEqual(res.headers['x-trace-id'], 'id for diagnostics'); })); req.on('response', common.mustCall((res) => { @@ -209,6 +210,7 @@ const testResBody = 'response content\n'; res.headers.link, '; rel=preload; as=style, ; rel=preload; as=script' ); + assert.strictEqual(res.headers['x-trace-id'], 'id for diagnostics'); })); req.on('response', common.mustCall((res) => { From 1beeff78f04322c291beb4bfac0bc4a0be1be428 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 29 Sep 2022 15:16:14 -0400 Subject: [PATCH 5/7] refactor: reuse functions and improve readability --- lib/_http_server.js | 78 ++++++++---------------------------- lib/internal/http2/compat.js | 75 ++++++---------------------------- lib/internal/validators.js | 44 +++++++++++++++++++- 3 files changed, 70 insertions(+), 127 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index f781ce84f51082..731fd3c39ffee2 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -81,7 +81,7 @@ const { const { validateInteger, validateBoolean, - validateLinkHeaderValue + validateLinkHeaderValue, } = require('internal/validators'); const Buffer = require('buffer').Buffer; const { setInterval, clearInterval } = require('timers'); @@ -296,81 +296,35 @@ ServerResponse.prototype.writeProcessing = function writeProcessing(cb) { this._writeRaw('HTTP/1.1 102 Processing\r\n\r\n', 'ascii', cb); }; -ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(links, cb) { +ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) { let head = 'HTTP/1.1 103 Early Hints\r\n'; - if (typeof links === 'string') { - validateLinkHeaderValue(links, 'links'); - head += 'Link: ' + links + '\r\n'; - } else if (ArrayIsArray(links)) { - if (!links.length) { + if (!ArrayIsArray(hints) && typeof hints === 'object') { + if (hints.link === null || hints.link === undefined) { return; } - head += 'Link: '; + const link = validateLinkHeaderValue(hints.link); - for (let i = 0; i < links.length; i++) { - const link = links[i]; - validateLinkHeaderValue(link, 'links'); - head += link; - - if (i !== links.length - 1) { - head += ', '; - } - } - - head += '\r\n'; - } else if (typeof links === 'object') { - const keys = ObjectKeys(links); - - if (!keys.length) { + if (link.length === 0) { return; } - const link = links.link; + head += 'Link: ' + link + '\r\n'; - if (typeof link === 'string') { - validateLinkHeaderValue(link, 'links'); - head += 'Link: ' + link + '\r\n'; - } else if (ArrayIsArray(link)) { - const linkLength = link.length; - if (!linkLength) { - return; - } - - head += 'Link: '; - - for (let i = 0; i < linkLength; i++) { - validateLinkHeaderValue(link[i], 'links'); - head += link[i]; - - if (i !== linkLength - 1) { - head += ', '; - } + for (const key of ObjectKeys(hints)) { + if (key !== 'link') { + head += key + ': ' + hints[key] + '\r\n'; } - - head += '\r\n'; - } else { - throw new ERR_INVALID_ARG_VALUE( - 'links', - links, - 'must be an array, object or string of format "; rel=preload; as=style"' - ); } + } else { + const link = validateLinkHeaderValue(hints); - for (const key of keys) { - if (key === 'link') { - continue; - } - - head += key + ': ' + links[key] + '\r\n'; + if (link.length === 0) { + return; } - } else { - throw new ERR_INVALID_ARG_VALUE( - 'links', - links, - 'must be an array, object or string of format "; rel=preload; as=style"' - ); + + head += 'Link: ' + link + '\r\n'; } head += '\r\n'; diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index a4e99e91728c71..dd464104fac2b2 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -847,75 +847,24 @@ class Http2ServerResponse extends Stream { return true; } - writeEarlyHints(links) { + writeEarlyHints(hints) { const headers = ObjectCreate(null); let linkHeaderValue = ''; - if (typeof links === 'string') { - validateLinkHeaderValue(links, 'links'); - linkHeaderValue += links; - } else if (ArrayIsArray(links)) { - if (!links.length) { - return; - } - - linkHeaderValue += ''; - - for (let i = 0; i < links.length; i++) { - const link = links[i]; - validateLinkHeaderValue(link, 'links'); - linkHeaderValue += link; - - if (i !== links.length - 1) { - linkHeaderValue += ', '; - } - } - } else if (typeof links === 'object') { - const keys = ObjectKeys(links); - - if (!keys.length) { - return; - } - - const link = links.link; + if (!ArrayIsArray(hints) && typeof hints === 'object') { + linkHeaderValue += validateLinkHeaderValue(hints.link); - if (typeof link === 'string') { - validateLinkHeaderValue(link, 'links'); - linkHeaderValue += link; - } else if (ArrayIsArray(link)) { - if (!link.length) { - return; + for (const key of ObjectKeys(hints)) { + if (key !== 'link') { + headers[key] = hints[key]; } - - for (let i = 0; i < link.length; i++) { - validateLinkHeaderValue(link[i]); - linkHeaderValue += link; - - if (i !== link.length - 1) { - linkHeaderValue += ', '; - } - } - } else { - throw new ERR_INVALID_ARG_VALUE( - 'links', - links, - 'must be an array, object or string of format "; rel=preload; as=style"' - ); - } - - for (const key of keys) { - if (key === 'link') { - continue; - } - - headers[key] = links[key]; } } else { - throw new ERR_INVALID_ARG_VALUE( - 'links', - links, - 'must be an array, object or string of format "; rel=preload; as=style"' - ); + linkHeaderValue += validateLinkHeaderValue(hints); + } + + if (linkHeaderValue.length === 0) { + return false; } const stream = this[kStream]; @@ -924,9 +873,9 @@ class Http2ServerResponse extends Stream { return false; stream.additionalHeaders({ + ...headers, [HTTP2_HEADER_STATUS]: HTTP_STATUS_EARLY_HINTS, 'Link': linkHeaderValue, - ...headers }); return true; diff --git a/lib/internal/validators.js b/lib/internal/validators.js index a192f9e6302ddb..e531104914a03f 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -403,9 +403,13 @@ function validateUnion(value, name, union) { } } -function validateLinkHeaderValue(value, name) { - const linkValueRegExp = /^(?:<[^>]*>;)\s*(?:rel=(")?[^;"]*\1;?)\s*(?:(?:as|anchor|title)=(")?[^;"]*\2)?$/; +const linkValueRegExp = /^(?:<[^>]*>;)\s*(?:rel=(")?[^;"]*\1;?)\s*(?:(?:as|anchor|title)=(")?[^;"]*\2)?$/; +/** + * @param {any} value + * @param {string} name + */ +function validateLinkHeaderFormat(value, name) { if ( typeof value === 'undefined' || !RegExpPrototypeExec(linkValueRegExp, value) @@ -424,6 +428,42 @@ const validateInternalField = hideStackFrames((object, fieldKey, className) => { } }); +/** + * @param {any} hints + * @return {string} + */ +function validateLinkHeaderValue(hints) { + if (typeof hints === 'string') { + validateLinkHeaderFormat(hints, 'hints'); + return hints; + } else if (ArrayIsArray(hints)) { + const hintsLength = hints.length; + let result = ''; + + if (hintsLength === 0) { + return result; + } + + for (let i = 0; i < hintsLength; i++) { + const link = hints[i]; + validateLinkHeaderFormat(link, 'hints'); + result += link; + + if (i !== hintsLength - 1) { + result += ', '; + } + } + + return result; + } + + throw new ERR_INVALID_ARG_VALUE( + 'hints', + hints, + 'must be an array or string of format "; rel=preload; as=style"' + ); +} + module.exports = { isInt32, isUint32, From 9fddf8effd92b6cb11e818ef72daa35ac6f044a3 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Fri, 30 Sep 2022 16:25:53 -0400 Subject: [PATCH 6/7] docs: update writeEarlyHints changes --- doc/api/http.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/http.md b/doc/api/http.md index 755afa374c72dd..dde23a36bd24d8 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -2141,6 +2141,10 @@ the request body should be sent. See the [`'checkContinue'`][] event on * `links` {string|Array|Object} From 25f2f3e9d1fe5f3884a9c11e3dd03d2b6ffe9409 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Tue, 4 Oct 2022 14:34:13 +0100 Subject: [PATCH 7/7] http,http2: update writeEarlyHints to use object --- doc/api/http.md | 20 +++++---- lib/_http_server.js | 33 ++++++-------- lib/internal/http2/compat.js | 16 +++---- .../test-http-early-hints-invalid-argument.js | 4 +- test/parallel/test-http-early-hints.js | 18 +++++--- ...write-early-hints-invalid-argument-type.js | 44 ++++++++++--------- ...rite-early-hints-invalid-argument-value.js | 42 ++++++++++++++++++ ...mpat-write-early-hints-invalid-argument.js | 38 ---------------- .../test-http2-compat-write-early-hints.js | 18 +++++--- 9 files changed, 123 insertions(+), 110 deletions(-) create mode 100644 test/parallel/test-http2-compat-write-early-hints-invalid-argument-value.js delete mode 100644 test/parallel/test-http2-compat-write-early-hints-invalid-argument.js diff --git a/doc/api/http.md b/doc/api/http.md index dde23a36bd24d8..99f6f5700ca866 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -2137,7 +2137,7 @@ Sends an HTTP/1.1 100 Continue message to the client, indicating that the request body should be sent. See the [`'checkContinue'`][] event on `Server`. -### `response.writeEarlyHints(links[, callback])` +### `response.writeEarlyHints(hints[, callback])` -* `links` {string|Array|Object} +* `hints` {Object} * `callback` {Function} Sends an HTTP/1.1 103 Early Hints message to the client with a Link header, indicating that the user agent can preload/preconnect the linked resources. -The `links` can be a string or an array of strings containing the values -of the `Link` header. The optional `callback` argument will be called when +The `hints` is an object containing the values of headers to be sent with +early hints message. The optional `callback` argument will be called when the response message has been written. -When links are of type object, the `link` key will be used to set the values -of the `Link` header. Any additional attributes will be used as a header. - **Example** ```js const earlyHintsLink = '; rel=preload; as=style'; -response.writeEarlyHints(earlyHintsLink); +response.writeEarlyHints({ + 'link': earlyHintsLink, +}); const earlyHintsLinks = [ '; rel=preload; as=style', '; rel=preload; as=script', ]; -response.writeEarlyHints(earlyHintsLinks); +response.writeEarlyHints({ + 'link': earlyHintsLinks, + 'x-trace-id': 'id for diagnostics' +}); const earlyHintsCallback = () => console.log('early hints message sent'); response.writeEarlyHints(earlyHintsLinks, earlyHintsCallback); diff --git a/lib/_http_server.js b/lib/_http_server.js index 731fd3c39ffee2..f1886a213a84b6 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -82,6 +82,7 @@ const { validateInteger, validateBoolean, validateLinkHeaderValue, + validateObject } = require('internal/validators'); const Buffer = require('buffer').Buffer; const { setInterval, clearInterval } = require('timers'); @@ -299,32 +300,24 @@ ServerResponse.prototype.writeProcessing = function writeProcessing(cb) { ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) { let head = 'HTTP/1.1 103 Early Hints\r\n'; - if (!ArrayIsArray(hints) && typeof hints === 'object') { - if (hints.link === null || hints.link === undefined) { - return; - } + validateObject(hints, 'hints'); - const link = validateLinkHeaderValue(hints.link); + if (hints.link === null || hints.link === undefined) { + return; + } - if (link.length === 0) { - return; - } + const link = validateLinkHeaderValue(hints.link); - head += 'Link: ' + link + '\r\n'; + if (link.length === 0) { + return; + } - for (const key of ObjectKeys(hints)) { - if (key !== 'link') { - head += key + ': ' + hints[key] + '\r\n'; - } - } - } else { - const link = validateLinkHeaderValue(hints); + head += 'Link: ' + link + '\r\n'; - if (link.length === 0) { - return; + for (const key of ObjectKeys(hints)) { + if (key !== 'link') { + head += key + ': ' + hints[key] + '\r\n'; } - - head += 'Link: ' + link + '\r\n'; } head += '\r\n'; diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index dd464104fac2b2..8ece2d912c6e9d 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -57,6 +57,7 @@ const { validateFunction, validateString, validateLinkHeaderValue, + validateObject, } = require('internal/validators'); const { kSocket, @@ -848,19 +849,16 @@ class Http2ServerResponse extends Stream { } writeEarlyHints(hints) { + validateObject(hints, 'hints'); + const headers = ObjectCreate(null); - let linkHeaderValue = ''; - if (!ArrayIsArray(hints) && typeof hints === 'object') { - linkHeaderValue += validateLinkHeaderValue(hints.link); + const linkHeaderValue = validateLinkHeaderValue(hints.link); - for (const key of ObjectKeys(hints)) { - if (key !== 'link') { - headers[key] = hints[key]; - } + for (const key of ObjectKeys(hints)) { + if (key !== 'link') { + headers[key] = hints[key]; } - } else { - linkHeaderValue += validateLinkHeaderValue(hints); } if (linkHeaderValue.length === 0) { diff --git a/test/parallel/test-http-early-hints-invalid-argument.js b/test/parallel/test-http-early-hints-invalid-argument.js index 62b83556fef383..2cf29666bef11d 100644 --- a/test/parallel/test-http-early-hints-invalid-argument.js +++ b/test/parallel/test-http-early-hints-invalid-argument.js @@ -8,7 +8,7 @@ const testResBody = 'response content\n'; const server = http.createServer(common.mustCall((req, res) => { debug('Server sending early hints...'); - res.writeEarlyHints('bad argument value'); + res.writeEarlyHints('bad argument type'); debug('Server sending full response...'); res.end(testResBody); @@ -27,7 +27,7 @@ server.listen(0, common.mustCall(() => { 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'); + assert.strictEqual(err.code, 'ERR_INVALID_ARG_TYPE'); process.exit(0); }); })); diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js index fdbb3e7430956c..7183d9516f6dda 100644 --- a/test/parallel/test-http-early-hints.js +++ b/test/parallel/test-http-early-hints.js @@ -11,7 +11,9 @@ const testResBody = 'response content\n'; const server = http.createServer(common.mustCall((req, res) => { debug('Server sending early hints...'); - res.writeEarlyHints('; rel=preload; as=style'); + res.writeEarlyHints({ + link: '; rel=preload; as=style' + }); debug('Server sending full response...'); res.end(testResBody); @@ -53,10 +55,12 @@ const testResBody = 'response content\n'; const server = http.createServer(common.mustCall((req, res) => { debug('Server sending early hints...'); - res.writeEarlyHints([ - '; rel=preload; as=style', - '; rel=preload; as=script', - ]); + res.writeEarlyHints({ + link: [ + '; rel=preload; as=style', + '; rel=preload; as=script', + ] + }); debug('Server sending full response...'); res.end(testResBody); @@ -100,7 +104,9 @@ const testResBody = 'response content\n'; const server = http.createServer(common.mustCall((req, res) => { debug('Server sending early hints...'); - res.writeEarlyHints([]); + res.writeEarlyHints({ + link: [] + }); debug('Server sending full response...'); res.end(testResBody); diff --git a/test/parallel/test-http2-compat-write-early-hints-invalid-argument-type.js b/test/parallel/test-http2-compat-write-early-hints-invalid-argument-type.js index cd5013fd97f29b..caf5824e072571 100644 --- a/test/parallel/test-http2-compat-write-early-hints-invalid-argument-type.js +++ b/test/parallel/test-http2-compat-write-early-hints-invalid-argument-type.js @@ -9,30 +9,34 @@ const debug = require('node:util').debuglog('test'); const testResBody = 'response content'; -const server = http2.createServer(); +{ + // Invalid object value -server.on('request', common.mustCall((req, res) => { - debug('Server sending early hints...'); - res.writeEarlyHints({ links: 'bad argument object' }); + const server = http2.createServer(); - debug('Server sending full response...'); - res.end(testResBody); -})); + server.on('request', common.mustCall((req, res) => { + debug('Server sending early hints...'); + res.writeEarlyHints('this should not be here'); -server.listen(0); + debug('Server sending full response...'); + res.end(testResBody); + })); -server.on('listening', common.mustCall(() => { - const client = http2.connect(`http://localhost:${server.address().port}`); - const req = client.request(); + server.listen(0); - debug('Client sending request...'); + server.on('listening', common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request(); - req.on('headers', common.mustNotCall()); + debug('Client sending request...'); - 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(0); - }); -})); + req.on('headers', 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); + }); + })); +} diff --git a/test/parallel/test-http2-compat-write-early-hints-invalid-argument-value.js b/test/parallel/test-http2-compat-write-early-hints-invalid-argument-value.js new file mode 100644 index 00000000000000..d640f13fae6f5e --- /dev/null +++ b/test/parallel/test-http2-compat-write-early-hints-invalid-argument-value.js @@ -0,0 +1,42 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) common.skip('missing crypto'); + +const assert = require('node:assert'); +const http2 = require('node:http2'); +const debug = require('node:util').debuglog('test'); + +const testResBody = 'response content'; + +{ + // Invalid link header value + + const server = http2.createServer(); + + server.on('request', common.mustCall((req, res) => { + debug('Server sending early hints...'); + res.writeEarlyHints({ link: BigInt(100) }); + + debug('Server sending full response...'); + res.end(testResBody); + })); + + server.listen(0); + + server.on('listening', common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request(); + + debug('Client sending request...'); + + req.on('headers', 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(0); + }); + })); +} diff --git a/test/parallel/test-http2-compat-write-early-hints-invalid-argument.js b/test/parallel/test-http2-compat-write-early-hints-invalid-argument.js deleted file mode 100644 index 1179723e1c883e..00000000000000 --- a/test/parallel/test-http2-compat-write-early-hints-invalid-argument.js +++ /dev/null @@ -1,38 +0,0 @@ -'use strict'; - -const common = require('../common'); -if (!common.hasCrypto) common.skip('missing crypto'); - -const assert = require('node:assert'); -const http2 = require('node:http2'); -const debug = require('node:util').debuglog('test'); - -const testResBody = 'response content'; - -const server = http2.createServer(); - -server.on('request', common.mustCall((req, res) => { - debug('Server sending early hints...'); - res.writeEarlyHints('bad argument value'); - - debug('Server sending full response...'); - res.end(testResBody); -})); - -server.listen(0); - -server.on('listening', common.mustCall(() => { - const client = http2.connect(`http://localhost:${server.address().port}`); - const req = client.request(); - - debug('Client sending request...'); - - req.on('headers', 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(0); - }); -})); diff --git a/test/parallel/test-http2-compat-write-early-hints.js b/test/parallel/test-http2-compat-write-early-hints.js index da6cd7446bddcd..d1f26d7c20bbc0 100644 --- a/test/parallel/test-http2-compat-write-early-hints.js +++ b/test/parallel/test-http2-compat-write-early-hints.js @@ -16,7 +16,9 @@ const testResBody = 'response content'; server.on('request', common.mustCall((req, res) => { debug('Server sending early hints...'); - res.writeEarlyHints('; rel=preload; as=style'); + res.writeEarlyHints({ + link: '; rel=preload; as=style' + }); debug('Server sending full response...'); res.end(testResBody); @@ -59,10 +61,12 @@ const testResBody = 'response content'; server.on('request', common.mustCall((req, res) => { debug('Server sending early hints...'); - res.writeEarlyHints([ - '; rel=preload; as=style', - '; rel=preload; as=script', - ]); + res.writeEarlyHints({ + link: [ + '; rel=preload; as=style', + '; rel=preload; as=script', + ] + }); debug('Server sending full response...'); res.end(testResBody); @@ -108,7 +112,9 @@ const testResBody = 'response content'; server.on('request', common.mustCall((req, res) => { debug('Server sending early hints...'); - res.writeEarlyHints([]); + res.writeEarlyHints({ + link: [] + }); debug('Server sending full response...'); res.end(testResBody);