Fetch API corrupts repetitive response headers

[sshymko]

Description

The Fetch API misinterprets repetitive response headers, for instance, Set-Cookie header passed separately for each cookie. All values of the header concatenate into a single string unusable for further processing.

Reproduction

fetch('https://github.com/facebook/react-native')
    .then((response) => {
        console.log(response.headers);
        console.log(response.headers.getAll('Set-Cookie'));
    });

Expected result:

{ map: 
	   { date: [ 'Fri, 10 Mar 2017 06:53:22 GMT' ],
	     'content-type': [ 'text/html; charset=utf-8' ],
	     'set-cookie': [
                 '_gh_sess=eyJsYXN0...; path=/; secure; HttpOnly',
                 'user_session=AYIZ4afT...; path=/; expires=Fri, 24 Mar 2017 07:18:28 -0000; secure; HttpOnly'
             ]
           }
}

[
    '_gh_sess=eyJsYXN0...; path=/; secure; HttpOnly',
    'user_session=AYIZ4afT...; path=/; expires=Fri, 24 Mar 2017 07:18:28 -0000; secure; HttpOnly'
]

Actual result:

{ map: 
	   { date: [ 'Fri, 10 Mar 2017 06:53:22 GMT' ],
	     'content-type': [ 'text/html; charset=utf-8' ],
	     'set-cookie': [
                 '_gh_sess=eyJsYXN0...; path=/; secure; HttpOnly, user_session=AYIZ4afT...; path=/; expires=Fri, 24 Mar 2017 07:18:28 -0000; secure; HttpOnly'
             ]
           }
}

[
    '_gh_sess=eyJsYXN0...; path=/; secure; HttpOnly, user_session=AYIZ4afT...; path=/; expires=Fri, 24 Mar 2017 07:18:28 -0000; secure; HttpOnly'
]

Solution

The underlying platform-specific code that implements the Fetch API needs to be fixed.

Additional Information

• React Native version: 0.42.0
• Platform: iOS (haven't tested on Android)
• Operating System: MacOS

[sshymko]

React Native also appears to discard cookie path (and perhaps domain) when automatically sending Cookie headers based on the previously received Set-Cookie. It may be unable to recognize cookie properties because values of multiple cookies are "glued" in the underlying data structure.

[nfriedly]

TL; DR:

I think React Native is more or less doing things correctly here, but it would be nice if it followed the example of Node.js and special-cased Set-Cookie headers as an array field rather than a combined one and/or provided access to the raw headers.

---

Long version with citations:

In reading things this morning, and talking to folks behind the Fetch spec, React Native seems to be following the fetch spec correctly, with the exception of allowing access to Set-Cookie headers at all. (And the second exception of including the now-deprecated Headers.getAll() API; but the value-combining behavior is correct for when it was in the spec).

The header combining behavior is allowed (but not required) according to the HTTP RFC, and also mentioned in the cookie RFC:

An origin server may include multiple Set-Cookie headers in a response. Note that an intervening gateway could fold multiple such headers into a single header.

It can be rather difficult to combine a combined Set-Cookie header since, for example, a expires field can contain the same ", " separator that is used to combine multiple headers into one.

The Fetch spec takes things a step further and requires this combining behavior. Fetch folks are aware that it doesn't work well for cookies but require the behavior anyways since their spec doesn't allow access to Set-Cookie headers.

So, as I said above, special-casing cookies and/or providing raw header access would be very beneficial here.

Looking at the code, that may be easier said then done...

---

Also, regarding this:

React Native also appears to discard cookie path (and perhaps domain) when automatically sending Cookie headers based on the previously received Set-Cookie.

I believe this is the correct behavior. Set-Cookie is allowed to contain that stuff, but not Cookie - it just contains name-value pairs as appropriate for the requested path and domain.

[sshymko]

Just wanted to clarify regarding:

React Native also appears to discard cookie path (and perhaps domain) when automatically sending Cookie headers based on the previously received Set-Cookie.

I believe this is the correct behavior. Set-Cookie is allowed to contain that stuff, but not Cookie - it just contains name-value pairs as appropriate for the requested path and domain.

React Native's fetch acts as a browser in terms of maintaining cookies. When a response contains Set-Cookie header, React Native memorizes it and starts to include the respective Cookie header into all subsequent requests. That is pretty cool, isn't it? Well, the problem is that it does not respect the cookie scope (path for sure, haven't checked the domain). For example, if a request to http://login.example.com/ returns Set-Cookie: token=123; Path=/protected/, React Native will keep sending Cookie: token=123 to URL with any path, not only the /protected/ path the cookie was meant for. This a security concern.

[bitinn]

I am the owner of node-fetch (which react-native uses) and was searching for a relevant issue about getAll.

Basically there are 2 issues to be fixed for node-fetch here:

• for v1.6.3, which react-native currently uses, you are saying set-cookie headers isn't parsed properly. I would be glad to take a PR with tests to fix it.

• we are currently working on v2 of node-fetch, which follows the Fetch Spec more closely, but because Fetch Spec didn't account for set-cookie headers being exposed, and has deprecated getAll, we somehow need to handle this special case which is not possible in browsers. I would be glad to take a PR with tests as well.

As for the cookie path issue, I think its a react-native problem, true?

You can follow up on a similar issue on node-fetch/node-fetch#251

EDIT: also note for node-fetch we have a custom API raw() for your raw header access need.

[hramos]

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

• Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
• If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
• Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

[stevef]

This is still an issue and shouldn't be closed.

[nfriedly]

I know this ticket is pretty old, but I think I finally have a good solution. Thanks to a PR from @chrusart, the set-cookie-parser library is now capable of splitting the combined cookie string! It can then be parsed as individual set-cookie headers. See https://www.npmjs.com/package/set-cookie-parser#usage-in-react-native

Please test it out and let me know how it works for you.