search

Khan / live-editor

A browser-based live coding environment.
Other
761 stars 184 forks source link

warn when people redefine builtin methods #399

Open kevinbarabash opened 9 years ago

kevinbarabash commented 9 years ago

Discovered by @SpongeJr in https://www.khanacademy.org/computer-programming/light-reflection-and-refraction/5872552844722176.

kevinbarabash commented 9 years ago

It causes escodegen to mess up because it relies on being able to push multiple values at the same time.

benburrill commented 9 years ago

Does redefining methods like that pose a security risk? Personally I think that although the result is pretty nasty if you actually do it, it's not a bug with KA, but a bug with your code. Redefining Array.prototype.push is usually a terrible idea - but why stop users from using a polyfill method or extending the prototypes of builtins just because you can break things if you want to.

If you really think this is a good idea, we shouldn't limit it to Array.prototype and start mass producing delicious JavaScript-library-flavored ice cream.

kevinbarabash commented 9 years ago

Warning against it would probably bet a nicer way of dealing with this issue.