NeilFraser
commented
3 years ago Testing this class of problem in the Chrome/Firefox consoles is tricky, since just typing/pasting the statement without pressing enter crashes the tab, since the console attempts to show a preview of the result as you type.
Other functions which are similarly affected include:
Array.prototype.indexOf.call({0: true, length: 'Infinity'}, false);
Array.prototype.shift.call({0: true, length: 'Infinity'});
Array.prototype.reverse.call({0: true, length: 'Infinity'});
Array.prototype.sort.call({0: true, length: 'Infinity'});I can't find any pathological behaviour for pop. And push, unshift, splice, slice, and join all appear to be safe since the native interpreter throws either RangeError or TypeError. And every, filter, foreach, map, reduce, reduceRight, some, and toLocaleString are all polyfills, so if they loop forever that's not our problem.
My suggestion is that all non-polyfilled functions (including the ones that apparently don't need it) have something like this added to the top of them:
if (isNaN(Interpreter.legalArrayLength(thisInterpreter.getProperty(this, 'length') || 0))) {
thisInterpreter.throwException(thisInterpreter.RANGE_ERROR, 'Invalid array length');
}Worth noting that this solution changes the behaviour in the case of a length getter:
var o = {0: true, get length() {return 'Infinity'}};
alert(Array.prototype.lastIndexOf.call(o, true));In native JavaScript this hangs the thread (but is trying to return 0). In the current JS-Interpreter it promptly returns -1 (which is wrong). Under the proposed solution it would throw 'Error: Getter not supported in that context' which is consistent with us not supporting getters and setters on arrays. I think this change is positive.
@cpcallen Can I get your LGTM on this approach, as well as your eyes on the equivalent parts of Code City?
The following takes forever to execute:
Array.prototype.lastIndexOf.call({0: true, length: 'Infinity'}, true);This hangs all browsers, node, and the JS-Interpreter.