From 9dc21a946f725e5f8c12338c89f99521d37fe0c1 Mon Sep 17 00:00:00 2001 From: Zohaib Arif Date: Thu, 29 Jan 2026 14:37:06 +0500 Subject: [PATCH 1/2] readline: initialize input before history manager --- lib/internal/readline/interface.js | 12 ++++++++---- .../test-readline-history-init-order.js | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-readline-history-init-order.js diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index 08f7aaa9e3e7e8..859ce3ebc398fd 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -215,7 +215,6 @@ function InterfaceConstructor(input, output, completer, terminal) { input.removeHistoryDuplicates = removeHistoryDuplicates; } - this.setupHistoryManager(input); if (completer !== undefined && typeof completer !== 'function') { throw new ERR_INVALID_ARG_VALUE('completer', completer); @@ -234,6 +233,7 @@ function InterfaceConstructor(input, output, completer, terminal) { this[kSubstringSearch] = null; this.output = output; this.input = input; + this.setupHistoryManager(input); this[kUndoStack] = []; this[kRedoStack] = []; this[kPreviousCursorCols] = -1; @@ -384,9 +384,13 @@ class Interface extends InterfaceConstructor { setupHistoryManager(options) { this.historyManager = new ReplHistory(this, options); - if (options.onHistoryFileLoaded) { - this.historyManager.initialize(options.onHistoryFileLoaded); - } + // Only initialize REPL history when createInterface() + // was called with an options object (options.input). + // This prevents accidental REPL history init when + // createInterface(input) is used. + if (options?.input && typeof options.onHistoryFileLoaded === 'function') { + this.historyManager.initialize(options.onHistoryFileLoaded); + } ObjectDefineProperty(this, 'history', { __proto__: null, configurable: true, enumerable: true, diff --git a/test/parallel/test-readline-history-init-order.js b/test/parallel/test-readline-history-init-order.js new file mode 100644 index 00000000000000..41bcd70b49b26c --- /dev/null +++ b/test/parallel/test-readline-history-init-order.js @@ -0,0 +1,18 @@ +'use strict'; + +const assert = require('assert'); +const readline = require('readline'); +const { PassThrough } = require('stream'); + +assert.doesNotThrow(() => { + const input = new PassThrough(); + + // This simulates the condition that previously caused + // accidental REPL history initialization. + input.onHistoryFileLoaded = () => {}; + + readline.createInterface({ + input, + output: new PassThrough() + }); +}); From 45e109687ede04daf17df73740f17949e911b6b1 Mon Sep 17 00:00:00 2001 From: Zohaib Arif Date: Thu, 12 Feb 2026 01:01:33 +0500 Subject: [PATCH 2/2] 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 --- lib/internal/readline/interface.js | 25 +++++++---- .../test-readline-history-init-order.js | 42 ++++++++++++++++--- test/parallel/test-repl-persistent-history.js | 4 +- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index 859ce3ebc398fd..9a6f75258ef60b 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -172,9 +172,11 @@ function InterfaceConstructor(input, output, completer, terminal) { let crlfDelay; let prompt = '> '; let signal; + let inputOptions; if (input?.input) { // An options object was given + inputOptions = input; output = input.output; completer = input.completer; terminal = input.terminal; @@ -233,7 +235,7 @@ function InterfaceConstructor(input, output, completer, terminal) { this[kSubstringSearch] = null; this.output = output; this.input = input; - this.setupHistoryManager(input); + this.setupHistoryManager(inputOptions || input); this[kUndoStack] = []; this[kRedoStack] = []; this[kPreviousCursorCols] = -1; @@ -384,13 +386,20 @@ class Interface extends InterfaceConstructor { setupHistoryManager(options) { this.historyManager = new ReplHistory(this, options); - // Only initialize REPL history when createInterface() - // was called with an options object (options.input). - // This prevents accidental REPL history init when - // createInterface(input) is used. - if (options?.input && typeof options.onHistoryFileLoaded === 'function') { - this.historyManager.initialize(options.onHistoryFileLoaded); - } + // Only initialize REPL history when called from REPL.setupHistory(), + // not from the readline constructor. + // Constructor passes: stream OR { input: stream, output: stream, ... } + // setupHistory passes: { filePath: ..., size: ..., onHistoryFileLoaded: ... } + // Detect constructor calls by checking for stream.on() or options.input + if (options && typeof options === 'object') { + const isStream = typeof options.on === 'function'; + const hasInputProperty = options.input !== undefined; + const isFromConstructor = isStream || hasInputProperty; + + if (!isFromConstructor && typeof options.onHistoryFileLoaded === 'function') { + this.historyManager.initialize(options.onHistoryFileLoaded); + } + } ObjectDefineProperty(this, 'history', { __proto__: null, configurable: true, enumerable: true, diff --git a/test/parallel/test-readline-history-init-order.js b/test/parallel/test-readline-history-init-order.js index 41bcd70b49b26c..6a2b44b94ce1fb 100644 --- a/test/parallel/test-readline-history-init-order.js +++ b/test/parallel/test-readline-history-init-order.js @@ -1,18 +1,48 @@ 'use strict'; -const assert = require('assert'); +const common = require('../common'); const readline = require('readline'); const { PassThrough } = require('stream'); -assert.doesNotThrow(() => { - const input = new PassThrough(); +// Regression test for https://github.com/nodejs/node/issues/61526 +// This test ensures that createInterface() doesn't crash when input +// has an onHistoryFileLoaded property (e.g., from a Proxy or jest.mock) - // This simulates the condition that previously caused - // accidental REPL history initialization. +// Test case 1: options object with onHistoryFileLoaded as function +{ + const input = new PassThrough(); input.onHistoryFileLoaded = () => {}; readline.createInterface({ input, output: new PassThrough() }); -}); +} + +// Test case 2: options object without onHistoryFileLoaded +{ + const input = new PassThrough(); + readline.createInterface({ + input, + output: new PassThrough() + }); +} + +// Test case 3: options object with onHistoryFileLoaded as non-function +{ + const input = new PassThrough(); + input.onHistoryFileLoaded = { some: 'object' }; + + readline.createInterface({ + input, + output: new PassThrough() + }); +} + +// Test case 4: direct stream with onHistoryFileLoaded (original bug scenario) +{ + const input = new PassThrough(); + input.onHistoryFileLoaded = () => {}; + + readline.createInterface(input, new PassThrough()); +} diff --git a/test/parallel/test-repl-persistent-history.js b/test/parallel/test-repl-persistent-history.js index 0807a10a08a8a6..682ffb0b3cf806 100644 --- a/test/parallel/test-repl-persistent-history.js +++ b/test/parallel/test-repl-persistent-history.js @@ -183,8 +183,10 @@ function cleanupTmpFile() { } // Copy our fixture to the tmp directory +const writeStream = fs.createWriteStream(historyPath); fs.createReadStream(historyFixturePath) - .pipe(fs.createWriteStream(historyPath)).on('unpipe', () => runTest()); + .pipe(writeStream); +writeStream.on('finish', () => runTest()); const runTestWrap = common.mustCall(runTest, numtests);