withCredentials flag gets ignored on iOS

[JonnyBurger]

Is this a bug report?

Yes

Have you read the Bugs section of the Contributing to React Native Guide?

Yes

Environment

1. react-native -v: 0.46

Then, specify:

1. Target Platform: iOS
2. Development Operating System: macOS
3. Build tools: Xcode

Description

In [email protected], a flag withCredentials was introduced. In [email protected], the behaviour was unintentionally changed by this commit: 047961f

withCredentials = true translates to HTTPShouldHandleCookies = YES in native code. After the commit that I linked above, the Cookie header gets set explicitly, which overrides any behaviour set by HTTPShouldHandleCookies.

Steps to Reproduce

Make a fetch request with the flag withCredentials: true.

Expected Behavior

The flag gets respected.
The cookie header only gets set explicitly if withCredentials = false

Actual Behavior

The flag gets overridden.

Reproducible Demo

Hard to make a demo, but the bug can be proven with the Apple documentation:

If your app sets the Cookie header on an NSMutableURLRequest object, then this method has no effect, and the cookie data you set in the header overrides all cookies from the cookie store.

Here you can see that HTTPShouldHandleCookies is being set based on the withCredentials flag.

Here you can see that the Cookie header is being set explicitly, which takes precedence over HTTPShouldHandleCookies.

Related issues

• withCredentials flag in XHRs should default to "true" withCredentials flag in XHRs should default to "true" #14063

cc

@clozr

[jozan]

We've hit this as well. Nice catch! :)

[JonnyBurger]

Made a PR: #14931

Reviews appreciated as Objective-C and especially Networking in it is not my strength.

[cbjs]

0.46.2, still not working

[clozr]

Sorry for late reply. back from vacation. It seems that withCredential flag and the fix to send the cookie by default conflicts each other. Let me analyze a bit more and will update.

[jamesreggio]

Any update on this issue? We're also affected.

[jamesreggio]

I raised some concerns in #14931 (comment), which contains the proposed fix.

I'm not sure if I'm clear on precisely what behavior you're expecting. Specifically, when you use withCredentials: true are you finding that the value of the Cookie header in the resulting request is missing some cookies? Or is it that Set-Cookie response headers aren't being respected.

I expect that the problem may be that cookies are missing, because the changes in 047961f switched to a different cookie jar. Understandably, this would be bad for existing users, who could potentially be logged out through the loss of the cookies that were set before that change. However, I'd like clarification from those of you who are affected, since I'm afraid #14931 may have the wrong fix.

[aarondail]

I believe I am running into this issue as well, in RN 0.46 and 0.47.

Basically, my specific problem is due to the change mentioned at the top of this thread (047961f), the Cookie header is always added to fetch requests even if withCredentials is false.

In my use case, I am using a WebView which can update the cookies unintentionally, so when I make fetch requests I explicitly set withCredentials to false and pass my own Cookie header.

The fix you have in #14931 looks like it gets close to fixing my issue but not all the way probably. With your change it looks like if I set withCredentials to false it would leave setHTTPShouldSetCookies as NO, which is good I assume (I am not that knowledgable about setHTTPShouldSetCookies TBH), BUT it looks like it would still add the Cookie header (the else case) even though I don't want it to.

I just need a way to "opt-out" of cookies being automatically added. I feel like setting withCredentials to false is reasonable... at least thats what I would expect. I could be wrong.

Also I believe this is the way Android already behaves.

[panunu]

Any news or progress on this? :-)

[jamesreggio]

Sorry, I got lazy.

I just opened a PR to revert the original breaking change in #16127.

If you agree, do you mind chiming in? @panunu @aarondail @jozan @cbjs @JonnyBurger

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.