-
-
Notifications
You must be signed in to change notification settings - Fork 35.2k
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
CORS support in THREE.Loader.load_image ? #779
Comments
Well spotted. Fixed! I wonder whether we should use |
These two should be equivalent:
|
But why is it not possible to use "use-credentials" in all this load image requests ? it is needed when you want request the files only with a valid session. Thanks |
We simply didn't encounter use case with credentials yet. So far I didn't even see some example on how to actually deal with credentials (both on client and server sides). Everybody just shows how to do anonymous requests for some big image repositories, giving permissions for everybody (as opposed to setting up your server on one domain to give permissions to just your application that is hosted on a different domain). |
yes, but why limit the possibilities ? i request my resources with a valid session and after that i make a redirect to a external resource (cross domain access). Is this not a use case ? ;) |
What would be needed to get it working? Is it enough just to have some flag to set That's what I meant - I didn't see so far any examples doing non-anonymous CORS requests (what to set on client, what to do on server). It's hard to implement feature you don't know how it works (and no, reading specs didn't help in this case). |
On the server side you need "Access-Control-Allow-Credentials" true in the headers. And on the client you need a modern browser like Chrome or Firefox and crossOrigin = '"use-credentials'. Then the browser send on this request the sessionid in the header. |
And what does it do? Does it show a popup to write down user/pass? What's the UX? |
No, no popup :) you need only a session. And you should set "use-credentials" only on this environment. |
I see.. Well, for |
Or a general load_image in ImageUtils and use it in Loader class. I mean something like the following (pseudo): THREE.Loader.prototype = {
...
function load_image( where, url ) {
...
THREE.ImageUtils.load_image(image, url);
}
THREE.ImageUtils = {
...
load_image: function(image, url) {
var crossOrigin = '';
if(typeof origin_select === 'function') {
crossOrigin = origin_select(url);
}
image.crossOrigin = crossOrigin;
image.src = url;
} "origin_select" is a callback in ImageUtils. Then it is possible to make something application specific per url. |
What's wrong with var loader = new THREE.JSONLoader();
loader.crossOrigin = 'use-credentials'; and THREE.ImageUtils.crossOrigin = 'use-credentials'; No callbacks, no dependencies... ? |
When the server has "Access-Control-Allow-Credentials" = false then it does not work. And for me it is not DRY :) Is it not cleaner when only one instance know something about CORS/ImageLoad ? For me is the Loader above ImageUtils, so the dependencies are correct. Is it not ? (But your idea is much simpler :) |
What doesn't work?
Well, if you think of it in a theorical way, yes. But if you go that way your code becomes harder to follow... order of dependencies at build time change. And then anytime you want to change something you need to keep in mind all the relations and dependencies and well... it just stops being fun :P |
|
|
Should fix mrdoob#779...
@mrdoob , i tested it with dev branch. It doesn't work on all cases: stacktrace: load_image() at Loader.js:139 At first the crossOrigin is defined correctly. But the code in "THREE.Loader.initMaterials(..)" does some wrong things like: "THREE.Loader.prototype.createMaterial" This is called statically ... Should this not fixed ? |
does anyone understand my reported issue ? :) |
Oh, so instead of |
yes, i think so. But i don't know the consequences ... |
Hi
"load_image" of Loader does not support CORS. I think this is a bug. Fix:
Cheers
Remo
The text was updated successfully, but these errors were encountered: