Cookies set in a redirect response are not persisted

[michaeleisel]

When I go to a site that returns a 303 along with a Set-Cookie header, it does not use the new cookie from that redirect when it goes to the new URL. This would be somewhat manageable if I could use redirect: 'manual' in fetch's request options and inspect the redirect myself, but that is also broken.

• I have reviewed the documentation
• I have searched existing issues
• I am using the latest React Native version

Environment

Environment:
OS: macOS High Sierra 10.13.2
Node: 10.0.0
Yarn: Not Found
npm: 5.6.0
Watchman: 4.9.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.1 AI-173.4697961

Packages: (wanted => installed)
react: 16.3.1 => 16.3.1
react-native: ^0.55.2 => 0.55.2

Steps to Reproduce

Go to a site that redirects with a 303 response and sets a cookie via Set-Cookie.

Expected Behavior

Cookie that it sets is sent up when the redirect continues with whatever URL the 303 sent you to.

Actual Behavior

Cookie is not sent up

[michaeleisel]

When I try this using an NSURLSession in Objective-C, the cookie is set as expected, so react native behaves differently from native.

[michaeleisel]

From my testing, android doesn't have the same problem, so I guess it's an iOS thing

[ghost]

It just stopped sending cookies for me from 0.56. Was working fine till 0.55. I am facing the issue in a normal fetch request.

[michaeleisel]

@kelset why add a “no repro steps” label? There are repro steps in the ticket

[ethanyuwang]

@km16 having the same issue but only on android

[ghost]

Even I am facing this on android. It works perfectly till 0.55.4. I just feel #19770 might fix this, when it get's merged.

[Jacse]

Please see #16127

[gtebbutt]

@Jacse Would that explain a difference between 0.55 and 0.56? I thought that PR was referencing an issue from much further back than that.

I'm seeing what appears to be a related issue on iOS: since updating from 0.55 to 0.57, the react-native-video package is no longer passing cookies to the CDN.

Perhaps worth noting that the recent chat on #19958 also suggests an explicit behaviour change after 0.55, since several users there are apparently needing to explicitly add credentials: 'include' where it wasn't needed before.

[GetSource1234]

I am also seeing this issue in rn 0.57 (ios 12.0).credentials: 'include' helps, but this flag does not help if you reopen the app. Cookie will be deleted after app relaunch(could be only my case).

[BKova]

I am also seeing this issue in rn 0.57 (ios 12.0).credentials: 'include' helps, but this flag does not help if you reopen the app. Cookie will be deleted after app relaunch(could be only my case).

Can confirm this works on Android in RN 0.57

[corradio]

I'm having exactly the same issue. Only happens on iOS, so I dug into the the iOS code.
It turns out this commit caused the issue: 047961f#diff-0b161cb06747782c515275d84ec94b14R234
In a nutshell, the HTTPShouldHandleCookies boolean has no effect when custom cookies are set (see https://developer.apple.com/documentation/foundation/nsmutableurlrequest/1415485-httpshouldhandlecookies?preferredLanguage=occ). Setting custom cookies prevents the cookies of a redirect response from being re-used in the subsequent request.

I've fixed it by patching the RCTHTTPRequestHandler.mm to listen for redirects and pass cookies along. Here's the code block I added:

- (void)URLSession:(NSURLSession *)session
              task:(NSURLSessionTask *)task
willPerformHTTPRedirection:(NSHTTPURLResponse *)response
        newRequest:(NSURLRequest *)request
 completionHandler:(void (^)(NSURLRequest *))completionHandler
{
  // Add the cookies to the new request
  // This is necessary because we're not letting iOS handle cookies by itself
  NSMutableURLRequest *nextRequest = [request mutableCopy];
  
  NSArray<NSHTTPCookie *> *cookies = [[NSHTTPCookieStorage sharedHTTPCookieStorage] cookiesForURL:request.URL];
  nextRequest.allHTTPHeaderFields = [NSHTTPCookie requestHeaderFieldsWithCookies:cookies];
  completionHandler(nextRequest);
}

Is this a bug in RN that should be fixed? Happy to do a PR with this change if needed.

Ref #14869 and #15918 #16127 cc @jamesreggio @SDrinkwater @Jacse

[Jacse]

@corradio I would love to see that as a PR. If the goal is to have the two platforms behave similarly then this would clearly need to be fixed, since cookies persist across redirects on Android.

[corradio]

PR submitted: #22178

[roots-ai]

@corradio Adding this code, still doesn't resolve the issue for me.