Access is denied errors on IE11 latest

niemyjski commented 9 years ago

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);
    }

KasperLK commented 9 years ago

+1

ljharb commented 9 years ago

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

niemyjski commented 9 years ago

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

niemyjski commented 9 years ago

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

ljharb commented 9 years ago

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 commented 9 years ago

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

zloirock commented 9 years ago

Interestingly.

niemyjski commented 9 years ago

Any ideas on the cause?

ljharb commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

niemyjski commented 9 years ago

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 commented 9 years ago

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

niemyjski commented 9 years ago

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

niemyjski commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

niemyjski commented 9 years ago

zloirock commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

@ljharb any wrapper breaking this code.

ljharb commented 9 years ago

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

niemyjski commented 9 years ago

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

ljharb commented 9 years ago

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 commented 9 years ago

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?

niemyjski commented 9 years ago

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

ljharb commented 9 years ago

@niemyjski stripe uses one though, i believe

niemyjski commented 9 years ago

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

metcalf commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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

ljharb commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

ljharb commented 9 years ago

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

knownasilya commented 9 years ago

Any progress on this?

ljharb commented 9 years ago

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 commented 9 years ago

+1 for fix or remove in short run

jacobq commented 9 years ago

+1 for remove (or disable for IE11)

ljharb commented 9 years ago

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 commented 9 years ago

@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.

niemyjski commented 9 years ago

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

ljharb commented 9 years ago

I should have it out today or tomorrow.

ljharb commented 9 years ago

Released as v0.31.3

niemyjski commented 9 years ago

Thank you!

paulmillr / es6-shim

Access is denied errors on IE11 latest #333