Repository navigation
Overzealous link header validation in writeEarlyHints #46453
Description
Version
v19.5.0
Platform
Darwin Kernel Version 22.2.0: Fri Nov 11 02:03:51 PST 2022; root:xnu-8792.61.2~4/RELEASE_ARM64_T6000 arm64
Subsystem
http
What steps will reproduce the bug?
import { createServer } from "node:http";
const server = createServer((req, res) => {
res.writeEarlyHints({
link: "<https://fonts.gstatic.com/>; rel=preconnect; crossorigin",
});
res.end();
});
server.listen(3000);How often does it reproduce? Is there a required condition?
No response
What is the expected behavior?
No response
What do you see instead?
node:internal/validators:473
throw new ERR_INVALID_ARG_VALUE(
^
TypeError [ERR_INVALID_ARG_VALUE]: The argument 'hints' must be an array or string of format "</styles.css>; rel=preload; as=style". Received '<https://fonts.gstatic.com/>; rel=preconnect; crossorigin'
at new NodeError (node:internal/errors:399:5)
at validateLinkHeaderFormat (node:internal/validators:473:11)
at validateLinkHeaderValue (node:internal/validators:493:5)
at ServerResponse.writeEarlyHints (node:_http_server:310:16)
at Server.<anonymous> (file:///Users/hargo/Code/wyyerd/i7n/test.mjs:4:7)
at Server.emit (node:events:512:28)
at parserOnIncoming (node:_http_server:1067:12)
at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17) {
code: 'ERR_INVALID_ARG_VALUE'
}
Node.js v19.5.0Additional information
This appears to be an overzelous validation that requires all link parameters to be followed by an =.
Per the ABNF in RFC8288 https://www.rfc-editor.org/rfc/rfc8288.html#section-3 the = is optional along with the parameter value.
Additionally it seems that the validation restricts parameters to a preset list of those headers currently defined by the HTML spec. This seems in conflict with RFC8288 which doesn't appear to place any restrictions on parameters, which seems more relevant that the parameters that happen to be specified in the current HTML specification.
Activity
bnoordhuis commentedon Feb 1, 2023
PR welcome, although the regex used to validate the header is... wrgh.
writeEarlyHints() was added in #44180 and then its validation was updated #44874. Personally I don't see the value in being so rigid. @nodejs/http what was the thinking there?
mcollina commentedon Feb 1, 2023
This is just a bug and no one caught it.
As for the script validation, the contributor proposed it and it seemed ok to me.
Can you take a look @anonrig @Uzlopak? You both discussed this problem in: https://github.com/nodejs/node/pull/44820/files#r983969846.
climba03003 commentedon Feb 2, 2023
I really doesn't know why
Linkheader is the only exception to own a validation.Maybe it provide a better DX, but in-consistence on behavior really cause a lot of confusion to the user.
http: fix validation of "Link" header
http: fix validation of "Link" header
http: fix validation of "Link" header
nithin-murali-arch commentedon Jun 5, 2023
Can this also be fixed on Node 18?
ShogunPanda commentedon Jun 5, 2023
@nithin-murali-arch Isn't this fixed in 18.16.0? #46466 has been backported there.