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>

Author: RaisinTen

lib/internal/repl/utils.js

@@ -245,7 +245,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
 }
 
 // Result and the text that was completed.
-const[rawCompletions,completeOn]=data;
+const{0: rawCompletions,1: completeOn}=data;
 
 if(!rawCompletions||rawCompletions.length===0){
 return;

lib/internal/util/inspect.js

@@ -309,7 +309,8 @@ function inspect(value, opts) {
 ctx.showHidden=opts;
 }elseif(opts){
 constoptKeys=ObjectKeys(opts);
-for(constkeyofoptKeys){
+for(leti=0;i<optKeys.length;++i){
+constkey=optKeys[i];
 // TODO(BridgeAR): Find a solution what to do about stylize. Either make
 // this function public or add a new API with a similar or better
 // functionality.
@@ -1869,18 +1870,18 @@ function tryStringify(arg) {
 }
 
 functionformat(...args){
-returnformatWithOptionsInternal(undefined,...args);
+returnformatWithOptionsInternal(undefined,args);
 }
 
 functionformatWithOptions(inspectOptions, ...args){
 if(typeofinspectOptions!=='object'||inspectOptions===null){
 thrownewERR_INVALID_ARG_TYPE(
 'inspectOptions','object',inspectOptions);
 }
-returnformatWithOptionsInternal(inspectOptions,...args);
+returnformatWithOptionsInternal(inspectOptions,args);
 }
 
-functionformatWithOptionsInternal(inspectOptions,...args){
+functionformatWithOptionsInternal(inspectOptions,args){
 constfirst=args[0];
 leta=0;
 letstr='';

lib/repl.js

@@ -46,6 +46,7 @@ const {
  ArrayPrototypeConcat,
  ArrayPrototypeFilter,
  ArrayPrototypeFindIndex,
+ ArrayPrototypeForEach,
  ArrayPrototypeIncludes,
  ArrayPrototypeJoin,
  ArrayPrototypeMap,
@@ -663,7 +664,7 @@ function REPLServer(prompt,
 letmatched=false;
 
 errStack='';
-for(constlineoflines){
+ArrayPrototypeForEach(lines,(line)=>{
 if(!matched&&
 RegExpPrototypeTest(/^\[?([A-Z][a-z0-9_]*)*Error/,line)){
 errStack+=writer.options.breakLength>=line.length ?
@@ -673,7 +674,7 @@ function REPLServer(prompt,
 }else{
 errStack+=line;
 }
-}
+});
 if(!matched){
 constln=lines.length===1 ? ' ' : ':\n';
 errStack=`Uncaught${ln}${errStack}`;
@@ -754,9 +755,7 @@ function REPLServer(prompt,
 constprioritizedSigintQueue=newSafeSet();
 self.on('SIGINT',functiononSigInt(){
 if(prioritizedSigintQueue.size>0){
-for(consttaskofprioritizedSigintQueue){
-task();
-}
+ArrayPrototypeForEach(prioritizedSigintQueue,(task)=>task());
 return;
 }
 
@@ -1010,13 +1009,13 @@ REPLServer.prototype.createContext = function() {
 },()=>{
 context=vm.createContext();
 });
-for(constnameofObjectGetOwnPropertyNames(global)){
+ArrayPrototypeForEach(ObjectGetOwnPropertyNames(global),(name)=>{
 // Only set properties that do not already exist as a global builtin.
 if(!globalBuiltins.has(name)){
 ObjectDefineProperty(context,name,
 ObjectGetOwnPropertyDescriptor(global,name));
 }
-}
+});
 context.global=context;
 const_console=newConsole(this.output);
 ObjectDefineProperty(context,'console',{
@@ -1231,7 +1230,7 @@ function complete(line, callback) {
 paths=ArrayPrototypeConcat(module.paths,CJSModule.globalPaths);
 }
 
-for(letdirofpaths){
+ArrayPrototypeForEach(paths,(dir)=>{
 dir=path.resolve(dir,subdir);
 constdirents=gracefulReaddir(dir,{withFileTypes: true})||[];
 for(constdirentofdirents){
@@ -1259,7 +1258,7 @@ function complete(line, callback) {
 }
 }
 }
-}
+});
 if(group.length){
 ArrayPrototypePush(completionGroups,group);
 }
@@ -1269,7 +1268,7 @@ function complete(line, callback) {
 }
 }elseif(RegExpPrototypeTest(fsAutoCompleteRE,line)&&
 this.allowBlockingCompletions){
-[completionGroups,completeOn]=completeFSFunctions(line);
+({0: completionGroups,1: completeOn}=completeFSFunctions(line));
 // Handle variable member lookup.
 // We support simple chained expressions like the following (no function
 // calls, etc.). That is for simplicity and also because we *eval* that
@@ -1282,7 +1281,7 @@ function complete(line, callback) {
 // foo.<|> # completions for 'foo' with filter ''
 }elseif(line.length===0||
 RegExpPrototypeTest(/\w|\.|\$/,line[line.length-1])){
-const[match]=RegExpPrototypeExec(simpleExpressionRE,line)||[''];
+const{0: match}=RegExpPrototypeExec(simpleExpressionRE,line)||[''];
 if(line.length!==0&&!match){
 completionGroupsLoaded();
 return;
@@ -1352,11 +1351,11 @@ function complete(line, callback) {
 
 if(memberGroups.length){
 expr+=chaining;
-for(constgroupofmemberGroups){
+ArrayPrototypeForEach(memberGroups,(group)=>{
 ArrayPrototypePush(completionGroups,
 ArrayPrototypeMap(group,
 (member)=>`${expr}${member}`));
-}
+});
 if(filter){
 filter=`${expr}${filter}`;
 }
@@ -1375,37 +1374,38 @@ function complete(line, callback) {
 // Filter, sort (within each group), uniq and merge the completion groups.
 if(completionGroups.length&&filter){
 constnewCompletionGroups=[];
-for(constgroupofcompletionGroups){
+ArrayPrototypeForEach(completionGroups,(group)=>{
 constfilteredGroup=ArrayPrototypeFilter(
 group,
 (str)=>StringPrototypeStartsWith(str,filter)
 );
 if(filteredGroup.length){
 ArrayPrototypePush(newCompletionGroups,filteredGroup);
 }
-}
+});
 completionGroups=newCompletionGroups;
 }
 
 constcompletions=[];
 // Unique completions across all groups.
-constuniqueSet=newSafeSet(['']);
+constuniqueSet=newSafeSet();
+uniqueSet.add('');
 // Completion group 0 is the "closest" (least far up the inheritance
 // chain) so we put its completions last: to be closest in the REPL.
-for(constgroupofcompletionGroups){
+ArrayPrototypeForEach(completionGroups,(group)=>{
 ArrayPrototypeSort(group,(a,b)=>(b>a ? 1 : -1));
 constsetSize=uniqueSet.size;
-for(constentryofgroup){
+ArrayPrototypeForEach(group,(entry)=>{
 if(!uniqueSet.has(entry)){
 ArrayPrototypeUnshift(completions,entry);
 uniqueSet.add(entry);
 }
-}
+});
 // Add a separator between groups.
 if(uniqueSet.size!==setSize){
 ArrayPrototypeUnshift(completions,'');
 }
-}
+});
 
 // Remove obsolete group entry, if present.
 if(completions[0]===''){
@@ -1569,14 +1569,13 @@ function defineDefaultCommands(repl) {
 constlongestNameLength=MathMax(
  ...ArrayPrototypeMap(names,(name)=>name.length)
 );
-for(letn=0;n<names.length;n++){
-constname=names[n];
+ArrayPrototypeForEach(names,(name)=>{
 constcmd=this.commands[name];
 constspaces=
 StringPrototypeRepeat(' ',longestNameLength-name.length+3);
 constline=`.${name}${cmd.help ? spaces+cmd.help : ''}\n`;
 this.output.write(line);
-}
+});
 this.output.write('\nPress Ctrl+C to abort current expression, '+
 'Ctrl+D to exit the REPL\n');
 this.displayPrompt();

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

@@ -504,7 +504,56 @@ const tests = [
 prompt,
 ],
 clean: true
-}
+},
+{
+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]());';
+yieldENTER;
+yield'const {next} = ArrayIteratorPrototype;';
+yieldENTER;
+yield'const realArrayIterator = Array.prototype[Symbol.iterator];';
+yieldENTER;
+yield'delete Array.prototype[Symbol.iterator];';
+yieldENTER;
+yield'delete ArrayIteratorPrototype.next;';
+yieldENTER;
+yieldUP;
+yieldUP;
+yieldDOWN;
+yieldDOWN;
+yield'fu';
+yield'n';
+yieldRIGHT;
+yieldBACKSPACE;
+yieldLEFT;
+yieldLEFT;
+yield'A';
+yieldBACKSPACE;
+yieldGO_TO_END;
+yieldBACKSPACE;
+yieldWORD_LEFT;
+yieldWORD_RIGHT;
+yieldESCAPE;
+yieldENTER;
+yield'Array.proto';
+yieldRIGHT;
+yield'.pu';
+yieldENTER;
+yield'ArrayIteratorPrototype.next = next;';
+yieldENTER;
+yield'Array.prototype[Symbol.iterator] = realArrayIterator;';
+yieldENTER;
+})(),
+expected: [],
+clean: false
+},
 ];
 constnumtests=tests.length;
 

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

@@ -0,0 +1,68 @@
+'use strict';
+constcommon=require('../common');
+constassert=require('assert');
+const{ spawn }=require('child_process');
+
+constreplProcess=spawn(process.argv0,['--interactive'],{
+stdio: ['pipe','pipe','inherit'],
+windowsHide: true,
+});
+
+replProcess.on('error',common.mustNotCall());
+
+constreplReadyState=(asyncfunction*(){
+letready;
+constSPACE=' '.charCodeAt();
+constBRACKET='>'.charCodeAt();
+constDOT='.'.charCodeAt();
+replProcess.stdout.on('data',(data)=>{
+ready=data[data.length-1]===SPACE&&(
+data[data.length-2]===BRACKET||(
+data[data.length-2]===DOT&&
+data[data.length-3]===DOT&&
+data[data.length-4]===DOT
+));
+});
+
+constprocessCrashed=newPromise((resolve,reject)=>
+replProcess.on('exit',reject)
+);
+while(true){
+awaitPromise.race([newPromise(setImmediate),processCrashed]);
+if(ready){
+ready=false;
+yield;
+}
+}
+})();
+asyncfunctionwriteLn(data,expectedOutput){
+awaitreplReadyState.next();
+if(expectedOutput){
+replProcess.stdout.once('data',common.mustCall((data)=>
+assert.match(data.toString('utf8'),expectedOutput)
+));
+}
+awaitnewPromise((resolve,reject)=>replProcess.stdin.write(
+`${data}\n`,
+(err)=>(err ? reject(err) : resolve())
+));
+}
+
+asyncfunctionmain(){
+awaitwriteLn(
+'const ArrayIteratorPrototype ='+
+' Object.getPrototypeOf(Array.prototype[Symbol.iterator]());'
+);
+awaitwriteLn('delete Array.prototype[Symbol.iterator];');
+awaitwriteLn('delete ArrayIteratorPrototype.next;');
+
+awaitwriteLn(
+'for(const x of [3, 2, 1]);',
+/UncaughtTypeError:\[3,2,1\]isnotiterable/
+);
+awaitwriteLn('.exit');
+
+assert(!replProcess.connected);
+}
+
+main().then(common.mustCall());