Repository navigation
Duplicate Authorization headers should not be ignored. #45699
Description
Activity
Can you share a minimal repro, the expected result, and the actual result?
Proof-of-concept
import Network from "node:net"; import HTTP from "node:http"; const server = HTTP.createServer(request => console.log(request.headers)) .listen(_ => { const client = Network.createConnection( server.address().port, _ => client.write( "GET / HTTP/1.1\r\n" + `Authorization: Digest username="ha"\r\n` + `Authorization: realm="no"\r\n\r\n` ) ); });
The actual output
{ authorization: 'Digest username="ha"' }The expected output
{ authorization: 'Digest username="ha", realm="no"' }
Reacted by Antoine du Hamel and emigreBut now that I think of it, splitting the
Authorizationheader field value like this (Authorization: realm="no") could be violation of the RFC,Authorization = credentials.@nodejs/http
RFC 9110 — HTTP semantics
5.2. Field Lines and Combined Field Value
Field sections are composed of any number of “field lines,” each with a “field name” (see Section 5.1) identifying the field, and a “field line value” that conveys data for that instance of the field.
…When a field name is repeated within a section, its combined field value consists of the list of corresponding field line values within that section, concatenated in order, with each field line value separated by a comma.
For example, this section:
Example-Field: Foo, Bar Example-Field: Bazcontains two field lines, both with the field name “
Example-Field.” The first field line has a field line value of “Foo, Bar,” while the second field line value is “Baz.” The field value for “Example-Field” is the list “Foo, Bar, Baz.”11.6.2.
Authorization…Its value consists of credentials containing the authentication information of the user agent for the realm of the resource being requested.
Authorization = credentialsI am not sure whether the “value” in the section §11.6.2 means a “(combined) field value” or “field line value.”
If it means the former,
Authorization: Digest username="ha" Authorization: realm="no"would be valid (meaning, Node.js should respect multiple occurrences of
Authorizationheaders),and if it means the latter, invalid (meaning, Node.js is fine as is).
According to section https://www.rfc-editor.org/rfc/rfc9110#name-changes-from-rfc-7230, if there is no clear indication of "field line value" then it refers to combined value.
Now, AFAIK Node implements RFC 7230 and not RFC 9110, so I wonder whether we should implement this or not.
@nodejs/http @nodejs/tsc WDYT?
Reacted by issuefilerOne problem I see is there's a non-zero risk of breaking existing programs if we make this change.
If we want to support this it would be behind an option.
Reacted by Marco IppolitoWe can leave the default behaviour as it is and implement a flag in
http.createServer()likeallowDuplicateHeadersand combine them with,. Maybe in a future release this can become the standard behaviour.
WDYT?That sounds good to me. The future documentation on that option would refer to RFC 9110.
As I said, I would recommend adding an option and possibly documenting it as experimental. I don't have much time to look into RFC 9110 and how it changed things.
Note that a lot of old clients and servers are very lenient on HTTP semantics, so they can expect whatever.
I will open a PR
- added a commit that references this issue
on Jan 3, 2023 - added 3 commits that reference this issue
on Jan 17, 2023 - added 2 commits that reference this issue
on Jan 26, 2023
The context
A sender can send multiple
Authorizationheaders in a request, because theAuthorizationheader’s definition,credentials, allows multipleauth-params to be recombined as a comma-separated list.RFC 9110 — HTTP semantics
Node.js MAY join them together with
,.RFC 9110 — HTTP semantics
Currently, Node.js ignores duplicate
Authorizationheaders when it createsmessage.headers.Node.js 19.2.0 documentation — HTTP
Suggestion
I suggest Node.js join the field line values of multiple
Authorizationheaders in a request with,, instead of ignoring them, when it createsmessage.headers.The related issue
#3591