1608979 - WebRequest Set-Cookie headers have invalid format: multiple Set-Cookie headers from server joined by newline (\n)

Status: NEW

(WebExtensions :: Request Handling, defect, P3)

Description

[adam.schenker]

Attachment: Firefox WebRequest Test.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.4 Safari/605.1.15

Steps to reproduce:

The WebRequest API returns multiple Set-Cookie headers in a format forbidden by RFC 6265 and inconsistent with implementations in other browsers (Chrome).

I used the attached test extension in both Firefox and Chrome, cleared my cookies and went to web sites that set multiple cookies for a domain.

Actual results:

Firefox returns a single JSON object for "name" Set-Cookie for a domain and combines all Set-Cookie values into a single "value" string separated by \n sequences.

An example with actual values replaced (for security) is shown below:

{"name":"Set-Cookie","value":"A=B; expires=Wed, 12-Jan-2022 21:34:20 GMT; path=/; domain=xyz.com\nC=D; domain=.xyz.com; expires=Thu, 13-Jan-2022 12:00:00 GMT; path=/\nE=F; domain=.xyz.com; expires=Thu, 13-Jan-2022 12:00:00 GMT; path=/"}

Expected results:

Each Set-Cookie request should be a separate JSON object. RFC 6265 states:

"Origin servers SHOULD NOT fold multiple Set-Cookie header fields into a single header field."

This is how the data is presented in Chrome:

{"name":"Set-Cookie","value":"A=B; expires=Wed, 12-Jan-2022 21:36:07 GMT; path=/; domain=xyz.com"}
{"name":"Set-Cookie","value":"C=D; domain=.xyz.com; expires=Thu, 13-Jan-2022 12:00:00 GMT; path=/"}
{"name":"Set-Cookie","value":"E=F; domain=.xyz.com; expires=Thu, 13-Jan-2022 12:00:00 GMT; path=/"}

Comment 1

[Shane Caraveo (:mixedpuppy)]

Those seem to be folded into a single header for consumption, but it really has nothing to do with RFC 6265. RFC 6265 defines how a server is to send those headers to the client. What the client does with those headers (in terms of something like the webextension api) afterwards is more an internal implementation detail.

In other words, Firefox is a client, not an "Origin server". How Firefox represents those headers to an extension API is not defined by RFC 6265, the rfc even states "It is beyond the scope of this document to recommend specific API designs".

Having said that, it terms of compatibility it would probably be better to not fold these together in the api. I'm not certain at the moment whether this happens in the lower httpChannel code or in the webRequest api implementation.

Comment 2

[adam.schenker]

I think the expectation is that the API will return the same headers sent by the server, which would imply a RFC 6265 compatible format. In this case a header that was never actually sent by the server is returned by the API. I didn't find any mention in the WebRequest API documentation that this special behavior is expected.

Comment 3

[muhammadsadi0909]

According to my understanding, the API should return the same headers sent by the server, which implies a format that is compatible with RFC 6265. In this case, the API returns a header that the server never sent. In the WebRequest API documentation here example(https://thecourtdirect.com/newark-municipal-court/), I didn't find any mention of this special behavior. I also face this problem but it automatically solve.

Comment 5

[Rob Wu [:robwu]]

URL that outputs response with two Set-Cookie headers: https://httpbin.org/response-headers?Set-Cookie=a&Set-Cookie=b
See also bug 1808069 for another test case.

Comment 6

[Jan Köhler]

@ :mixedpuppy & :robwu
What are the odds that the behavior will be adjusted? 😃 Meaning: Is there any chance, that the \n will be replaced?

Comment 7

[Rob Wu [:robwu]]

The headers are internally stored with a newline as separator. Here's the relevant source code: https://searchfox.org/mozilla-central/rev/01a0d864a9442d0fe2dbd4beee5c88b9b46e96bd/netwerk/protocol/http/nsHttpHeaderArray.h#247-265

That result is read internally by the webRequest implementation at https://searchfox.org/mozilla-central/rev/01a0d864a9442d0fe2dbd4beee5c88b9b46e96bd/toolkit/components/extensions/webrequest/WebRequest.sys.mjs#304 , derived from the internals at ChannelWrapper::GetResponseHeaders.

If you'd like to normalize the internal behavior to be comparable like Chrome (i.e. each Set-Cookie header being its own entry in the array), then the parser should change its logic, to split Set-Cookie header values at \n (and output each at its own line). And a unit test should be added that verifies the correct behavior when the server responds with multiple Set-Cookie headers, e.g. as seen in https://searchfox.org/mozilla-central/rev/11d085b63cf74b35737d9c036be80434883dd3f6/toolkit/components/extensions/test/xpcshell/test_ext_dnr_modifyHeaders.js#71-73

The odds of this changing anytime soon are low, unless a patch is contributed externally. If you're interested in contributing a patch, see https://wiki.mozilla.org/WebExtensions/Contribution_Onramp

Comment 8

[Jan Köhler]

Hello Rob,

thank you for this very detailed explanation!

Maybe a silly question: Which is the to-be-changed parser, you're referring to?
Do you mean the ResponseHeaderChanger located here?

In principle, I would be willing to contribute an adjustment here. I have read your contribution onramp. Even if it's all very extensively described... phew, I'm not sure if I can get into it that easily...