Skip to content

Commit 45e1096

Browse files
committed
readline: fix setupHistoryManager initialization order and stream handling
- Reorder initialization: assign this.input before setupHistoryManager() - Preserve original options object for proper history manager initialization - Fix REPL.setupHistory() callback invocation logic - Fix stream event handling in repl persistent history test Fixes: #61526
1 parent 9dc21a9 commit 45e1096

3 files changed

Lines changed: 56 additions & 15 deletions

File tree

lib/internal/readline/interface.js

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,11 @@ function InterfaceConstructor(input, output, completer, terminal) {
172172
let crlfDelay;
173173
let prompt = '> ';
174174
let signal;
175+
let inputOptions;
175176

176177
if (input?.input) {
177178
// An options object was given
179+
inputOptions = input;
178180
output = input.output;
179181
completer = input.completer;
180182
terminal = input.terminal;
@@ -233,7 +235,7 @@ function InterfaceConstructor(input, output, completer, terminal) {
233235
this[kSubstringSearch] = null;
234236
this.output = output;
235237
this.input = input;
236-
this.setupHistoryManager(input);
238+
this.setupHistoryManager(inputOptions || input);
237239
this[kUndoStack] = [];
238240
this[kRedoStack] = [];
239241
this[kPreviousCursorCols] = -1;
@@ -384,13 +386,20 @@ class Interface extends InterfaceConstructor {
384386
setupHistoryManager(options) {
385387
this.historyManager = new ReplHistory(this, options);
386388

387-
// Only initialize REPL history when createInterface()
388-
// was called with an options object (options.input).
389-
// This prevents accidental REPL history init when
390-
// createInterface(input) is used.
391-
if (options?.input && typeof options.onHistoryFileLoaded === 'function') {
392-
this.historyManager.initialize(options.onHistoryFileLoaded);
393-
}
389+
// Only initialize REPL history when called from REPL.setupHistory(),
390+
// not from the readline constructor.
391+
// Constructor passes: stream OR { input: stream, output: stream, ... }
392+
// setupHistory passes: { filePath: ..., size: ..., onHistoryFileLoaded: ... }
393+
// Detect constructor calls by checking for stream.on() or options.input
394+
if (options && typeof options === 'object') {
395+
const isStream = typeof options.on === 'function';
396+
const hasInputProperty = options.input !== undefined;
397+
const isFromConstructor = isStream || hasInputProperty;
398+
399+
if (!isFromConstructor && typeof options.onHistoryFileLoaded === 'function') {
400+
this.historyManager.initialize(options.onHistoryFileLoaded);
401+
}
402+
}
394403

395404
ObjectDefineProperty(this, 'history', {
396405
__proto__: null, configurable: true, enumerable: true,
Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,48 @@
11
'use strict';
22

3-
const assert = require('assert');
3+
const common = require('../common');
44
const readline = require('readline');
55
const { PassThrough } = require('stream');
66

7-
assert.doesNotThrow(() => {
8-
const input = new PassThrough();
7+
// Regression test for https://github.com/nodejs/node/issues/61526
8+
// This test ensures that createInterface() doesn't crash when input
9+
// has an onHistoryFileLoaded property (e.g., from a Proxy or jest.mock)
910

10-
// This simulates the condition that previously caused
11-
// accidental REPL history initialization.
11+
// Test case 1: options object with onHistoryFileLoaded as function
12+
{
13+
const input = new PassThrough();
1214
input.onHistoryFileLoaded = () => {};
1315

1416
readline.createInterface({
1517
input,
1618
output: new PassThrough()
1719
});
18-
});
20+
}
21+
22+
// Test case 2: options object without onHistoryFileLoaded
23+
{
24+
const input = new PassThrough();
25+
readline.createInterface({
26+
input,
27+
output: new PassThrough()
28+
});
29+
}
30+
31+
// Test case 3: options object with onHistoryFileLoaded as non-function
32+
{
33+
const input = new PassThrough();
34+
input.onHistoryFileLoaded = { some: 'object' };
35+
36+
readline.createInterface({
37+
input,
38+
output: new PassThrough()
39+
});
40+
}
41+
42+
// Test case 4: direct stream with onHistoryFileLoaded (original bug scenario)
43+
{
44+
const input = new PassThrough();
45+
input.onHistoryFileLoaded = () => {};
46+
47+
readline.createInterface(input, new PassThrough());
48+
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,10 @@ function cleanupTmpFile() {
183183
}
184184

185185
// Copy our fixture to the tmp directory
186+
const writeStream = fs.createWriteStream(historyPath);
186187
fs.createReadStream(historyFixturePath)
187-
.pipe(fs.createWriteStream(historyPath)).on('unpipe', () => runTest());
188+
.pipe(writeStream);
189+
writeStream.on('finish', () => runTest());
188190

189191
const runTestWrap = common.mustCall(runTest, numtests);
190192

0 commit comments

Comments
 (0)