fails to parse headers containing line feed

[pseidemann]

hi,
I have some quite old user agent Mozilla/5.0 (QtEmbedded; Linux) AppleWebKit/534.34 (KHTML, like Gecko) webkit/0.1 Safari/534.34 (some proprietary set-top box) which seem to include line feed between cookies when the server sends multiple Set-Cookie headers. I know that the new spec is that the user agent should join these headers with a comma , but it seems like before this was \n.
line feeds are deprecated but still allowed per https://tools.ietf.org/html/rfc7230.
if I change the regex for splitting the headers returned by xhr.getAllResponseHeaders() from /\r?\n/ to /\r\n/ the problem is solved for this user agent https://github.com/github/fetch/blob/eebaa2a1bc21eeba98ee00c9f94a0a4c2007cff1/fetch.js#L361.
from the specs it seems headers should always be delimited by \r\n so I wonder if this is the proper fix for my user agent.
without the fix, my requests to the server fail while using credentials: 'same-origin' because cookies are not send like they were received from the server.

[mislav]

We did avoid splitting by \n before #449 /cc @woudini. Now you're suggesting to revert that change. From my perspective, I can see both sides of the argument, but I'm not sure which approach is the more "correct" one.

However, I'm really surprised that you can read cookies from the server response (regardless of the value having line breaks or not). The browser usually doesn't let you read the Set-Cookie response header or set the Cookie request header because it's listed among "forbidden" headers for XHR. In other words, you are not supposed to interface with cookies directly, since that's the responsibility of the browser.

Could it be that your user agent is so old that it doesn't implement the concept of "forbidden" headers?

[pseidemann]

@mislav yes I think the browser is too old to know about forbidden headers.
what came to my mind is maybe a way which would allow both ways. what if we check first if the headers string contains any '\r' at all, and if so we split at '\r\n' (response follows spec strictly), otherwise we split on /\r?\n/ as before (tolerant version as per https://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html#sec19.3).
would this be an option?

[mislav]

I'm thinking such handling might be too “magic”. I must admit I don't understand the Tolerance provision fully. The spec says that \n is a delimiter between multiple values of the same header, e.g. Set-Cookie can have multiple values on multiple lines. It also says that \r\n is an official delimiter between headers. But then the Tolerance provision says that \n should be taken as separator between headers as well. That basically negates the original use of \n.

[pseidemann]

@mislav, from the issue you mentioned #449, I cannot find any evidence that ios would give headers without \r. maybe this was some bad http server and ios was just passing the response through.

@woudini could you give some details what the issue was? I'm a little surprised that the content-type would change when changing the splitting from '\r\n' to /\r?\n/. maybe there was a different issue, unrelated to this?

[kwgithubusername]

@mpfluger and I came up with this fix: #491

The confusion seems to be coming from the assumption that \r\n (or \n) is used to separate headers as well as multiple values of the same header.

https://tools.ietf.org/html/rfc7230#section-3 states that all header-fields are delimited by CRLF. To support the tolerance provision, CRLF here must be interchangeable with LF. Therefore the delimiter between headers is indeed /\r?\n/.

https://tools.ietf.org/html/rfc7230#section-3.2.4 states:

"Historically, HTTP header field values could be extended over multiple lines by preceding each extra line with at least one space or horizontal tab (obs-fold)...A user agent that receives an obs-fold in a response message that is not within a message/http container MUST replace each received obs-fold with one or more SP octets prior to interpreting the field value."

obs-fold is defined in Section 3.2 as CRLF 1*( SP / HTAB ). To support the tolerance provision, CRLF here must be interchangeable with LF. Therefore the delimiter between multiple values of the same header is not /\r?\n/ but /\r?\n/ plus at least one space or horizontal tab. This is likely what @pseidemann is seeing with the old user agent. The regex in this PR searches for these instances and replaces them with a space to conform to the above statement. Let me know if that works.

[mislav]

@woudini Thanks for the research! I think it's clear now that support for obs-fold is what we were missing. I've merged your PR and with that, I think that this issue is fixed since we now handle header values spread across multiple lines— as long as every consecutive value is indented, that is.