YuzuJS/setImmediate

new Function('return this')() is eval == bad code

Closed this issue · 8 comments

duzun commented

I'm considering using this library in a Firefox Addon and Chrome Extension, but because of the last line my addon will be rejected by Mozilla Addon reviewers, as it is using eval.

Chrome also complains:
Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' blob: filesystem: chrome-extension-resource:"., and I don't like the idea of relaxing Content Security Policy.

Is there a special reason for getting the global that way?

Why not typeof global != 'undefined' ? global : this ?
This should work in all browsers/extensions + node.js.

We need eval anyway because the spec mandates that any string arguments are evaled. So if your goal is to use this in a CSP context you cannot use this library.

duzun commented

I agree we need eval for setImmediate(), but not sure about global.
For eg. AMO validation tool allows setTimeout(function(){}), but doesn't allow setTimeout('string')
Same is true with Chrome Extension - unless you touch eval, it can sit there in some function body.

It's not needed for global; a chain of conditions would work as well. But since the library already uses eval for the main implementation, there's no harm in using it to get the global. Unless I am missing something.

duzun commented

There is "harm".
User of this library can choose to never use eval by never supplying 'string' to setImmediate.
But new Function("return this")() from the bottom always gets called and thus deprives the user from the "right" to never use eval.

I just replaced that code with typeof window == 'undefined' ? typeof global == 'undefined' ? this : global : window and now I'm able to use the library in Chrome Extension.

The code:
setTimeout(function(){log('timeout')}); setImmediate(function(){log('immediate')});
outputs:
immediate
timeout

and no more errors in the console.

OK, I think I see. If you replace window with self so that it works in web workers, I would accept a pull request.

duzun commented

Great!

Thanks!

duzun commented

@HounD complains about my PR.

Please, tell me, how should I make it right?

It is complaining too much. That must have been put in by YuzuJS people after I left. I will do some style tweaks when I merge.