Repository navigation
store header char check too strict? #47479
Description
Not sure what is correct here but a test in undici started to fail in node 19 with:
Invalid character in header content ["content-disposition"]
With:
res.writeHead(200, {
'content-length': 2,
'content-disposition': "attachment; filename='år.pdf'"
})I don't think å should cause problems?
Activity
ronag commentedon Apr 8, 2023
@nodejs/http
ronag commentedon Apr 8, 2023
I just noticed nodejs/undici#2020 but it seems like no one has a solution. Help would be appreciated.
mcollina commentedon Apr 8, 2023
I think this is a change we might want to revert.
cc @nodejs/tsc
targos commentedon Apr 8, 2023
Which change?
marco-ippolito commentedon Apr 8, 2023
I think we should revert this #46528 and rethink a better solution, the problem that this pr tried to solve is smaller than the one it created
bnoordhuis commentedon Apr 15, 2023
https://httpwg.org/specs/rfc9110.html#fields.values still says:
Thus technically node is in the clear. The spec also says this:
But the
åin the example is UTF-8, not ISO-8859-1. The spec then goes on to say:So I suppose we could transmit the character as C3 A5 (UTF-8) and well-behaved recipients will accept it but then it's ambiguous how the recipient decodes it. Probably as ISO-8859-1.
We could also transcode from UTF-8 to ISO-8859-1 but that's lossy and it still doesn't really resolve the ambiguity.
RFC8187 defines a configurable character encoding scheme for HTTP header values but literally nothing supports it AFAIK so yeah, good idea but not very practical.
ShogunPanda commentedon Apr 21, 2023
I tend to agree with Ben: if the spec allows that, we allow it as well.
The biggest issue I see is we don't technically support RFC 9110 but, for instance, llhttp looks at RFC 7230. So if we start this trend we might end up having conflicts. What shall we do?
bnoordhuis commentedon Apr 23, 2023
Newer RFCs generally obsolete older ones so, unless there's very good reason not to, node should follow the newer one.
ShogunPanda commentedon Apr 23, 2023
I absolutely agree. I'm also willing to follow the newer RFC.
I'm just wondering if we're gonna introduce conflicts if we slowly migrate to 9110.
mcollina commentedon May 8, 2023
Should we close this then?
marco-ippolito commentedon May 20, 2023
closing this as we already implement the latest rfc.