FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

repl: refactor to avoid unsafe array iteration · nodejs/node@d29c0a9 · GitHub

/ node Public

Commit d29c0a9

Browse files
RaisinTen
authored and
ruyadorno
committed
repl: refactor to avoid unsafe array iteration
PR-URL: #36663 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 7083d99 commit d29c0a9

5 files changed

Lines changed: 146 additions & 29 deletions

File tree

‎lib/internal/repl/utils.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
245245
}
246246

247247
// Result and the text that was completed.
248-
const [rawCompletions, completeOn] = data;
248+
const { 0: rawCompletions, 1: completeOn } = data;
249249

250250
if (!rawCompletions || rawCompletions.length === 0) {
251251
return;

‎lib/internal/util/inspect.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,8 @@ function inspect(value, opts) {
309309
ctx.showHidden = opts;
310310
} else if (opts) {
311311
const optKeys = ObjectKeys(opts);
312-
for (const key of optKeys) {
312+
for (let i = 0; i < optKeys.length; ++i) {
313+
const key = optKeys[i];
313314
// TODO(BridgeAR): Find a solution what to do about stylize. Either make
314315
// this function public or add a new API with a similar or better
315316
// functionality.
@@ -1869,18 +1870,18 @@ function tryStringify(arg) {
18691870
}
18701871

18711872
function format(...args) {
1872-
return formatWithOptionsInternal(undefined, ...args);
1873+
return formatWithOptionsInternal(undefined, args);
18731874
}
18741875

18751876
function formatWithOptions(inspectOptions, ...args) {
18761877
if (typeof inspectOptions !== 'object' || inspectOptions === null) {
18771878
throw new ERR_INVALID_ARG_TYPE(
18781879
'inspectOptions', 'object', inspectOptions);
18791880
}
1880-
return formatWithOptionsInternal(inspectOptions, ...args);
1881+
return formatWithOptionsInternal(inspectOptions, args);
18811882
}
18821883

1883-
function formatWithOptionsInternal(inspectOptions, ...args) {
1884+
function formatWithOptionsInternal(inspectOptions, args) {
18841885
const first = args[0];
18851886
let a = 0;
18861887
let str = '';

‎lib/repl.js‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const {
4646
ArrayPrototypeConcat,
4747
ArrayPrototypeFilter,
4848
ArrayPrototypeFindIndex,
49+
ArrayPrototypeForEach,
4950
ArrayPrototypeIncludes,
5051
ArrayPrototypeJoin,
5152
ArrayPrototypeMap,
@@ -663,7 +664,7 @@ function REPLServer(prompt,
663664
let matched = false;
664665

665666
errStack = '';
666-
for (const line of lines) {
667+
ArrayPrototypeForEach(lines, (line) => {
667668
if (!matched &&
668669
RegExpPrototypeTest(/^\[?([A-Z][a-z0-9_]*)*Error/, line)) {
669670
errStack += writer.options.breakLength >= line.length ?
@@ -673,7 +674,7 @@ function REPLServer(prompt,
673674
} else {
674675
errStack += line;
675676
}
676-
}
677+
});
677678
if (!matched) {
678679
const ln = lines.length === 1 ? ' ' : ':\n';
679680
errStack = `Uncaught${ln}${errStack}`;
@@ -754,9 +755,7 @@ function REPLServer(prompt,
754755
const prioritizedSigintQueue = new SafeSet();
755756
self.on('SIGINT', function onSigInt() {
756757
if (prioritizedSigintQueue.size > 0) {
757-
for (const task of prioritizedSigintQueue) {
758-
task();
759-
}
758+
ArrayPrototypeForEach(prioritizedSigintQueue, (task) => task());
760759
return;
761760
}
762761

@@ -1010,13 +1009,13 @@ REPLServer.prototype.createContext = function() {
10101009
}, () => {
10111010
context = vm.createContext();
10121011
});
1013-
for (const name of ObjectGetOwnPropertyNames(global)) {
1012+
ArrayPrototypeForEach(ObjectGetOwnPropertyNames(global), (name) => {
10141013
// Only set properties that do not already exist as a global builtin.
10151014
if (!globalBuiltins.has(name)) {
10161015
ObjectDefineProperty(context, name,
10171016
ObjectGetOwnPropertyDescriptor(global, name));
10181017
}
1019-
}
1018+
});
10201019
context.global = context;
10211020
const _console = new Console(this.output);
10221021
ObjectDefineProperty(context, 'console', {
@@ -1231,7 +1230,7 @@ function complete(line, callback) {
12311230
paths = ArrayPrototypeConcat(module.paths, CJSModule.globalPaths);
12321231
}
12331232

1234-
for (let dir of paths) {
1233+
ArrayPrototypeForEach(paths, (dir) => {
12351234
dir = path.resolve(dir, subdir);
12361235
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
12371236
for (const dirent of dirents) {
@@ -1259,7 +1258,7 @@ function complete(line, callback) {
12591258
}
12601259
}
12611260
}
1262-
}
1261+
});
12631262
if (group.length) {
12641263
ArrayPrototypePush(completionGroups, group);
12651264
}
@@ -1269,7 +1268,7 @@ function complete(line, callback) {
12691268
}
12701269
} else if (RegExpPrototypeTest(fsAutoCompleteRE, line) &&
12711270
this.allowBlockingCompletions) {
1272-
[completionGroups, completeOn] = completeFSFunctions(line);
1271+
({ 0: completionGroups, 1: completeOn } = completeFSFunctions(line));
12731272
// Handle variable member lookup.
12741273
// We support simple chained expressions like the following (no function
12751274
// calls, etc.). That is for simplicity and also because we *eval* that
@@ -1282,7 +1281,7 @@ function complete(line, callback) {
12821281
// foo.<|> # completions for 'foo' with filter ''
12831282
} else if (line.length === 0 ||
12841283
RegExpPrototypeTest(/\w|\.|\$/, line[line.length - 1])) {
1285-
const [match] = RegExpPrototypeExec(simpleExpressionRE, line) || [''];
1284+
const { 0: match } = RegExpPrototypeExec(simpleExpressionRE, line) || [''];
12861285
if (line.length !== 0 && !match) {
12871286
completionGroupsLoaded();
12881287
return;
@@ -1352,11 +1351,11 @@ function complete(line, callback) {
13521351

13531352
if (memberGroups.length) {
13541353
expr += chaining;
1355-
for (const group of memberGroups) {
1354+
ArrayPrototypeForEach(memberGroups, (group) => {
13561355
ArrayPrototypePush(completionGroups,
13571356
ArrayPrototypeMap(group,
13581357
(member) => `${expr}${member}`));
1359-
}
1358+
});
13601359
if (filter) {
13611360
filter = `${expr}${filter}`;
13621361
}
@@ -1375,37 +1374,38 @@ function complete(line, callback) {
13751374
// Filter, sort (within each group), uniq and merge the completion groups.
13761375
if (completionGroups.length && filter) {
13771376
const newCompletionGroups = [];
1378-
for (const group of completionGroups) {
1377+
ArrayPrototypeForEach(completionGroups, (group) => {
13791378
const filteredGroup = ArrayPrototypeFilter(
13801379
group,
13811380
(str) => StringPrototypeStartsWith(str, filter)
13821381
);
13831382
if (filteredGroup.length) {
13841383
ArrayPrototypePush(newCompletionGroups, filteredGroup);
13851384
}
1386-
}
1385+
});
13871386
completionGroups = newCompletionGroups;
13881387
}
13891388

13901389
const completions = [];
13911390
// Unique completions across all groups.
1392-
const uniqueSet = new SafeSet(['']);
1391+
const uniqueSet = new SafeSet();
1392+
uniqueSet.add('');
13931393
// Completion group 0 is the "closest" (least far up the inheritance
13941394
// chain) so we put its completions last: to be closest in the REPL.
1395-
for (const group of completionGroups) {
1395+
ArrayPrototypeForEach(completionGroups, (group) => {
13961396
ArrayPrototypeSort(group, (a, b) => (b > a ? 1 : -1));
13971397
const setSize = uniqueSet.size;
1398-
for (const entry of group) {
1398+
ArrayPrototypeForEach(group, (entry) => {
13991399
if (!uniqueSet.has(entry)) {
14001400
ArrayPrototypeUnshift(completions, entry);
14011401
uniqueSet.add(entry);
14021402
}
1403-
}
1403+
});
14041404
// Add a separator between groups.
14051405
if (uniqueSet.size !== setSize) {
14061406
ArrayPrototypeUnshift(completions, '');
14071407
}
1408-
}
1408+
});
14091409

14101410
// Remove obsolete group entry, if present.
14111411
if (completions[0] === '') {
@@ -1569,14 +1569,13 @@ function defineDefaultCommands(repl) {
15691569
const longestNameLength = MathMax(
15701570
...ArrayPrototypeMap(names, (name) => name.length)
15711571
);
1572-
for (let n = 0; n < names.length; n++) {
1573-
const name = names[n];
1572+
ArrayPrototypeForEach(names, (name) => {
15741573
const cmd = this.commands[name];
15751574
const spaces =
15761575
StringPrototypeRepeat(' ', longestNameLength - name.length + 3);
15771576
const line = `.${name}${cmd.help ? spaces + cmd.help : ''}\n`;
15781577
this.output.write(line);
1579-
}
1578+
});
15801579
this.output.write('\nPress Ctrl+C to abort current expression, ' +
15811580
'Ctrl+D to exit the REPL\n');
15821581
this.displayPrompt();

‎test/parallel/test-repl-history-navigation.js‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,56 @@ const tests = [
504504
prompt,
505505
],
506506
clean: true
507-
}
507+
},
508+
{
509+
env: { NODE_REPL_HISTORY: defaultHistoryPath },
510+
test: (function*() {
511+
// Deleting Array iterator should not break history feature.
512+
//
513+
// Using a generator function instead of an object to allow the test to
514+
// keep iterating even when Array.prototype[Symbol.iterator] has been
515+
// deleted.
516+
yield 'const ArrayIteratorPrototype =';
517+
yield ' Object.getPrototypeOf(Array.prototype[Symbol.iterator]());';
518+
yield ENTER;
519+
yield 'const {next} = ArrayIteratorPrototype;';
520+
yield ENTER;
521+
yield 'const realArrayIterator = Array.prototype[Symbol.iterator];';
522+
yield ENTER;
523+
yield 'delete Array.prototype[Symbol.iterator];';
524+
yield ENTER;
525+
yield 'delete ArrayIteratorPrototype.next;';
526+
yield ENTER;
527+
yield UP;
528+
yield UP;
529+
yield DOWN;
530+
yield DOWN;
531+
yield 'fu';
532+
yield 'n';
533+
yield RIGHT;
534+
yield BACKSPACE;
535+
yield LEFT;
536+
yield LEFT;
537+
yield 'A';
538+
yield BACKSPACE;
539+
yield GO_TO_END;
540+
yield BACKSPACE;
541+
yield WORD_LEFT;
542+
yield WORD_RIGHT;
543+
yield ESCAPE;
544+
yield ENTER;
545+
yield 'Array.proto';
546+
yield RIGHT;
547+
yield '.pu';
548+
yield ENTER;
549+
yield 'ArrayIteratorPrototype.next = next;';
550+
yield ENTER;
551+
yield 'Array.prototype[Symbol.iterator] = realArrayIterator;';
552+
yield ENTER;
553+
})(),
554+
expected: [],
555+
clean: false
556+
},
508557
];
509558
const numtests = tests.length;
510559

‎test/parallel/test-repl-unsafe-array-iteration.js‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { spawn } = require('child_process');
5+
6+
const replProcess = spawn(process.argv0, ['--interactive'], {
7+
stdio: ['pipe', 'pipe', 'inherit'],
8+
windowsHide: true,
9+
});
10+
11+
replProcess.on('error', common.mustNotCall());
12+
13+
const replReadyState = (async function* () {
14+
let ready;
15+
const SPACE = ' '.charCodeAt();
16+
const BRACKET = '>'.charCodeAt();
17+
const DOT = '.'.charCodeAt();
18+
replProcess.stdout.on('data', (data) => {
19+
ready = data[data.length - 1] === SPACE && (
20+
data[data.length - 2] === BRACKET || (
21+
data[data.length - 2] === DOT &&
22+
data[data.length - 3] === DOT &&
23+
data[data.length - 4] === DOT
24+
));
25+
});
26+
27+
const processCrashed = new Promise((resolve, reject) =>
28+
replProcess.on('exit', reject)
29+
);
30+
while (true) {
31+
await Promise.race([new Promise(setImmediate), processCrashed]);
32+
if (ready) {
33+
ready = false;
34+
yield;
35+
}
36+
}
37+
})();
38+
async function writeLn(data, expectedOutput) {
39+
await replReadyState.next();
40+
if (expectedOutput) {
41+
replProcess.stdout.once('data', common.mustCall((data) =>
42+
assert.match(data.toString('utf8'), expectedOutput)
43+
));
44+
}
45+
await new Promise((resolve, reject) => replProcess.stdin.write(
46+
`${data}\n`,
47+
(err) => (err ? reject(err) : resolve())
48+
));
49+
}
50+
51+
async function main() {
52+
await writeLn(
53+
'const ArrayIteratorPrototype =' +
54+
' Object.getPrototypeOf(Array.prototype[Symbol.iterator]());'
55+
);
56+
await writeLn('delete Array.prototype[Symbol.iterator];');
57+
await writeLn('delete ArrayIteratorPrototype.next;');
58+
59+
await writeLn(
60+
'for(const x of [3, 2, 1]);',
61+
/Uncaught TypeError: \[3,2,1\] is not iterable/
62+
);
63+
await writeLn('.exit');
64+
65+
assert(!replProcess.connected);
66+
}
67+
68+
main().then(common.mustCall());

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL