Why is using the JavaScript eval function a bad idea?

Why is using the JavaScript eval function a bad idea?

The eval function is a powerful and easy way to dynamically generate code, so what are the caveats?

Solutions/Answers:

Solution 1:

  1. Improper use of eval opens up your
    code for injection attacks

  2. Debugging can be more challenging
    (no line numbers, etc.)

  3. eval’d code executes slower (no opportunity to compile/cache eval’d code)

Edit: As @Jeff Walden points out in comments, #3 is less true today than it was in 2008. However, while some caching of compiled scripts may happen this will only be limited to scripts that are eval’d repeated with no modification. A more likely scenario is that you are eval’ing scripts that have undergone slight modification each time and as such could not be cached. Let’s just say that SOME eval’d code executes more slowly.

Solution 2:

eval isn’t always evil. There are times where it’s perfectly appropriate.

However, eval is currently and historically massively over-used by people who don’t know what they’re doing. That includes people writing JavaScript tutorials, unfortunately, and in some cases this can indeed have security consequences – or, more often, simple bugs. So the more we can do to throw a question mark over eval, the better. Any time you use eval you need to sanity-check what you’re doing, because chances are you could be doing it a better, safer, cleaner way.

To give an all-too-typical example, to set the colour of an element with an id stored in the variable ‘potato’:

eval('document.' + potato + '.style.color = "red"');

If the authors of the kind of code above had a clue about the basics of how JavaScript objects work, they’d have realised that square brackets can be used instead of literal dot-names, obviating the need for eval:

document[potato].style.color = 'red';

…which is much easier to read as well as less potentially buggy.

(But then, someone who /really/ knew what they were doing would say:

document.getElementById(potato).style.color = 'red';

which is more reliable than the dodgy old trick of accessing DOM elements straight out of the document object.)

Solution 3:

I believe it’s because it can execute any JavaScript function from a string. Using it makes it easier for people to inject rogue code into the application.

Solution 4:

Two points come to mind:

  1. Security (but as long as you generate the string to be evaluated yourself, this might be a non-issue)

  2. Performance: until the code to be executed is unknown, it cannot be optimized. (about javascript and performance, certainly Steve Yegge’s presentation)

Solution 5:

Passing user input to eval() is a security risk, but also each invocation of eval() creates a new instance of the JavaScript interpreter. This can be a resource hog.

Solution 6:

It’s generally only an issue if you’re passing eval user input.