Checking of strings within eval(), setTimeout(), etc #514

Merged
merged 6 commits into from Aug 29, 2012
View
@@ -200,13 +200,13 @@
close, closed, closure, comment, condition, confirm, console, constructor,
content, couch, create, css, curly, d, data, datalist, dd, debug, decodeURI,
decodeURIComponent, defaultStatus, defineClass, deserialize, devel, document,
- dojo, dijit, dojox, define, else, emit, encodeURI, encodeURIComponent,
+ dojo, dijit, dojox, define, elem, else, emit, encodeURI, encodeURIComponent,
entityify, eqeq, eqeqeq, eqnull, errors, es5, escape, esnext, eval, event, evidence, evil,
ex, exception, exec, exps, expr, exports, FileReader, first, floor, focus,
forin, fragment, frames, from, fromCharCode, fud, funcscope, funct, function, functions,
g, gc, getComputedStyle, getRow, getter, getterToken, GLOBAL, global, globals, globalstrict,
hasOwnProperty, help, history, i, id, identifier, immed, implieds, importPackage, include,
- indent, indexOf, init, ins, instanceOf, isAlpha, isApplicationRunning, isArray,
+ indent, indexOf, init, ins, internals, instanceOf, isAlpha, isApplicationRunning, isArray,
isDigit, isFinite, isNaN, iterator, java, join, jshint,
JSHINT, json, jquery, jQuery, keys, label, labelled, last, lastsemic, laxbreak, laxcomma,
latedef, lbp, led, left, length, line, load, loadClass, localStorage, location,
@@ -218,7 +218,7 @@
proto, prototype, prototypejs, provides, push, quit, range, raw, reach, reason, regexp,
readFile, readUrl, regexdash, removeEventListener, replace, report, require,
reserved, resizeBy, resizeTo, resolvePath, resumeUpdates, respond, rhino, right,
- runCommand, scroll, screen, scripturl, scrollBy, scrollTo, scrollbar, search, seal,
+ runCommand, scope, scroll, screen, scripturl, scrollBy, scrollTo, scrollbar, search, seal,
send, serialize, sessionStorage, setInterval, setTimeout, setter, setterToken, shift, slice,
smarttabs, sort, spawn, split, stack, status, start, strict, sub, substr, supernew, shadow,
supplant, sum, sync, test, toLowerCase, toString, toUpperCase, toint32, token, top, trailing,
@@ -1010,6 +1010,7 @@ var JSHINT = (function () {
evidence: lines[l - 1] || '',
line: l,
character: ch,
+ scope: JSHINT.scope,
a: a,
b: b,
c: c,
@@ -1045,6 +1046,17 @@ var JSHINT = (function () {
}, a, b, c, d);
}
+ // Tracking of "internal" scripts, like eval containing a static string
+ function addInternalSrc(elem, src) {
+ var i;
+ i = {
+ id: '(internal)',
+ elem: elem,
+ value: src
+ };
+ JSHINT.internals.push(i);
+ return i;
+ }
// lexical analysis and token construction
@@ -3145,11 +3157,25 @@ loop: for (;;) {
if (left.value === 'eval' || left.value === 'Function' ||
left.value === 'execScript') {
warning("eval is evil.", left);
+ if (p[0] && p[0].id === '(string)') {
@valueof
valueof May 28, 2012 Member

Do you need to check for (string) here? What else could be inside of eval?

@TheJosh
TheJosh May 28, 2012 Contributor

Some sort of non-constant expression or a function call

+ addInternalSrc(left, p[0].value);
+ }
} else if (p[0] && p[0].id === '(string)' &&
(left.value === 'setTimeout' ||
left.value === 'setInterval')) {
warning(
"Implied eval is evil. Pass a function instead of a string.", left);
+ addInternalSrc(left, p[0].value);
+
+ // window.setTimeout/setInterval
+ } else if (p[0] && p[0].id === '(string)' &&
+ left.value === '.' &&
+ left.left.value === 'window' &&
+ (left.right === 'setTimeout' ||
+ left.right === 'setInterval')) {
+ warning(
+ "Implied eval is evil. Pass a function instead of a string.", left);
+ addInternalSrc(left, p[0].value);
}
}
if (!left.identifier && left.id !== '.' && left.id !== '[' &&
@@ -4121,13 +4147,19 @@ loop: for (;;) {
// The actual JSHINT function itself.
- var itself = function (s, o, g) {
+ var itself = function (s, o, g, n) {
var a, i, k, x,
optionKeys,
newOptionObj = {};
- JSHINT.errors = [];
- JSHINT.undefs = [];
+ if (typeof(n) === 'undefined') {
@valueof
valueof May 28, 2012 Member

Hm, why do we need another argument here?

@TheJosh
TheJosh May 28, 2012 Contributor

That argument is the "scope". It goes into the error messages, and changes the behavior a little. It's undefined for the main function call, and then defined when the function is re-called the secondary times.

+ JSHINT.errors = [];
+ JSHINT.undefs = [];
+ JSHINT.internals = [];
+ JSHINT.scope = '(main)';
+ } else {
+ JSHINT.scope = n;
+ }
predefined = Object.create(standard);
combine(predefined, g || {});
if (o) {
@@ -4280,6 +4312,14 @@ loop: for (;;) {
}
}
+ // Loop over the listed "internals", and check them too
+ if (typeof(n) === 'undefined') {
+ for (i = 0; i < JSHINT.internals.length; i += 1) {
+ k = JSHINT.internals[i];
+ itself(k.value, o, g, k.elem);
+ }
+ }
+
return JSHINT.errors.length === 0;
};
View
@@ -314,6 +314,30 @@ exports.yesEmptyStmt = function () {
.test(src, { curly: false, expr: true });
};
+exports.insideEval = function () {
+ var src = fs.readFileSync(__dirname + '/fixtures/insideEval.js', 'utf8');
+
+ TestRun()
+ .addError(1, "eval is evil.")
+ .addError(3, "eval is evil.")
+ .addError(5, "eval is evil.")
+ .addError(7, "eval is evil.")
+ .addError(9, "eval is evil.")
+ .addError(11, "Implied eval is evil. Pass a function instead of a string.")
+ .addError(13, "Implied eval is evil. Pass a function instead of a string.")
+ .addError(15, "Implied eval is evil. Pass a function instead of a string.")
+ .addError(17, "Implied eval is evil. Pass a function instead of a string.")
+
+ // The "TestRun" class (and these errors) probably needs some
+ // facility for checking the expected scope of the error
+ .addError(1, "Unexpected early end of program.")
+ .addError(1, "Expected an identifier and instead saw '(end)'.")
+ .addError(1, "Expected ')' and instead saw ''.")
+ .addError(1, "Missing semicolon.")
+
+ .test(src, { evil: false });
+};
+
// Regression test for GH-394.
exports.noExcOnTooManyUndefined = function () {
var code = 'a(); b();';
@@ -0,0 +1,17 @@
+eval("func_" + 123 + "();");
+
+eval("func_" + func() + "();");
+
+eval("evil_yet_valid();");
+
+eval("evil_and_invalid(");
+
+eval("this_" + "can_be" + "_tested_too();");
+
+setTimeout("evil_yet_valid();", 1000);
+
+setTimeout("evil_and_invalid(", 1000);
+
+window.setTimeout("evil_yet_valid();", 1000);
+
+window.setTimeout("evil_and_invalid(", 1000);