Skip to content

Commit 1e641e5

Browse files
khalsahkhalsah
committed
http: Relax writeEarlyHints validations
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: #46453
1 parent dc90810 commit 1e641e5

File tree

7 files changed

+19
-186
lines changed

7 files changed

+19
-186
lines changed

‎lib/_http_server.js‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ const { ConnectionsList } = internalBinding('http_parser');
5151
const {
5252
kUniqueHeaders,
5353
parseUniqueHeadersOption,
54-
OutgoingMessage
54+
OutgoingMessage,
55+
validateHeaderName,
56+
validateHeaderValue,
5557
} = require('_http_outgoing');
5658
const {
5759
kOutHeaders,
@@ -84,7 +86,6 @@ const {
8486
const {
8587
validateInteger,
8688
validateBoolean,
87-
validateLinkHeaderValue,
8889
validateObject
8990
} = require('internal/validators');
9091
const Buffer = require('buffer').Buffer;
@@ -305,20 +306,16 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) {
305306

306307
validateObject(hints, 'hints');
307308

308-
if (hints.link === null || hints.link === undefined) {
309-
return;
310-
}
311-
312-
const link = validateLinkHeaderValue(hints.link);
313-
314-
if (link.length === 0) {
315-
return;
316-
}
317-
318-
head += 'Link: ' + link + '\r\n';
319-
320309
for (const key of ObjectKeys(hints)) {
321-
if (key !== 'link') {
310+
const hint = hints[key];
311+
validateHeaderName(key);
312+
validateHeaderValue(key, hint);
313+
314+
if (ArrayIsArray(hint)) {
315+
for (let i = 0; i < hint.length; i++) {
316+
head += key + ': ' + hint[i] + '\r\n';
317+
}
318+
} else {
322319
head += key + ': ' + hints[key] + '\r\n';
323320
}
324321
}

‎lib/internal/http2/compat.js‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ const {
5555
const {
5656
validateFunction,
5757
validateString,
58-
validateLinkHeaderValue,
5958
validateObject,
6059
} = require('internal/validators');
6160
const {
@@ -850,29 +849,14 @@ class Http2ServerResponse extends Stream {
850849
writeEarlyHints(hints) {
851850
validateObject(hints, 'hints');
852851

853-
const headers = { __proto__: null };
854-
855-
const linkHeaderValue = validateLinkHeaderValue(hints.link);
856-
857-
for (const key of ObjectKeys(hints)) {
858-
if (key !== 'link') {
859-
headers[key] = hints[key];
860-
}
861-
}
862-
863-
if (linkHeaderValue.length === 0) {
864-
return false;
865-
}
866-
867852
const stream = this[kStream];
868853

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

872857
stream.additionalHeaders({
873-
...headers,
858+
...hints,
874859
[HTTP2_HEADER_STATUS]: HTTP_STATUS_EARLY_HINTS,
875-
'Link': linkHeaderValue,
876860
});
877861

878862
return true;

‎lib/internal/validators.js‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -459,67 +459,12 @@ function validateUnion(value, name, union) {
459459
}
460460
}
461461

462-
const linkValueRegExp = /^(?:<[^>]*>;)\s*(?:rel=(")?[^;"]*\1;?)\s*(?:(?:as|anchor|title|crossorigin|disabled|fetchpriority|rel|referrerpolicy)=(")?[^;"]*\2)?$/;
463-
464-
/**
465-
* @param {any} value
466-
* @param {string} name
467-
*/
468-
function validateLinkHeaderFormat(value, name) {
469-
if (
470-
typeof value === 'undefined' ||
471-
!RegExpPrototypeExec(linkValueRegExp, value)
472-
) {
473-
throw new ERR_INVALID_ARG_VALUE(
474-
name,
475-
value,
476-
'must be an array or string of format "</styles.css>; rel=preload; as=style"'
477-
);
478-
}
479-
}
480-
481462
const validateInternalField = hideStackFrames((object, fieldKey, className) => {
482463
if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, fieldKey)) {
483464
throw new ERR_INVALID_ARG_TYPE('this', className, object);
484465
}
485466
});
486467

487-
/**
488-
* @param {any} hints
489-
* @return {string}
490-
*/
491-
function validateLinkHeaderValue(hints) {
492-
if (typeof hints === 'string') {
493-
validateLinkHeaderFormat(hints, 'hints');
494-
return hints;
495-
} else if (ArrayIsArray(hints)) {
496-
const hintsLength = hints.length;
497-
let result = '';
498-
499-
if (hintsLength === 0) {
500-
return result;
501-
}
502-
503-
for (let i = 0; i < hintsLength; i++) {
504-
const link = hints[i];
505-
validateLinkHeaderFormat(link, 'hints');
506-
result += link;
507-
508-
if (i !== hintsLength - 1) {
509-
result += ', ';
510-
}
511-
}
512-
513-
return result;
514-
}
515-
516-
throw new ERR_INVALID_ARG_VALUE(
517-
'hints',
518-
hints,
519-
'must be an array or string of format "</styles.css>; rel=preload; as=style"'
520-
);
521-
}
522-
523468
module.exports = {
524469
isInt32,
525470
isUint32,
@@ -545,6 +490,5 @@ module.exports = {
545490
validateUndefined,
546491
validateUnion,
547492
validateAbortSignal,
548-
validateLinkHeaderValue,
549493
validateInternalField,
550494
};

‎test/parallel/test-http-early-hints.js‎

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -99,47 +99,6 @@ const testResBody = 'response content\n';
9999
}));
100100
}
101101

102-
{
103-
// Happy flow - empty array
104-
105-
const server = http.createServer(common.mustCall((req, res) => {
106-
debug('Server sending early hints...');
107-
res.writeEarlyHints({
108-
link: []
109-
});
110-
111-
debug('Server sending full response...');
112-
res.end(testResBody);
113-
}));
114-
115-
server.listen(0, common.mustCall(() => {
116-
const req = http.request({
117-
port: server.address().port, path: '/'
118-
});
119-
debug('Client sending request...');
120-
121-
req.on('information', common.mustNotCall());
122-
123-
req.on('response', common.mustCall((res) => {
124-
let body = '';
125-
126-
assert.strictEqual(res.statusCode, 200, `Final status code was ${res.statusCode}, not 200.`);
127-
128-
res.on('data', (chunk) => {
129-
body += chunk;
130-
});
131-
132-
res.on('end', common.mustCall(() => {
133-
debug('Got full response.');
134-
assert.strictEqual(body, testResBody);
135-
server.close();
136-
}));
137-
}));
138-
139-
req.end();
140-
}));
141-
}
142-
143102
{
144103
// Happy flow - object argument with string
145104

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

259-
req.on('information', common.mustNotCall());
218+
req.on('information', common.mustCall((info) => {
219+
assert.strictEqual(info.statusCode, 103)
220+
}));
260221

261222
req.on('response', common.mustCall((res) => {
262223
let body = '';

‎test/parallel/test-http2-compat-write-early-hints-invalid-argument-value.js‎

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎test/parallel/test-http2-compat-write-early-hints.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,9 @@ const testResBody = 'response content';
128128

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

131-
req.on('headers', common.mustNotCall());
131+
req.on('headers', common.mustCall((headers) => {
132+
assert.strictEqual(headers[':status'], 103);
133+
}));
132134

133135
req.on('response', common.mustCall((headers) => {
134136
assert.strictEqual(headers[':status'], 200);

‎test/parallel/test-validators.js‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ const {
1212
validateString,
1313
validateInt32,
1414
validateUint32,
15-
validateLinkHeaderValue,
1615
} = require('internal/validators');
1716
const { MAX_SAFE_INTEGER, MIN_SAFE_INTEGER } = Number;
1817
const outOfRangeError = {
@@ -155,15 +154,3 @@ const invalidArgValueError = {
155154
code: 'ERR_INVALID_ARG_TYPE'
156155
}));
157156
}
158-
159-
{
160-
// validateLinkHeaderValue type validation.
161-
[
162-
['</styles.css>; rel=preload; as=style', '</styles.css>; rel=preload; as=style'],
163-
['</styles.css>; rel=preload; title=hello', '</styles.css>; rel=preload; title=hello'],
164-
['</styles.css>; rel=preload; crossorigin=hello', '</styles.css>; rel=preload; crossorigin=hello'],
165-
['</styles.css>; rel=preload; disabled=true', '</styles.css>; rel=preload; disabled=true'],
166-
['</styles.css>; rel=preload; fetchpriority=high', '</styles.css>; rel=preload; fetchpriority=high'],
167-
['</styles.css>; rel=preload; referrerpolicy=origin', '</styles.css>; rel=preload; referrerpolicy=origin'],
168-
].forEach(([value, expected]) => assert.strictEqual(validateLinkHeaderValue(value), expected));
169-
}

0 commit comments

Comments
 (0)