Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit c377834

Browse filesBrowse files
aduh95danielleadams
authored andcommitted
repl: refactor to avoid unsafe array iteration
PR-URL: #37188 Reviewed-By: Zijian Liu <lxxyxzj@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
1 parent eb7ec1b commit c377834
Copy full SHA for c377834

File tree

Expand file treeCollapse file tree

3 files changed

+227
-12
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+227
-12
lines changed
Open diff view settings
Collapse file

‎lib/readline.js‎

Copy file name to clipboardExpand all lines: lib/readline.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) {
607607
}
608608

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

612612
if (!completions || completions.length === 0) {
613613
return;
Collapse file

‎lib/repl.js‎

Copy file name to clipboardExpand all lines: lib/repl.js
+11-11Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const {
5454
ArrayPrototypePush,
5555
ArrayPrototypeReverse,
5656
ArrayPrototypeShift,
57+
ArrayPrototypeSome,
5758
ArrayPrototypeSort,
5859
ArrayPrototypeSplice,
5960
ArrayPrototypeUnshift,
@@ -1155,7 +1156,7 @@ REPLServer.prototype.complete = function() {
11551156

11561157
function gracefulReaddir(...args) {
11571158
try {
1158-
return fs.readdirSync(...args);
1159+
return ReflectApply(fs.readdirSync, null, args);
11591160
} catch {}
11601161
}
11611162

@@ -1236,11 +1237,11 @@ function complete(line, callback) {
12361237
ArrayPrototypeForEach(paths, (dir) => {
12371238
dir = path.resolve(dir, subdir);
12381239
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
1239-
for (const dirent of dirents) {
1240+
ArrayPrototypeForEach(dirents, (dirent) => {
12401241
if (RegExpPrototypeTest(versionedFileNamesRe, dirent.name) ||
12411242
dirent.name === '.npm') {
12421243
// Exclude versioned names that 'npm' installs.
1243-
continue;
1244+
return;
12441245
}
12451246
const extension = path.extname(dirent.name);
12461247
const base = StringPrototypeSlice(dirent.name, 0, -extension.length);
@@ -1249,18 +1250,17 @@ function complete(line, callback) {
12491250
(!subdir || base !== 'index')) {
12501251
ArrayPrototypePush(group, `${subdir}${base}`);
12511252
}
1252-
continue;
1253+
return;
12531254
}
12541255
ArrayPrototypePush(group, `${subdir}${dirent.name}/`);
12551256
const absolute = path.resolve(dir, dirent.name);
1256-
const subfiles = gracefulReaddir(absolute) || [];
1257-
for (const subfile of subfiles) {
1258-
if (ArrayPrototypeIncludes(indexes, subfile)) {
1259-
ArrayPrototypePush(group, `${subdir}${dirent.name}`);
1260-
break;
1261-
}
1257+
if (ArrayPrototypeSome(
1258+
gracefulReaddir(absolute) || [],
1259+
(subfile) => ArrayPrototypeIncludes(indexes, subfile)
1260+
)) {
1261+
ArrayPrototypePush(group, `${subdir}${dirent.name}`);
12621262
}
1263-
}
1263+
});
12641264
});
12651265
if (group.length) {
12661266
ArrayPrototypePush(completionGroups, group);
Collapse file
+215Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
'use strict';
2+
3+
// Flags: --expose-internals
4+
5+
const common = require('../common');
6+
const stream = require('stream');
7+
const REPL = require('internal/repl');
8+
const assert = require('assert');
9+
const fs = require('fs');
10+
const path = require('path');
11+
const { inspect } = require('util');
12+
13+
common.skipIfDumbTerminal();
14+
15+
const tmpdir = require('../common/tmpdir');
16+
tmpdir.refresh();
17+
18+
process.throwDeprecation = true;
19+
20+
const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history');
21+
22+
// Create an input stream specialized for testing an array of actions
23+
class ActionStream extends stream.Stream {
24+
run(data) {
25+
const _iter = data[Symbol.iterator]();
26+
const doAction = () => {
27+
const next = _iter.next();
28+
if (next.done) {
29+
// Close the repl. Note that it must have a clean prompt to do so.
30+
this.emit('keypress', '', { ctrl: true, name: 'd' });
31+
return;
32+
}
33+
const action = next.value;
34+
35+
if (typeof action === 'object') {
36+
this.emit('keypress', '', action);
37+
} else {
38+
this.emit('data', `${action}`);
39+
}
40+
setImmediate(doAction);
41+
};
42+
doAction();
43+
}
44+
resume() {}
45+
pause() {}
46+
}
47+
ActionStream.prototype.readable = true;
48+
49+
// Mock keys
50+
const ENTER = { name: 'enter' };
51+
const UP = { name: 'up' };
52+
const DOWN = { name: 'down' };
53+
const LEFT = { name: 'left' };
54+
const RIGHT = { name: 'right' };
55+
const BACKSPACE = { name: 'backspace' };
56+
const TABULATION = { name: 'tab' };
57+
const WORD_LEFT = { name: 'left', ctrl: true };
58+
const WORD_RIGHT = { name: 'right', ctrl: true };
59+
const GO_TO_END = { name: 'end' };
60+
const SIGINT = { name: 'c', ctrl: true };
61+
const ESCAPE = { name: 'escape', meta: true };
62+
63+
const prompt = '> ';
64+
65+
const tests = [
66+
{
67+
env: { NODE_REPL_HISTORY: defaultHistoryPath },
68+
test: (function*() {
69+
// Deleting Array iterator should not break history feature.
70+
//
71+
// Using a generator function instead of an object to allow the test to
72+
// keep iterating even when Array.prototype[Symbol.iterator] has been
73+
// deleted.
74+
yield 'const ArrayIteratorPrototype =';
75+
yield ' Object.getPrototypeOf(Array.prototype[Symbol.iterator]());';
76+
yield ENTER;
77+
yield 'const {next} = ArrayIteratorPrototype;';
78+
yield ENTER;
79+
yield 'const realArrayIterator = Array.prototype[Symbol.iterator];';
80+
yield ENTER;
81+
yield 'delete Array.prototype[Symbol.iterator];';
82+
yield ENTER;
83+
yield 'delete ArrayIteratorPrototype.next;';
84+
yield ENTER;
85+
yield UP;
86+
yield UP;
87+
yield DOWN;
88+
yield DOWN;
89+
yield 'fu';
90+
yield 'n';
91+
yield RIGHT;
92+
yield BACKSPACE;
93+
yield LEFT;
94+
yield LEFT;
95+
yield 'A';
96+
yield BACKSPACE;
97+
yield GO_TO_END;
98+
yield BACKSPACE;
99+
yield WORD_LEFT;
100+
yield WORD_RIGHT;
101+
yield ESCAPE;
102+
yield ENTER;
103+
yield 'require("./';
104+
yield TABULATION;
105+
yield SIGINT;
106+
yield 'Array.proto';
107+
yield RIGHT;
108+
yield '.pu';
109+
yield ENTER;
110+
yield 'ArrayIteratorPrototype.next = next;';
111+
yield ENTER;
112+
yield 'Array.prototype[Symbol.iterator] = realArrayIterator;';
113+
yield ENTER;
114+
})(),
115+
expected: [],
116+
clean: false
117+
},
118+
];
119+
const numtests = tests.length;
120+
121+
const runTestWrap = common.mustCall(runTest, numtests);
122+
123+
function cleanupTmpFile() {
124+
try {
125+
// Write over the file, clearing any history
126+
fs.writeFileSync(defaultHistoryPath, '');
127+
} catch (err) {
128+
if (err.code === 'ENOENT') return true;
129+
throw err;
130+
}
131+
return true;
132+
}
133+
134+
function runTest() {
135+
const opts = tests.shift();
136+
if (!opts) return; // All done
137+
138+
const { expected, skip } = opts;
139+
140+
// Test unsupported on platform.
141+
if (skip) {
142+
setImmediate(runTestWrap, true);
143+
return;
144+
}
145+
const lastChunks = [];
146+
let i = 0;
147+
148+
REPL.createInternalRepl(opts.env, {
149+
input: new ActionStream(),
150+
output: new stream.Writable({
151+
write(chunk, _, next) {
152+
const output = chunk.toString();
153+
154+
if (!opts.showEscapeCodes &&
155+
(output[0] === '\x1B' || /^[\r\n]+$/.test(output))) {
156+
return next();
157+
}
158+
159+
lastChunks.push(output);
160+
161+
if (expected.length && !opts.checkTotal) {
162+
try {
163+
assert.strictEqual(output, expected[i]);
164+
} catch (e) {
165+
console.error(`Failed test # ${numtests - tests.length}`);
166+
console.error('Last outputs: ' + inspect(lastChunks, {
167+
breakLength: 5, colors: true
168+
}));
169+
throw e;
170+
}
171+
// TODO(BridgeAR): Auto close on last chunk!
172+
i++;
173+
}
174+
175+
next();
176+
}
177+
}),
178+
allowBlockingCompletions: true,
179+
completer: opts.completer,
180+
prompt,
181+
useColors: false,
182+
preview: opts.preview,
183+
terminal: true
184+
}, function(err, repl) {
185+
if (err) {
186+
console.error(`Failed test # ${numtests - tests.length}`);
187+
throw err;
188+
}
189+
190+
repl.once('close', () => {
191+
if (opts.clean)
192+
cleanupTmpFile();
193+
194+
if (opts.checkTotal) {
195+
assert.deepStrictEqual(lastChunks, expected);
196+
} else if (expected.length !== i) {
197+
console.error(tests[numtests - tests.length - 1]);
198+
throw new Error(`Failed test # ${numtests - tests.length}`);
199+
}
200+
201+
setImmediate(runTestWrap, true);
202+
});
203+
204+
if (opts.columns) {
205+
Object.defineProperty(repl, 'columns', {
206+
value: opts.columns,
207+
enumerable: true
208+
});
209+
}
210+
repl.input.run(opts.test);
211+
});
212+
}
213+
214+
// run the tests
215+
runTest();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.