From 9c7e66478ca21746f749b5b433266e7bed857e24 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 6 Oct 2022 18:03:47 +0100 Subject: [PATCH] http2: make early hints generic PR-URL: https://github.com/nodejs/node/pull/44820 Fixes: https://github.com/nodejs/node/issues/44816 Reviewed-By: Matteo Collina Reviewed-By: Minwoo Jung Reviewed-By: James M Snell Reviewed-By: Rafael Gonzaga --- doc/api/http.md | 21 ++- lib/_http_server.js | 42 ++--- lib/internal/http2/compat.js | 39 ++--- lib/internal/validators.js | 44 ++++- ...-http-early-hints-invalid-argument-type.js | 33 ---- .../test-http-early-hints-invalid-argument.js | 4 +- test/parallel/test-http-early-hints.js | 156 +++++++++++++++++- ...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 +- 11 files changed, 318 insertions(+), 163 deletions(-) delete mode 100644 test/parallel/test-http-early-hints-invalid-argument-type.js 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 0576ab54a3b8e0..05cf220ac06af7 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -2124,32 +2124,41 @@ 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} +* `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. **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 caf22580755f37..58e6f24696d9eb 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -81,7 +81,8 @@ const { const { validateInteger, validateBoolean, - validateLinkHeaderValue + validateLinkHeaderValue, + validateObject } = require('internal/validators'); const Buffer = require('buffer').Buffer; const { @@ -301,36 +302,27 @@ 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) { - return; - } + validateObject(hints, 'hints'); - head += 'Link: '; + if (hints.link === null || hints.link === undefined) { + return; + } - for (let i = 0; i < links.length; i++) { - const link = links[i]; - validateLinkHeaderValue(link, 'links'); - head += link; + const link = validateLinkHeaderValue(hints.link); - if (i !== links.length - 1) { - head += ', '; - } - } + if (link.length === 0) { + return; + } - head += '\r\n'; - } else { - throw new ERR_INVALID_ARG_VALUE( - 'links', - links, - 'must be an array or string of format "; rel=preload; as=style"' - ); + head += 'Link: ' + link + '\r\n'; + + for (const key of ObjectKeys(hints)) { + if (key !== 'link') { + head += key + ': ' + hints[key] + '\r\n'; + } } head += '\r\n'; diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 727192e23fade2..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, @@ -847,34 +848,21 @@ class Http2ServerResponse extends Stream { return true; } - writeEarlyHints(links) { - let linkHeaderValue = ''; + writeEarlyHints(hints) { + validateObject(hints, 'hints'); - if (typeof links === 'string') { - validateLinkHeaderValue(links, 'links'); - linkHeaderValue += links; - } else if (ArrayIsArray(links)) { - if (!links.length) { - return; - } - - linkHeaderValue += ''; + const headers = ObjectCreate(null); - for (let i = 0; i < links.length; i++) { - const link = links[i]; - validateLinkHeaderValue(link, 'links'); - linkHeaderValue += link; + const linkHeaderValue = validateLinkHeaderValue(hints.link); - if (i !== links.length - 1) { - linkHeaderValue += ', '; - } + for (const key of ObjectKeys(hints)) { + if (key !== 'link') { + headers[key] = hints[key]; } - } else { - throw new ERR_INVALID_ARG_VALUE( - 'links', - links, - 'must be an array or string of format "; rel=preload; as=style"' - ); + } + + if (linkHeaderValue.length === 0) { + return false; } const stream = this[kStream]; @@ -883,8 +871,9 @@ class Http2ServerResponse extends Stream { return false; stream.additionalHeaders({ + ...headers, [HTTP2_HEADER_STATUS]: HTTP_STATUS_EARLY_HINTS, - 'Link': linkHeaderValue + 'Link': linkHeaderValue, }); return true; diff --git a/lib/internal/validators.js b/lib/internal/validators.js index de8a8bb9b83b34..dae0f9bc056b3b 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) @@ -418,6 +422,42 @@ function validateLinkHeaderValue(value, name) { } } +/** + * @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, 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-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 975be2e112a5b3..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,147 @@ 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); + })); + + 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(); + })); +} + +{ + // Happy flow - object argument with string + + 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' + ); + assert.strictEqual(res.headers['x-trace-id'], 'id for diagnostics'); + })); + + 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: '/' + }); + debug('Client sending request...'); + + req.on('information', common.mustCall((res) => { + assert.strictEqual( + 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) => { + 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); 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);