Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS: respect "withCredentials: false" in XMLHttpRequest #24080

Closed
DimitryDushkin opened this issue Mar 21, 2019 · 3 comments
Closed

iOS: respect "withCredentials: false" in XMLHttpRequest #24080

DimitryDushkin opened this issue Mar 21, 2019 · 3 comments
Labels
Bug 🌐Networking Related to a networking API. Platform: iOS iOS applications. Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.

Comments

@DimitryDushkin
Copy link

DimitryDushkin commented Mar 21, 2019

🐛 Bug Report

Even with withCredentials: false network requests still sending cookies.

To Reproduce

  1. Request yandex.ru (it will set yandexuid cookie in response)
  2. Request anything else with withCredentials: false and you will see that cookies are sent

Expected Behavior

No cookies with withCredentials: false.

Code Example

Actually there is very easy fix to it

--- node_modules/react-native/Libraries/Network/RCTNetworking.mm	2019-03-18 17:28:57.000000000 +0300
+++ node_modules/react-native/Libraries/Network/RCTNetworking.mm	2019-03-18 17:29:36.000000000 +0300
@@ -245,10 +245,13 @@
   NSURL *URL = [RCTConvert NSURL:query[@"url"]]; // this is marked as nullable in JS, but should not be null
   NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:URL];
   request.HTTPMethod = [RCTConvert NSString:RCTNilIfNull(query[@"method"])].uppercaseString ?: @"GET";
+  request.HTTPShouldHandleCookies = [RCTConvert BOOL:query[@"withCredentials"]];

-  // Load and set the cookie header.
-  NSArray<NSHTTPCookie *> *cookies = [[NSHTTPCookieStorage sharedHTTPCookieStorage] cookiesForURL:URL];
-  request.allHTTPHeaderFields = [NSHTTPCookie requestHeaderFieldsWithCookies:cookies];
+  if (request.HTTPShouldHandleCookies == YES) {
+      // Load and set the cookie header.
+      NSArray<NSHTTPCookie *> *cookies = [[NSHTTPCookieStorage sharedHTTPCookieStorage] cookiesForURL:URL];
+      request.allHTTPHeaderFields = [NSHTTPCookie requestHeaderFieldsWithCookies:cookies];
+  }

   // Set supplied headers.
   NSDictionary *headers = [RCTConvert NSDictionary:query[@"headers"]];
@@ -259,7 +262,6 @@
   }];

   request.timeoutInterval = [RCTConvert NSTimeInterval:query[@"timeout"]];
-  request.HTTPShouldHandleCookies = [RCTConvert BOOL:query[@"withCredentials"]];
   NSDictionary<NSString *, id> *data = [RCTConvert NSDictionary:RCTNilIfNull(query[@"data"])];
   NSString *trackingName = data[@"trackingName"];
   if (trackingName) {

If everyone is okay I can make PR for it.

Environment

 React Native Environment Info:
    System:
      OS: macOS 10.14.3
      CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
      Memory: 29.28 MB / 16.00 GB
      Shell: 3.0.2 - /usr/local/bin/fish
    Binaries:
      Node: 11.5.0 - ~/.nvm/versions/node/v11.5.0/bin/node
      Yarn: 1.15.2 - /usr/local/bin/yarn
      npm: 6.9.0 - ~/.nvm/versions/node/v11.5.0/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.1, macOS 10.14, tvOS 12.1, watchOS 5.1
    IDEs:
      Android Studio: 3.3 AI-182.5107.16.33.5264788
      Xcode: 10.1/10B61 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.7.0 => 16.7.0
      react-native: 0.57.8 => 0.57.8
@Salakar Salakar added the 🌐Networking Related to a networking API. label Mar 21, 2019
@Salakar
Copy link
Contributor

Salakar commented Mar 21, 2019

@DimitryDushkin yes please; could you send a PR over and link to this issue + tag me. Are you also able to confirm if this is the same behaviour on Android?

Thanks

@react-native-bot
Copy link
Collaborator

It looks like you are using an older version of React Native. Please update to the latest release, v0.59 and verify if the issue still exists.

The "Resolution: Old Version" label will be removed automatically once you edit your original post with the results of running `react-native info` on a project using the latest release.

@react-native-bot react-native-bot added the Ran Commands One of our bots successfully processed a command. label Apr 9, 2019
@react-native-bot
Copy link
Collaborator

I am closing this issue because it does not appear to have been verified on the latest release, and there has been no followup in a while.

If you found this thread after encountering the same issue in the latest release, please feel free to create a new issue with up-to-date information by clicking here.

facebook-github-bot pushed a commit that referenced this issue Apr 29, 2019
Summary:
Fixes #24080.

Even with `withCredentials: false` network requests still sending cookies. Fix this behaviour according to https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/withCredentials.

[iOS] [Fixed] - Respect "withCredentials: false" in network requests
Pull Request resolved: #24629

Differential Revision: D15120420

Pulled By: cpojer

fbshipit-source-id: 78b9924436b02584c4fc1aa04763dff085eea78c
@facebook facebook locked as resolved and limited conversation to collaborators Apr 9, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Apr 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug 🌐Networking Related to a networking API. Platform: iOS iOS applications. Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants