Skip to content

Commit

Permalink
http: Relax writeEarlyHints validations
Browse files Browse the repository at this point in the history 
Removes the requirement that every call to writeEarlyHints include a
`link` header. While the `link` header is clearly the most common usage
of `103 Early Hints`, I could find no requirement to include a `link`
header as part of [RFC8297](https://www.rfc-editor.org/rfc/rfc8297.html).

Additionally this removes the existing incorrect validation of the Link
header format in favor of only validating that it is a valid header
value. While the validation could be updated to better match [RFC8288
Section 3](https://www.rfc-editor.org/rfc/rfc8288.html#section-3), it
appears it would be the only place in the node.js code base where we
proactively validate header values beyond verifying they are valid at
the HTTP protocol layer.

Fixes: nodejs#46453
  • Loading branch information
@khalsah
khalsah committed Feb 1, 2023
1 parent dc90810 commit 1e641e5
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 186 deletions.
27 changes: 12 additions & 15 deletions lib/_http_server.js
View file
Expand Up @@ -51,7 +51,9 @@ const { ConnectionsList } = internalBinding('http_parser');
const {
kUniqueHeaders,
parseUniqueHeadersOption,
OutgoingMessage
OutgoingMessage,
validateHeaderName,
validateHeaderValue,
} = require('_http_outgoing');
const {
kOutHeaders,
Expand Down Expand Up @@ -84,7 +86,6 @@ const {
const {
validateInteger,
validateBoolean,
validateLinkHeaderValue,
validateObject
} = require('internal/validators');
const Buffer = require('buffer').Buffer;
Expand Down Expand Up @@ -305,20 +306,16 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) {

validateObject(hints, 'hints');

if (hints.link === null || hints.link === undefined) {
return;
}

const link = validateLinkHeaderValue(hints.link);

if (link.length === 0) {
return;
}

head += 'Link: ' + link + '\r\n';

for (const key of ObjectKeys(hints)) {
if (key !== 'link') {
const hint = hints[key];
validateHeaderName(key);
validateHeaderValue(key, hint);

if (ArrayIsArray(hint)) {
for (let i = 0; i < hint.length; i++) {
head += key + ': ' + hint[i] + '\r\n';
}
} else {
head += key + ': ' + hints[key] + '\r\n';
}
}
Expand Down
18 changes: 1 addition & 17 deletions lib/internal/http2/compat.js
View file
Expand Up @@ -55,7 +55,6 @@ const {
const {
validateFunction,
validateString,
validateLinkHeaderValue,
validateObject,
} = require('internal/validators');
const {
Expand Down Expand Up @@ -850,29 +849,14 @@ class Http2ServerResponse extends Stream {
writeEarlyHints(hints) {
validateObject(hints, 'hints');

const headers = { __proto__: null };

const linkHeaderValue = validateLinkHeaderValue(hints.link);

for (const key of ObjectKeys(hints)) {
if (key !== 'link') {
headers[key] = hints[key];
}
}

if (linkHeaderValue.length === 0) {
return false;
}

const stream = this[kStream];

if (stream.headersSent || this[kState].closed)
return false;

stream.additionalHeaders({
...headers,
...hints,
[HTTP2_HEADER_STATUS]: HTTP_STATUS_EARLY_HINTS,
'Link': linkHeaderValue,
});

return true;
Expand Down
56 changes: 0 additions & 56 deletions lib/internal/validators.js
View file
Expand Up @@ -459,67 +459,12 @@ function validateUnion(value, name, union) {
}
}

const linkValueRegExp = /^(?:<[^>]*>;)\s*(?:rel=(")?[^;"]*\1;?)\s*(?:(?:as|anchor|title|crossorigin|disabled|fetchpriority|rel|referrerpolicy)=(")?[^;"]*\2)?$/;

/**
* @param {any} value
* @param {string} name
*/
function validateLinkHeaderFormat(value, name) {
if (
typeof value === 'undefined' ||
!RegExpPrototypeExec(linkValueRegExp, value)
) {
throw new ERR_INVALID_ARG_VALUE(
name,
value,
'must be an array or string of format "</styles.css>; rel=preload; as=style"'
);
}
}

const validateInternalField = hideStackFrames((object, fieldKey, className) => {
if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, fieldKey)) {
throw new ERR_INVALID_ARG_TYPE('this', className, object);
}
});

/**
* @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 "</styles.css>; rel=preload; as=style"'
);
}

module.exports = {
isInt32,
isUint32,
Expand All @@ -545,6 +490,5 @@ module.exports = {
validateUndefined,
validateUnion,
validateAbortSignal,
validateLinkHeaderValue,
validateInternalField,
};
45 changes: 3 additions & 42 deletions test/parallel/test-http-early-hints.js
View file
Expand Up @@ -99,47 +99,6 @@ const testResBody = 'response content\n';
}));
}

{
// Happy flow - empty array

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: '/'
});
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

Expand Down Expand Up @@ -256,7 +215,9 @@ const testResBody = 'response content\n';
});
debug('Client sending request...');

req.on('information', common.mustNotCall());
req.on('information', common.mustCall((info) => {
assert.strictEqual(info.statusCode, 103)
}));

req.on('response', common.mustCall((res) => {
let body = '';
Expand Down
42 changes: 0 additions & 42 deletions test/parallel/test-http2-compat-write-early-hints-invalid-argument-value.js
View file

This file was deleted.

4 changes: 3 additions & 1 deletion test/parallel/test-http2-compat-write-early-hints.js
View file
Expand Up @@ -128,7 +128,9 @@ const testResBody = 'response content';

debug('Client sending request...');

req.on('headers', common.mustNotCall());
req.on('headers', common.mustCall((headers) => {
assert.strictEqual(headers[':status'], 103);
}));

req.on('response', common.mustCall((headers) => {
assert.strictEqual(headers[':status'], 200);
Expand Down
13 changes: 0 additions & 13 deletions test/parallel/test-validators.js
View file
Expand Up @@ -12,7 +12,6 @@ const {
validateString,
validateInt32,
validateUint32,
validateLinkHeaderValue,
} = require('internal/validators');
const { MAX_SAFE_INTEGER, MIN_SAFE_INTEGER } = Number;
const outOfRangeError = {
Expand Down Expand Up @@ -155,15 +154,3 @@ const invalidArgValueError = {
code: 'ERR_INVALID_ARG_TYPE'
}));
}

{
// validateLinkHeaderValue type validation.
[
['</styles.css>; rel=preload; as=style', '</styles.css>; rel=preload; as=style'],
['</styles.css>; rel=preload; title=hello', '</styles.css>; rel=preload; title=hello'],
['</styles.css>; rel=preload; crossorigin=hello', '</styles.css>; rel=preload; crossorigin=hello'],
['</styles.css>; rel=preload; disabled=true', '</styles.css>; rel=preload; disabled=true'],
['</styles.css>; rel=preload; fetchpriority=high', '</styles.css>; rel=preload; fetchpriority=high'],
['</styles.css>; rel=preload; referrerpolicy=origin', '</styles.css>; rel=preload; referrerpolicy=origin'],
].forEach(([value, expected]) => assert.strictEqual(validateLinkHeaderValue(value), expected));
}

0 comments on commit 1e641e5

Please sign in to comment.