From 703e566aa125e19975e94e58f241fd4f61d0467b Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
Date: Tue, 2 Feb 2021 15:22:04 +0100
Subject: [PATCH] repl: refactor to avoid unsafe array iteration
PR-URL: https://github.com/nodejs/node/pull/37188
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
---
lib/readline.js | 2 +-
lib/repl.js | 22 +--
test/parallel/test-repl-autocomplete.js | 215 ++++++++++++++++++++++++
3 files changed, 227 insertions(+), 12 deletions(-)
create mode 100644 test/parallel/test-repl-autocomplete.js
diff --git a/lib/readline.js b/lib/readline.js
index 616a4a2fc270c5..9cad848bf45380 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -615,7 +615,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;
diff --git a/lib/repl.js b/lib/repl.js
index 09494c3f2fcf15..d5d9371de34428 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -54,6 +54,7 @@ const {
ArrayPrototypePush,
ArrayPrototypeReverse,
ArrayPrototypeShift,
+ ArrayPrototypeSome,
ArrayPrototypeSort,
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
@@ -1155,7 +1156,7 @@ REPLServer.prototype.complete = function() {
function gracefulReaddir(...args) {
try {
- return fs.readdirSync(...args);
+ return ReflectApply(fs.readdirSync, null, args);
} catch {}
}
@@ -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);
@@ -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);
diff --git a/test/parallel/test-repl-autocomplete.js b/test/parallel/test-repl-autocomplete.js
new file mode 100644
index 00000000000000..e1d2189fdb17cb
--- /dev/null
+++ b/test/parallel/test-repl-autocomplete.js
@@ -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();