Repository navigation
multiple headers of the "same name" allowed in http #3591
Description
Activity
It should be the last one. Headers are added in object insertion order, the last one wins out.
The behavior itself is most likely undocumented but I think we have tests for this (not easy to grep for though.)
Should it allow multiple by the same at all? Is it too costly to check for this?
FYI -> http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma. The order in which header fields with the same field-name are received is therefore significant to the interpretation of the combined field value, and thus a proxy MUST NOT change the order of these field values when a message is forwarded.
/cc @nodejs/http
Potentially there are a few solutions here:
- Only use the first key when there are duplicate keys once the keys have been converted to lower-case.
Basically, this solution would be to say that since HTTP headers are not case-sensitive, if
{ cookie: 'foo=bar;', Cookie: 'foo=meh;' }is given, act on it as if the user had really written{ cookie: 'foo=bar;', cookie: 'foo=meh;' }, which in JavaScript would have been converted to{ cookie: 'foo=meh;' }:$ node -pe "({ 'cookie': 'foo=bar;', 'cookie': 'foo=meh;' })" { cookie: 'foo=meh;' }- When given an object to set headers from (i.e.
res.writeHead(status, obj)), if theobjcontains duplicate keys once the keys have been converted to lower case, throw aTypeError.
This would signal to users that there is an issue in the given object, which was expected to only contains header-value pairs.
Option two sounds more appealing since if you can comma separated list syntax to define multiple headers otherwise.
Another option to consider is allowing an optional array syntax for that:
{ cookie: ['foo=bar', 'foo=meh'] }
The problem I saw in production was where one library was using
cookieand one external source usedCookieand the headers were merged and one was lost.Another option to consider is allowing an optional array syntax for that:
{ cookie: ['foo=bar', 'foo=meh'] }
I didn't mention it, because that is already supported; each element in the array will be a different header, making that response look like the following:
Cookie: foo=bar Cookie: foo=mehAssuming we are talking about
res.writeHead. If you are referring to a different Node.js API, let me know, as I don't see what API is being used actually mentioned here.Perfect :)
In that case I presume we can close an issue.
Why would you make that presumption?
FYI -> http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive.
Having a similar problem with X-Robots-Tag. I need to support the following
X-Robots-Tag: googlebot: nofollow X-Robots-Tag: otherbot: noindex, nofollowAs specified here.
@cressie176 According to the documentation:
Use an array of strings here if you need to send multiple headers with the same name.
So I think this would work for your case:
response.setHeader('X-Robots-Tag', ['googlebot: nofollow', 'otherbot: noindex, nofollow']);@Trott, thanks for the quick response. Unfortunately I've tried that - it generates a single header which I don't think conforms to googles spec.
x-robots-tag: 'googlebot: nofollow, otherbot: noindex, nofollow',Furthermore if I add an additional directive to apply to all user agents, it gets concatenated with the last one in the list, i.e.
response.setHeader('X-Robots-Tag', ['googlebot: nofollow', 'otherbot: noindex, nofollow', 'noimageindex']);Results in
x-robots-tag: 'googlebot: nofollow, otherbot: noindex, nofollow, noimageindex'47 remaining items
It seems like the solution here is to either allow multiple "same name" headers, or, fail loudly when the user tries to send them.
If anyone is interested in solving this, the two things that would likely be helpful are:
- detailing the exact desired solution in a comment here (if different from my summary, or if adding important missing details)
- sending a PR implementing one or all of those solutions.
It seems like the solution here is to either allow multiple "same name" headers, or, fail loudly when the user tries to send them.
Failing loudly would be a breaking change, and also isn't really a solution since sending multiple headers with the same name is supported by the HTTP specification and required by certain clients. Failing loudly in response to a valid action could also result in someone posting another issue, and we'd be back to square one. Therefore I suggest the only actual solution is to allow multiple headers with the same name.
Reacted by Yahor SiarheyenkaTherefore I suggest the only actual solution is to allow multiple headers with the same name.
I don't agree with continuing doing something bad simply because fixing it would break things.
Can we start with a warning "depreciated" message ?
Deprecated: Multiple header names are being passed <stack trace>. This will be deprecated as of version XReacted by Yahor SiarheyenkaMaybe I've misunderstood something, or not explained myself clearly enough.
As I understand things, what node does now is bad, because instead of honouring multiple headers with the same name (which is allowed in the HTTP specification and necessary in some circumstance) it squashes them.
Changing this to instead of honouring multiple headers with the same name, node reports an error, is still bad and arguably even worse.
Reacted by Yahor SiarheyenkaYou mean it's allowed because it's not expressly prohibited? Under what circumstances would it be necessary? (Aside from a mistake)
You mean it's allowed because it's not expressly prohibited?
@danielb2 The title of this issue is somewhat confusing. It talks about multiple headers of the "same name", but does not differentiate between requests and responses. From my understanding there are different problems with Node's implementation of both, however I am only talking about responses. Section 4.2 of the HTTP specification explicitly allows multiple headers with the same name to be sent, providing their values can be combined into a comma separated list.
Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma. The order in which header fields with the same field-name are received is therefore significant to the interpretation of the combined field value, and thus a proxy MUST NOT change the order of these field values when a message is forwarded.
Node does not currently support adding multiple headers to a response. The closest you can get is to call
response.setHeaderwith an array, which will still result a single header. Therefore Node is not fully compatible with the HTTP specification.Under what circumstances would it be necessary? (Aside from a mistake)
Google's X-Robots-Tag specification depends on being able to send multiple
X-Robots-Tagheaders on the response (in a violation of the HTTP specification since they cannot be combined). For example...X-Robots-Tag: googlebot: noindex X-Robots-Tag: bingbot: noindex, nofollow X-Robots-Tag: noimageindexThe above headers:
- Instruct googlebot not to index the page, but allow it to follow links to other pages
- Instruct bingbot not to index the page or follow links to other pages
- Instruct all bots not to index images
Combining this into a single header would result in:
X-Robots-Tag: googlebot: noindex, bingbot: noindex, nofollow, noimageindexThe X-Robots-Tag specification is silent of whether this syntax would even work, but at best the
noimageindexdirective would only apply to bingbot rather than all robots, thus changing the semantic meaning of the X-Robots-Tag header.Others have cited the following examples where the need to be able to send multiple headers with the same name as part of the HTTP response.
- multiple headers of the "same name" allowed in http #3591 (comment)
- multiple headers of the "same name" allowed in http #3591 (comment)
It is not unreasonable to suspect there may be more examples which have not yet been discovered or reported.
In current situation 💡, it would make sense to add Headers into Request and Response and gently deprecate ⛔ old methods, or even leave them as is for backward compatibility with few notes 📝
Unfortunately this still wouldn't work where multiple response headers with the same name are necessary since according to the append will combine values, rather than writing multiple response headers, which is necessary in some situations.
The difference between set() and append() is that if the specified header already exists and accepts multiple values, set() will overwrite the existing value with the new one, whereas append() will append the new value onto the end of the set of values.
@cressie176 what about semicolon or newline delimiter '\n' as an option to set multiple values for the same header? 🤔
@cressie176 what about semicolon or newline delimiter '\n' as an option to set multiple values for the same header? 🤔
Hi @bricss, I don't see anything in the HTTP specification about semicolons or new line delimiters having special meaning. I assume that they would be treated as part of the field value, rather than indicating multiple headers.
It seems like the solution here is to either allow multiple "same name" headers, or, fail loudly when the user tries to send them.
If anyone is interested in solving this, the two things that would likely be helpful are:
1. detailing the exact desired solution in a comment here (if different from my summary, or if adding important missing details) 2. sending a PR implementing one or all of those solutions.@ljharb what do you think about this PR: #6865 😅 I know it's old
Construct header using
req.rawHeaders// Server code http.createServer(function (req, res) { const rqHeaders = req.rawHeaders.reduce((map, header, i, rawHeaders) => { if (i % 2 === 0) return map; const headerName = rawHeaders[i-1]; const headerValue = rawHeaders[i]; const prevHeaderValue = map[headerName]; if (prevHeaderValue) { if (Array.isArray(prevHeaderValue)) prevHeaderValue.push(headerValue); else map[headerName] = [prevHeaderValue, headerValue]; } else map[headerName] = headerValue; return map; }, {} as any); console.log(rqHeaders); // This prints - { 'X-Robots-Tag': ['googlebot: noindex', 'bingbot: noindex, nofollow', 'noimageindex'] } }) .listen(0); // Client code const rq = http.request({ // options }, res => { rq.setHeader('X-Robots-Tag', ['googlebot: noindex', 'bingbot: noindex, nofollow', 'noimageindex']); rq.write('Hello World!'); rq.end(); });Reacted by Yahor SiarheyenkaRelative PR has just landed in master. Closing this.
If
{ 'cookie: 'foo=bar;', 'Cookie: 'foo=meh;' }is passed, you have no idea which one will be used.If a request relies on, for example, cookie information, a request may work seemingly at random and can be rather difficult to debug.