Skip to content

Commit

Permalink
repl: refactor to avoid unsafe array iteration
Browse files Browse the repository at this point in the history
PR-URL: #37188
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
aduh95 authored and danielleadams committed Feb 16, 2021
1 parent eb7ec1b commit c377834
Show file tree
Hide file tree
Showing 3 changed files with 227 additions and 12 deletions.
2 changes: 1 addition & 1 deletion lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) {
}

// Result and the text that was completed.
const [completions, completeOn] = value;
const { 0: completions, 1: completeOn } = value;

if (!completions || completions.length === 0) {
return;
Expand Down
22 changes: 11 additions & 11 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const {
ArrayPrototypePush,
ArrayPrototypeReverse,
ArrayPrototypeShift,
ArrayPrototypeSome,
ArrayPrototypeSort,
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
Expand Down Expand Up @@ -1155,7 +1156,7 @@ REPLServer.prototype.complete = function() {

function gracefulReaddir(...args) {
try {
return fs.readdirSync(...args);
return ReflectApply(fs.readdirSync, null, args);
} catch {}
}

Expand Down Expand Up @@ -1236,11 +1237,11 @@ function complete(line, callback) {
ArrayPrototypeForEach(paths, (dir) => {
dir = path.resolve(dir, subdir);
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
for (const dirent of dirents) {
ArrayPrototypeForEach(dirents, (dirent) => {
if (RegExpPrototypeTest(versionedFileNamesRe, dirent.name) ||
dirent.name === '.npm') {
// Exclude versioned names that 'npm' installs.
continue;
return;
}
const extension = path.extname(dirent.name);
const base = StringPrototypeSlice(dirent.name, 0, -extension.length);
Expand All @@ -1249,18 +1250,17 @@ function complete(line, callback) {
(!subdir || base !== 'index')) {
ArrayPrototypePush(group, `${subdir}${base}`);
}
continue;
return;
}
ArrayPrototypePush(group, `${subdir}${dirent.name}/`);
const absolute = path.resolve(dir, dirent.name);
const subfiles = gracefulReaddir(absolute) || [];
for (const subfile of subfiles) {
if (ArrayPrototypeIncludes(indexes, subfile)) {
ArrayPrototypePush(group, `${subdir}${dirent.name}`);
break;
}
if (ArrayPrototypeSome(
gracefulReaddir(absolute) || [],
(subfile) => ArrayPrototypeIncludes(indexes, subfile)
)) {
ArrayPrototypePush(group, `${subdir}${dirent.name}`);
}
}
});
});
if (group.length) {
ArrayPrototypePush(completionGroups, group);
Expand Down
215 changes: 215 additions & 0 deletions test/parallel/test-repl-autocomplete.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
'use strict';

// Flags: --expose-internals

const common = require('../common');
const stream = require('stream');
const REPL = require('internal/repl');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const { inspect } = require('util');

common.skipIfDumbTerminal();

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

process.throwDeprecation = true;

const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history');

// Create an input stream specialized for testing an array of actions
class ActionStream extends stream.Stream {
run(data) {
const _iter = data[Symbol.iterator]();
const doAction = () => {
const next = _iter.next();
if (next.done) {
// Close the repl. Note that it must have a clean prompt to do so.
this.emit('keypress', '', { ctrl: true, name: 'd' });
return;
}
const action = next.value;

if (typeof action === 'object') {
this.emit('keypress', '', action);
} else {
this.emit('data', `${action}`);
}
setImmediate(doAction);
};
doAction();
}
resume() {}
pause() {}
}
ActionStream.prototype.readable = true;

// Mock keys
const ENTER = { name: 'enter' };
const UP = { name: 'up' };
const DOWN = { name: 'down' };
const LEFT = { name: 'left' };
const RIGHT = { name: 'right' };
const BACKSPACE = { name: 'backspace' };
const TABULATION = { name: 'tab' };
const WORD_LEFT = { name: 'left', ctrl: true };
const WORD_RIGHT = { name: 'right', ctrl: true };
const GO_TO_END = { name: 'end' };
const SIGINT = { name: 'c', ctrl: true };
const ESCAPE = { name: 'escape', meta: true };

const prompt = '> ';

const tests = [
{
env: { NODE_REPL_HISTORY: defaultHistoryPath },
test: (function*() {
// Deleting Array iterator should not break history feature.
//
// Using a generator function instead of an object to allow the test to
// keep iterating even when Array.prototype[Symbol.iterator] has been
// deleted.
yield 'const ArrayIteratorPrototype =';
yield ' Object.getPrototypeOf(Array.prototype[Symbol.iterator]());';
yield ENTER;
yield 'const {next} = ArrayIteratorPrototype;';
yield ENTER;
yield 'const realArrayIterator = Array.prototype[Symbol.iterator];';
yield ENTER;
yield 'delete Array.prototype[Symbol.iterator];';
yield ENTER;
yield 'delete ArrayIteratorPrototype.next;';
yield ENTER;
yield UP;
yield UP;
yield DOWN;
yield DOWN;
yield 'fu';
yield 'n';
yield RIGHT;
yield BACKSPACE;
yield LEFT;
yield LEFT;
yield 'A';
yield BACKSPACE;
yield GO_TO_END;
yield BACKSPACE;
yield WORD_LEFT;
yield WORD_RIGHT;
yield ESCAPE;
yield ENTER;
yield 'require("./';
yield TABULATION;
yield SIGINT;
yield 'Array.proto';
yield RIGHT;
yield '.pu';
yield ENTER;
yield 'ArrayIteratorPrototype.next = next;';
yield ENTER;
yield 'Array.prototype[Symbol.iterator] = realArrayIterator;';
yield ENTER;
})(),
expected: [],
clean: false
},
];
const numtests = tests.length;

const runTestWrap = common.mustCall(runTest, numtests);

function cleanupTmpFile() {
try {
// Write over the file, clearing any history
fs.writeFileSync(defaultHistoryPath, '');
} catch (err) {
if (err.code === 'ENOENT') return true;
throw err;
}
return true;
}

function runTest() {
const opts = tests.shift();
if (!opts) return; // All done

const { expected, skip } = opts;

// Test unsupported on platform.
if (skip) {
setImmediate(runTestWrap, true);
return;
}
const lastChunks = [];
let i = 0;

REPL.createInternalRepl(opts.env, {
input: new ActionStream(),
output: new stream.Writable({
write(chunk, _, next) {
const output = chunk.toString();

if (!opts.showEscapeCodes &&
(output[0] === '\x1B' || /^[\r\n]+$/.test(output))) {
return next();
}

lastChunks.push(output);

if (expected.length && !opts.checkTotal) {
try {
assert.strictEqual(output, expected[i]);
} catch (e) {
console.error(`Failed test # ${numtests - tests.length}`);
console.error('Last outputs: ' + inspect(lastChunks, {
breakLength: 5, colors: true
}));
throw e;
}
// TODO(BridgeAR): Auto close on last chunk!
i++;
}

next();
}
}),
allowBlockingCompletions: true,
completer: opts.completer,
prompt,
useColors: false,
preview: opts.preview,
terminal: true
}, function(err, repl) {
if (err) {
console.error(`Failed test # ${numtests - tests.length}`);
throw err;
}

repl.once('close', () => {
if (opts.clean)
cleanupTmpFile();

if (opts.checkTotal) {
assert.deepStrictEqual(lastChunks, expected);
} else if (expected.length !== i) {
console.error(tests[numtests - tests.length - 1]);
throw new Error(`Failed test # ${numtests - tests.length}`);
}

setImmediate(runTestWrap, true);
});

if (opts.columns) {
Object.defineProperty(repl, 'columns', {
value: opts.columns,
enumerable: true
});
}
repl.input.run(opts.test);
});
}

// run the tests
runTest();

0 comments on commit c377834

Please sign in to comment.