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

No default withCredentials. Updated version of #47 #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gsf
Copy link

@gsf gsf commented Jun 23, 2014

Any reason to alter the default for xhr.withCredentials?

@gobengo
Copy link

gobengo commented Jul 8, 2014

I think it is a bad idea to change the default on such an important flag without a major version bump, or at least minor.

Though I would say if this was a fresh project that withCredentials should be false by default.

@gsf
Copy link
Author

gsf commented Jul 8, 2014

As discussed at naugtur/xhr#33 (click "Show outdated diff"), some believe the withCredentials default in the spec was a mistake, as was the Access-Control-Allow-Origin wildcard. I haven't found many resources to back this up, however. The commented text at http://enable-cors.org/server_nginx.html suggests this, but others (including http://fetch.spec.whatwg.org/#basic-safe-cors-protocol-setup) seem to favor the wildcard and the default of false.

@bmpvieira
Copy link

I would like to see this merged since withCredentials default to true prevents accessing resources on many servers and the solution might not be obvious for users. Especially if your like me using http-browserify through request browserified.

Resources examples:
http://data-gov.tw.rpi.edu/raw/1576/data-1576.nt.gz
http://ftp.ebi.ac.uk/pub/databases/ensembl/encode/integration_data_jan2011/hub.txt

@eush77
Copy link

eush77 commented Nov 5, 2014

Please merge. The current default behavior is totally unexpected.

@zwhitchcox
Copy link

Holy shit, please merge. That took me forever to figure out.

@recursify
Copy link

Any update on if/when this will get merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants