Skip to content

Commit

Permalink
repl: fix .load infinite loop caused by shared use of lineEnding RegExp
Browse files Browse the repository at this point in the history
Since the lineEnding Regular Expression is declared on the module scope,
recursive invocations of its `[kTtyWrite]` method share one instance of
this Regular Expression.
Since the state of a RegExp is managed by instance,
alternately calling RegExpPrototypeExec with the same RegExp on
different strings can lead to the state changing unexpectedly.
This is the root cause of this infinite loop bug when calling .load on
javascript files of certain shapes.

PR-URL: #46742
Fixes: #46731
Reviewed-By: Kohei Ueno <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
  • Loading branch information
Theo-Steiner authored and danielleadams committed Apr 11, 2023
1 parent 4d0faf4 commit 09739a2
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 9 deletions.
22 changes: 13 additions & 9 deletions lib/internal/readline/interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -1329,18 +1329,22 @@ class Interface extends InterfaceConstructor {
// falls through
default:
if (typeof s === 'string' && s) {
// Erase state of previous searches.
lineEnding.lastIndex = 0;
let nextMatch = RegExpPrototypeExec(lineEnding, s);
if (nextMatch !== null) {
this[kInsertString](StringPrototypeSlice(s, 0, nextMatch.index));
let { lastIndex } = lineEnding;
while ((nextMatch = RegExpPrototypeExec(lineEnding, s)) !== null) {
this[kLine]();
// If no line endings are found, just insert the string as is.
if (nextMatch === null) {
this[kInsertString](s);
} else {
// Keep track of the end of the last match.
let lastIndex = 0;
do {
this[kInsertString](StringPrototypeSlice(s, lastIndex, nextMatch.index));
({ lastIndex } = lineEnding);
}
if (lastIndex === s.length) this[kLine]();
} else {
this[kInsertString](s);
this[kLine]();
// Restore lastIndex as the call to kLine could have mutated it.
lineEnding.lastIndex = lastIndex;
} while ((nextMatch = RegExpPrototypeExec(lineEnding, s)) !== null);
}
}
}
Expand Down
33 changes: 33 additions & 0 deletions test/parallel/test-readline-interface-recursive-writes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict';
const common = require('../common');
const ArrayStream = require('../common/arraystream');
const assert = require('assert');

common.skipIfDumbTerminal();

const readline = require('readline');
const rli = new readline.Interface({
terminal: true,
input: new ArrayStream(),
});

let recursionDepth = 0;

// Minimal reproduction for #46731
const testInput = ' \n}\n';
const numberOfExpectedLines = testInput.match(/\n/g).length;

rli.on('line', () => {
// Abort in case of infinite loop
if (recursionDepth > numberOfExpectedLines) {
return;
}
recursionDepth++;
// Write something recursively to readline
rli.write('foo');
});


rli.write(testInput);

assert.strictEqual(recursionDepth, numberOfExpectedLines);

0 comments on commit 09739a2

Please sign in to comment.