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

Loader in certain situations causes issues in Chrome #39

Closed
kitsonk opened this issue Jan 13, 2016 · 12 comments
Closed

Loader in certain situations causes issues in Chrome #39

kitsonk opened this issue Jan 13, 2016 · 12 comments
Assignees
Milestone

Comments

@kitsonk
Copy link
Member

kitsonk commented Jan 13, 2016

From @rodneyrehm on dojo/dojo2#15:

The line (<any> node).crossOrigin = 'anonymous'; makes Google Chrome request the resource without any cookies, thus killing my session.

I observed this when opening intern/client.html in a browser, while served from a server that requires authentication (session cookie). All resources are loaded from the same origin, so the forced CORS request imho makes no sense. Shouldn't this only be activated in case we're actually loading something from a different origin?

@kitsonk kitsonk added the bug label Jan 13, 2016
@kitsonk kitsonk added this to the beta.1 milestone Feb 24, 2016
@kitsonk kitsonk modified the milestones: beta.3, beta.2 Mar 11, 2016
@vansimke
Copy link
Contributor

I've played around with this and I think this may be related to a performance issue in CI. If I set the timeout for the .get() test to 60 seconds and the others to 30 seconds, then all the tests pass in Chrome and Android. Not sure if that is an acceptable solution though since this means that these tests are going to be prone to fail at seemingly random intervals.

@kitsonk
Copy link
Member Author

kitsonk commented Mar 11, 2016

I suspect this is unrelated with #95 then... what you are saying might be true for #95, but I don't think you recreated the scenario. I might have over correlated the issues. We should try to see, as reported above and see the cookie behaviour between the two.

@vansimke
Copy link
Contributor

Sorry, left comment on wrong issue...

@tomdye
Copy link
Member

tomdye commented Mar 15, 2016

Looks like it was added to enable better error stack traces for cross domain scripts:
https://errorception.com/docs/cors

The original commit:
dojo/intern-only-dojo@5485dfc

@tomdye
Copy link
Member

tomdye commented Mar 15, 2016

requirejs/text performs some checks to determine if the resource is on another domain, we could implement similar? https://github.com/requirejs/text/blob/master/text.js#L128

@rodneyrehm
Copy link

Chrome bug 412163 for context.

@tomdye
Copy link
Member

tomdye commented Mar 15, 2016

If you look at the documentation here:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-crossorigin

It suggests that the crossOrigin: anonymous is designed for exactly the case where no cookie etc.. is sent. However other documentation suggesting it's use for enabling cross origin stack traces say to use anonymous. Might be worth changing to use-credentials or implementing a check to see if the resource is on a different domain before adding anything at all.

The documentation further says that the use of crossOrigin dictates that the request must be done using CORS or not.

@tomdye
Copy link
Member

tomdye commented Mar 15, 2016

@kitsonk has suggested adding a cross-origin-stack-trace has flag to the loader, I anticipate that this would take an argument of anonymous or use-credentials and default to false which would not add the crossOrigin attr to the script tag in the first place.

@jason0x43 any thoughts on if this will impact intern?

@jason0x43
Copy link
Member

I'm not really familiar enough with the issue to say. As long as the behavior is optional, Intern could set the parameter to 'anonymous' by default in client.html to maintain current behavior, and a query param or some other mechanism could be used to changed it if necessary.

@kitsonk
Copy link
Member Author

kitsonk commented Oct 4, 2016

I still think we don't have a loader that works in a secured cross browser environment. We need to dig into this more.

@dylans dylans added this to the 2017.01 milestone Jan 12, 2017
@rorticus rorticus self-assigned this Jan 12, 2017
@rorticus
Copy link
Contributor

PR for controlling the crossorigin property on the script tags,

#103

@rorticus
Copy link
Contributor

Talked with @tomdye and we agreed we can close this once the PR lands.

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

No branches or pull requests

7 participants