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

Access is denied errors on IE11 latest #333

Closed
niemyjski opened this issue Apr 13, 2015 · 50 comments
Closed

Access is denied errors on IE11 latest #333

niemyjski opened this issue Apr 13, 2015 · 50 comments
Assignees
Labels

Comments

@niemyjski
Copy link

I'm running on windows 8.1 x64 and I noticed that I'm getting access denied errors. If it matters I'm running es5 and es6 shim on ie edge.

if (!objectGOPNAcceptsPrimitives) {
      var originalObjectGetOwnPropertyNames = Object.getOwnPropertyNames;
      defineProperty(Object, 'getOwnPropertyNames', function getOwnPropertyNames(value) {
        return originalObjectGetOwnPropertyNames(ES.ToObject(value));
      }, true);
      Value.preserveToString(Object.getOwnPropertyNames, originalObjectGetOwnPropertyNames);
    }

image

image

@KasperLK
Copy link

+1

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

Thanks for the report! I'll check on this asap. Is this the latest published version of es6-shim (which specifically?)?

@niemyjski
Copy link
Author

I was seeing this in 0.27.x as well as 0.28.2

@niemyjski
Copy link
Author

The code looks fine as well as the passed in objects, I'm not sure why ie is blowing up but it is.

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

Are you using any other libraries? react, core.js, things like that? All the tests pass in IE 11 so it's either another library, or I'm missing test coverage :-)

@niemyjski
Copy link
Author

Angular: Here is the site I saw this error on: https://github.com/exceptionless/Exceptionless.ui

@zloirock
Copy link

Interestingly.

@niemyjski
Copy link
Author

Any ideas on the cause?

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

Object.getOwnPropertyNames is working just fine in my manual testing. Exceptionless is pretty big so I'm not sure how I'd figure out the problem, but since this has happened a couple times before, I suspect that something in Exceptionless is either a) modifying globals, or b) using host objects in unexpected ways.

@niemyjski
Copy link
Author

I'm wrapping everything inside of a function and using strict mode. No where am I aware of adding a global / leaking/modifying anything.

@niemyjski
Copy link
Author

I get it on any screen, any of the auth ones / status page are not loading up very much.

@niemyjski
Copy link
Author

Okay! good news, I can reproduce it in a very simple sample:

<!DOCTYPE html>
<html>
    <head>
        <title>Exceptionless</title>
    </head>
    <body>
        <script type="text/javascript" src="https://js.stripe.com/v2/"></script>
        <script src="bower_components/es6-shim/es6-shim.js"></script>
    </body>
</html>

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

ooh, ok so it may be a conflict with Stripe's code. I'll look into that, thanks

@niemyjski
Copy link
Author

I just sent them an email as well with a link to this issue.

@niemyjski
Copy link
Author

Please let me know if there is anything I can do. I think this issue is causing other js errors (because this one is uncaught) on our site with ie11.

@metcalf
Copy link

metcalf commented Apr 13, 2015

I work on Stripe.js and am trying to reproduce the problem. I'm not seeing the same thing on a Windows 8.1 VM with a slightly older version of IE. Does something like this repro the problem for you or do I need something standalone that doesn't frame the content?

https://jsfiddle.net/n09c2jww/1/

@niemyjski
Copy link
Author

Thanks for looking into this. Yes, I can reproduce using that fiddle:
image

@niemyjski
Copy link
Author

image

@zloirock
Copy link

Simple reproducible case with this script, .call(Object, it) not works too:

<script>
var getOwnPropertyNames = Object.getOwnPropertyNames;
Object.getOwnPropertyNames = function(it){
  return getOwnPropertyNames(it);
}
</script>
<script src="https://js.stripe.com/v2/"></script>

F***** IE :)

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

@metcalf Thanks for responding so quickly!

I can not reproduce the error with https://jsfiddle.net/n09c2jww/1/ on IE 11 TP, fwiw.

@zloirock Are you saying that the .call(Object is what's breaking it?

@zloirock
Copy link

@ljharb any wrapper breaking this code.

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

@metcalf Are you doing anything with Object.getOwnPropertyNames in stripe's code besides calling it directly?

@niemyjski
Copy link
Author

If you guys can't reproduce I'll let you repo on my machine :). I've been able to reproduce on two machines.

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

Oddly enough I can't reproduce it on Windows 8.1 IE 11, but I can on Windows 7 IE 11 (on browserstack).

It looks like it's failing on Object.getOwnPropertyNames(window).

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

ah HA - it's because it's receiving a window object that the same origin policy doesn't allow it to access.

At the moment it's failing, value !== window and originalObjectGetOwnPropertyNames(window) works, but originalObjectGetOwnPropertyNames(value) gives an access denied error.

@metcalf This is almost certainly related to iframes, and attempting to enumerate the child iframe's global object from within the parent window - does that seem like it might be right?

@ljharb ljharb added the IE label Apr 13, 2015
@niemyjski
Copy link
Author

I'm seeing this in our app and we have no iframes....

@ljharb
Copy link
Collaborator

ljharb commented Apr 13, 2015

@niemyjski stripe uses one though, i believe

@niemyjski
Copy link
Author

@ljharb ahh :) I haven't looked through their code.

@metcalf
Copy link

metcalf commented Apr 14, 2015

OK, I upgraded IE inside my VM to the latest version and can now reproduce.

@ljharb we do create an iframe for some of the communications back to our API server. This seems to be a more general problem with scripts that work with iframes given that it affects Stripe Checkout as well:
https://jsfiddle.net/tm0bsbt1/

I'm trying to come up with a minimal repro and suspecting something to do with posting messages to the frame or doing some of the related feature detection.

@metcalf
Copy link

metcalf commented Apr 14, 2015

This appears to reproduce simply by embedding a cross-domain iframe in the page somewhere:
https://jsfiddle.net/tm0bsbt1/9/

The issue goes away if you point the frame at "about:blank":
https://jsfiddle.net/tm0bsbt1/12/

@ljharb
Copy link
Collaborator

ljharb commented Apr 14, 2015

@metcalf I can't reproduce it on https://jsfiddle.net/tm0bsbt1/9/ - but still can on https://jsfiddle.net/tm0bsbt1/ . Could you provide the lines of your stripe code that's calling Object.getOwnPropertyNames? https://checkout.stripe.com/checkout.js is minified.

@metcalf
Copy link

metcalf commented Apr 15, 2015

I don't actually see anything from our code in the stack trace, nor do we call getOwnPropertyNames anywhere in checkout.js. I just double checked the repro on https://jsfiddle.net/tm0bsbt1/9/. Are you using Windows 8.1 with build 11.0.9600.0.17960?

My suspicion is that IE is calling getOwnPropertyNames internally somewhere in the layout engine. The native implementation has special, elevated permissions but overriding it means that the engine calls out to "user space" javascript and drops permissions. Just a guess but it seems consistent with the lack of stack trace, lack of call to getOwnPropertyNames and repro with a simple iframe.

@niemyjski
Copy link
Author

I'm able to reproduce (ran windows updates yest) using that sample on
image

@ljharb
Copy link
Collaborator

ljharb commented Apr 15, 2015

@metcalf I think you're correct.

I'm not sure sure what to do here:

  • It would be a shame to make Object.getOwnPropertyNames not accept primitives in the shim everywhere just to accomodate this one specific IE 11 on specific Windows versions.
  • It wouldn't be acceptable imo to have it only not accept primitives on that one browser - since that'd just be pushing the exception to a later time.

@use-strict
Copy link

Reproduces like described by @niemyjski, with a twitter widget iframe. It appears that somehow IE uses the patched Object inside the iframe and can no longer access the code which originated from a different domain (the parent frame). Edit: stack trace starts from twitter sdk.js, inside the iframe.

@ljharb, given this limitation, it may not be possible to polyfill object static methods to accept primitives on IE. It would be safer to treat this as non-polyfillable and exclude it completely, adding a notice about this limitation on the readme, similar to WeakMap-s. I wonder why/if this happens for other polyfilled methods in a similar manner or it's just specific to Object or Object.getOwnPropertyNames. It could have bigger implications.

The second option that you suggested doesn't help at all, because it defeats the purpose of the polyfill, which is to provide the API on any non-ES6 compliant browsers. Writing a browser dependent polyfill makes no sense imho.

@ljharb
Copy link
Collaborator

ljharb commented Apr 20, 2015

@use-strict In order to avoid polyfilling it on IE (and it seems specific to IE 11 only), I'd need a reliable way to detect it in JS - the iframe approach doesn't seem try/catchable, and using window.onerror is not something I'd feel comfortable doing.

I agree that the second option defeats the purpose of the polyfill.

Since this seems to only occur for getOwnPropertyNames, that's the only one I'm concerned with at the moment.

@ljharb
Copy link
Collaborator

ljharb commented Apr 20, 2015

I've spoken to the IE team and this bug has been filed, so hopefully that will either/both shed some light on how to detect/avoid this problem, and fix it in future versions (hopefully in an IE 11 patch even).

@chris-shelton
Copy link

I'm having this same issue too. I've even tried other polyfils like core-js, babel's browser-polyfill.js file, all come up with the same error ass above, the getOwnPropertyNames(it)...it being the window. 'access is denied'.

edit: Another thing, I simple typed in document.frames in the console and it immediately threw the same error. Even though we are not using iframes anywhere.

@niemyjski
Copy link
Author

@ljharb did you ever hear back from the IE team?

@ljharb
Copy link
Collaborator

ljharb commented May 13, 2015

I'm afraid nothing just yet. I'll ping again.

@knownasilya
Copy link

Any progress on this?

@ljharb
Copy link
Collaborator

ljharb commented May 28, 2015

It's not looking like it'll be fixed in anything but the latest IE, if that.

I'm likely to remove this from the shim if I can't come up with a way to fix it.

@seeden
Copy link

seeden commented May 28, 2015

+1 for fix or remove in short run

@jacobq
Copy link

jacobq commented May 28, 2015

+1 for remove (or disable for IE11)

@ljharb
Copy link
Collaborator

ljharb commented May 28, 2015

I definitely won't conditionally disable it for IE. I think I can probably fix it, but I'll need time to figure that out.

@jacobq
Copy link

jacobq commented May 28, 2015

@ljharb fix is even better -- I was just in panic mode last night because I got a "sev 1" bug report due to updating ES6-shim from v0.20.4 to v0.31.2 in a QA environment without testing in IE (shame on me), which resulted in a large number of pages blowing up. I downgraded to v0.24.0 to resolve for now. It just seemed to me that it'd be better not to have a feature than to break things :)

BTW, thanks for your huge contributions to this project.

@ljharb ljharb closed this as completed in cee098d Jun 1, 2015
@ljharb ljharb self-assigned this Jun 1, 2015
@niemyjski
Copy link
Author

@ljharb Do you know when this fix will be released?

@ljharb
Copy link
Collaborator

ljharb commented Jun 2, 2015

I should have it out today or tomorrow.

@ljharb
Copy link
Collaborator

ljharb commented Jun 2, 2015

Released as v0.31.3

@niemyjski
Copy link
Author

Thank you!

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

No branches or pull requests

10 participants