new Function('return this')() is eval == bad code
Closed this issue · 8 comments
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.
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.
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.
Great!
Thanks!
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.