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
nodejs · Feb 1, 2023 · 78dd37e · 78dd37e
1 parent dc90810
commit 78dd37e
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 186 deletions.
diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -51,7 +51,9 @@ const { ConnectionsList } = internalBinding('http_parser');
 const {
   kUniqueHeaders,
   parseUniqueHeadersOption,
-  OutgoingMessage
+  OutgoingMessage,
+  validateHeaderName,
+  validateHeaderValue,
 } = require('_http_outgoing');
 const {
   kOutHeaders,
@@ -84,7 +86,6 @@ const {
 const {
   validateInteger,
   validateBoolean,
-  validateLinkHeaderValue,
   validateObject
 } = require('internal/validators');
 const Buffer = require('buffer').Buffer;
@@ -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';
     }
   }

diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
@@ -55,7 +55,6 @@ const {
 const {
   validateFunction,
   validateString,
-  validateLinkHeaderValue,
   validateObject,
 } = require('internal/validators');
 const {
@@ -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;

diff --git a/lib/internal/validators.js b/lib/internal/validators.js
@@ -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,
@@ -545,6 +490,5 @@ module.exports = {
   validateUndefined,
   validateUnion,
   validateAbortSignal,
-  validateLinkHeaderValue,
   validateInternalField,
 };
diff --git a/test/parallel/test-http-early-hints.js b/test/parallel/test-http-early-hints.js
@@ -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
 
@@ -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 = '';

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
diff --git a/test/parallel/test-http2-compat-write-early-hints.js b/test/parallel/test-http2-compat-write-early-hints.js
@@ -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);

diff --git a/test/parallel/test-validators.js b/test/parallel/test-validators.js
@@ -12,7 +12,6 @@ const {
   validateString,
   validateInt32,
   validateUint32,
-  validateLinkHeaderValue,
 } = require('internal/validators');
 const { MAX_SAFE_INTEGER, MIN_SAFE_INTEGER } = Number;
 const outOfRangeError = {
@@ -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));
-}