es6-shim breaks Chrome App CSP

[stewart]

Unless sandboxed, es6-shim cannot run inside of Chrome Apps right now due to this line, which triggers the CSP, and is functionally an eval() statement.

I'm unsure if the best fix is reverting ceeb51c, or if a better solution can be found.

[ljharb]

It's definitely an eval statement, and it's the only way to reliably get the global object in any environment.

Are there no CSP directives that can allow for it?

I'm definitely open to another alternative to reliably get the global object if you can suggest one.

[stewart]

There are CSP directives to allow for eval() usage, but they're not permitted in packaged apps, only extensions.

If there's not a clear fix, for now would it make sense to add a caveat to the README indicating that the module will conflict with Content Security Policy in Chrome extensions/apps, and suggest the appropriate fixes (CSP directive or sandboxing)?

[rwaldron]

@ljharb What are the target platforms (general) for es6-shim? Just browser and node, right? (I'm looking for ways around new Function('return this;');

[AndersDJohnson]

+1 Ran into this problem today.

[ljharb]

The target platforms for es6-shim are currently all web browsers, desktop or mobile (ie, run as a normal web page through a normal protocol); and every released version of node (but practically, 0.6 and higher).

Additional platforms are always ideally supported but the decision what to support or not is generally on a case by case basis. #259 specifically was to support node-webkit (#258) which was broken by a fix to make this work in web workers (4e4dc69).

[anthonyryan1]

Skimming the previous issues, this is a summary of my understanding:

	Browsers	Node	Node Webkit	Web Workers
self	yes			yes
window	yes		yes	broken
global		yes	broken

Unless I'm missing something on that table, it sounds like checking self, then window, then global and using the first one found would cover all these environments and avoid eval.

[ljharb]

That definitely seems like it would work :-) In addition, the es*-shims always assume they're the first to run, so shadowing/tampering with global objects isn't much of a concern. However, we don't have any node-webkit nor great web worker code coverage, and I'm worried about adding untested code paths.

It's really a shame that Function('return this')() is so simple, bulletproof, and unfakeable, and that there's no similar non-eval alternative.

If someone were able to prepare a PR that could add test coverage for node-webkit and ensure this path is covered for Web Workers as well, I'd be happy to then modify the getGlobal function to return self || window || global;.

(one idea that pops in my head to ensure correctness is testing that global.Array === [].constructor && global.RegExp === /a/g.constructor && global.Number === 42..constructor && global.String === ''.constructor && global.Function === function () {}.constructor && global.Object === ({}).constructor and throwing an exception if not)

[rwaldron]

However, we don't have any node-webkit

IIRC, you'll get both window and global.

[Yaffle]

can somebody explain me, why this cannot be used ?

(using factory(root) - https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L24 or define(function () { factory(root); }))

[ljharb]

When es6-shim is used in a global context and first-run, perhaps we could. But, whatever method we use needs to work in a CommonJS/AMD context as well, where "this" can not be guaranteed to be the global object.

[Yaffle]

@ljharb , not loader context, but the context of the es6-shim.js, because you are controlling factory call.
And if you are loading es5-shim.js with <script src=..> or with new Worker(...) or with node es5-shim.js (altough, I do not know about new versions of node), this points to the global object for me

[ljharb]

Hmm. It appears that since the initial commit (https://github.com/paulmillr/es6-shim/blame/a57577fdd71cd6726fec6354a1ab296bdd885209/es6-shim.js#L12) the shim's been using "window || global", and over time, evolved to check "self", then to use the current eval approach.

@Yaffle If the es6-shim code happens to be included in a strict mode context, then wouldn't "this" be undefined?

[Yaffle]

<script>
"use strict";
console.log(this.isFinite);
(function (root) {
  "use strict";
  console.log(root.isFinite);
  console.log(this == undefined);
}(this));
</script>

[timmarinin]

This also breaks Firefox OS CSP for privileged apps.

[sandstrom]

CSP is quite handy and — in addition to being used in Chrome apps and Firefox OS — many web applications also use it. We're one of them and just ran into this.

Would anthonyryan1's method work? [https://github.com//issues/301#issuecomment-64966395]

[ljharb]

@sandstrom See my response right below it: #301 (comment)

[stewart]

I think I've got a possible fix here - https://gist.github.com/stewart/321560330b152dfde649.

Didn't cause any issues in the environments I tested in, but maybe I'm missing something?

[ljharb]

@stewart What did you test it on? It would need to work back to IE 6, in node 0.6 and later, all io.js versions, node-webkit/nw.js, web workers, the node vm module's sandbox, and probably in all of a CJS, AMD, and no-module environment for each.

[stewart]

The one major drawback to my approach that the CSP-violating version doesn't have is that if es6-shim gets wrapped up in an IIFE w/ strict mode enabled (say by a minifier or project bundler), it'll stop working.

On the other hand, a situation like that is easier to work around for developers than a CSP-violating dependency, and relatively easy for the library to detect (check if globals === undefined).

Travis indicates at least Node/iojs support about on par with master - https://travis-ci.org/stewart/es6-shim/builds/69499282, I'll see about testing other platforms later (don't have access to IEs older than 10 though, I'm afraid). I did test in Chrome/Safari/Firefox and they all seemed to like it.

[ljharb]

I'm less concerned about browsers and node (where window and global are reliable enough), than I am about web workers and node-webkit, both of which we can't currently test on travis, nor do I have any way to test locally.

[stewart]

I don't have test harnesses available for either at present, but I'll see about getting something worked together over the weekend, unless someone else beats me to it.

[ljharb]

Thanks!

[ljharb]

I've worked on this with @rwaldron and @jugglinmike in person, and we've found a non-eval approach that seems to work in browsers, workers, node-webkit, and node.

In order to unblock Chrome apps from having CSP issues, I'm going to go ahead with this change. Please let me know if this change has broken anything.

[Yaffle]

@ljharb this throws an error when running using WSH.
Altough, es6-shim does not work in WSH anyway...

[ljharb]

Thanks, that's good to know - what is the thrown error?

If you can provide me with various exceptions that the shim throws (as separate issues, please) I'll do my best to fix them!

[Yaffle]

throw new Error('unable to locate global object');

[ljharb]

Perfect, that's exactly what it's supposed to do :-) It appears that in WSH, the only option is to get this in the global context. http://stackoverflow.com/questions/14450424/equivalent-to-window-in-jscript-runtime

I'd suggest then that in wsh, users need to do something like var global = this; in the global context prior to including the es6-shim.

[Yaffle]

@ljharb , setTimeout is not supported, so

var promiseSupportsSubclassing = supportsSubclassing(globals.Promise, function(S) {
            return S.resolve(42).then(function() {}) instanceof S;
        });

promiseSupportsSubclassing test throws an error...

[ljharb]

@Yaffle Try the latest commit in WSH - please file a new issue for me if there's any further issues.

[Yaffle]

@ljharb , well... it works with var global = this; and es5-shim, but throws some errors:

1. es6-shim/es6-shim.js

   Line 1155 in 1bdb1ce

   var NumberShim = function Number(value) {

   
   "Number" is a bad name for IE8 (probablye because of http://kiro.me/blog/nfe_dilemma.html), so before this line is reached, Number is called and throws the error, because isBinary is not defined yet.
2. wrapConstructor uses Object.getOwnPropertyNames, which is not supported in IE8 event with es5-shim;
3. Map and Set are not polyfilled in IE8 because of if (supportsDescriptors) {

[ljharb]

@Yaffle Can you please file an issue for 1 and 2? 3 is expected and desired.

[chmx]

@ljharb Using var global = this; before loading es6-shim works with Nashorn too. Thanks!