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

BREAKING: Android: Support withCredentials flag in XHRs #12276

Closed
wants to merge 2 commits into from

Conversation

rigdern
Copy link
Contributor

@rigdern rigdern commented Feb 8, 2017

Corresponding iOS PR: #12275

Respect the withCredentials XMLHttpRequest flag for sending cookies with requests. This can reduce payload sizes where large cookies are set for domains.

This should fix #5347.

This is a breaking change because it alters the default behavior of XHR. Prior to this change, XHR would send cookies by default. After this change, by default, XHR does not send cookies which is consistent with the default behavior of XHR on web for cross-site requests. Developers can restore the previous behavior by passing true for XHR's withCredentials argument.

Test plan (required)

Verified in a test app that XHR works properly when specifying withCredentials as true, false, and undefined. Also, my team uses this change in our app.

Adam Comella
Microsoft Corp.

Corresponding iOS PR: facebook#12275

Respect the withCredentials XMLHttpRequest flag for sending cookies with requests. This can reduce payload sizes where large cookies are set for domains.

This should fix facebook#5347.

This is a breaking change because it alters the default behavior of XHR. Prior to this change, XHR would send cookies by default. After this change, by default, XHR does not send cookies which is consistent with the default behavior of XHR on web for cross-site requests. Developers can restore the previous behavior by passing `true` for XHR's `withCredentials` argument.

**Test plan (required)**

Verified in a test app that XHR works properly when specifying `withCredentials` as `true`, `false`, and `undefined`. Also, my team uses this change in our app.

Adam Comella
Microsoft Corp.
@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Feb 8, 2017
@rigdern rigdern changed the title Breaking Change: Android: Support withCredentials flag in XHRs BREAKING: Android: Support withCredentials flag in XHRs Feb 9, 2017
@matthargett
Copy link
Contributor

Any chance you could submit a similar PR to react-native-windows?

@mkonicek
Copy link
Contributor

mkonicek commented Mar 8, 2017

CookieJar 😂

Looks great, thank you!

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Mar 8, 2017
@facebook-github-bot
Copy link
Contributor

@mkonicek has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Mar 8, 2017
Summary:
Corresponding Android PR: #12276

Respect the withCredentials XMLHttpRequest flag for sending cookies with requests. This can reduce payload sizes where large cookies are set for domains.

This should fix #5347.

This is a breaking change because it alters the default behavior of XHR. Prior to this change, XHR would send cookies by default. After this change, by default, XHR does not send cookies which is consistent with the default behavior of XHR on web for cross-site requests. Developers can restore the previous behavior by passing `true` for XHR's `withCredentials` argument.

**Test plan (required)**

Verified in a test app that XHR works properly when specifying `withCredentials` as `true`, `false`, and `undefined`. Also, my team uses this change in our app.

Adam Comella
Microsoft Corp.
Closes #12275

Differential Revision: D4673644

Pulled By: mkonicek

fbshipit-source-id: 2fd8f536d02fb39d872eb849584c5c4f7e7698c5
@facebook-github-bot
Copy link
Contributor

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Mar 9, 2017
thotegowda pushed a commit to thotegowda/react-native that referenced this pull request May 7, 2017
Summary:
Corresponding iOS PR: facebook#12275

Respect the withCredentials XMLHttpRequest flag for sending cookies with requests. This can reduce payload sizes where large cookies are set for domains.

This should fix facebook#5347.

This is a breaking change because it alters the default behavior of XHR. Prior to this change, XHR would send cookies by default. After this change, by default, XHR does not send cookies which is consistent with the default behavior of XHR on web for cross-site requests. Developers can restore the previous behavior by passing `true` for XHR's `withCredentials` argument.

**Test plan (required)**

Verified in a test app that XHR works properly when specifying `withCredentials` as `true`, `false`, and `undefined`. Also, my team uses this change in our app.

Adam Comella
Microsoft Corp.
Closes facebook#12276

Differential Revision: D4673646

Pulled By: ericvicenti

fbshipit-source-id: 924c230c9df72071b3cf9151c3ac201905ac28a5
@rigdern rigdern deleted the rigdern/androidXhrWithCreds branch May 18, 2017 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XMLHttpRequest polyfill doesn't provide the withCredentials property
5 participants