From e4c70ce8158646083d87410a58b7eb664f358a2a Mon Sep 17 00:00:00 2001 From: piyushrajyadav Date: Fri, 28 Aug 2026 11:46:27 +0530 Subject: [PATCH] readline: fix crash when navigating wrapped multiline history When navigating multiline history in readline/REPL with the UP/DOWN arrow keys, [kMultilineMove] used getCursorPos() (which returns the terminal screen's visual row and column index) to index into the logical \n-delimited splitLines array. When a logical line was long enough to wrap across multiple terminal columns, cursorPos.rows exceeded splitLines.length - 1, causing splitLines[cursorPos.rows] to be undefined and throwing: TypeError: Cannot read properties of undefined (reading 'length') This commit adds getLogicalCursorPos() to compute the logical row and column based on \n characters in this.line and uses it across multiline history navigation. Fixes: https://github.com/nodejs/node/issues/59431 Signed-off-by: piyushrajyadav Assisted-by: Antigravity --- lib/internal/readline/interface.js | 54 +++++++++++++++++------- test/parallel/test-readline-interface.js | 38 ++++++++++++++++- 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index bc8f30427616..f99af9e1b602 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -25,6 +25,7 @@ const { StringPrototypeCodePointAt, StringPrototypeEndsWith, StringPrototypeIncludes, + StringPrototypeLastIndexOf, StringPrototypeRepeat, StringPrototypeReplaceAll, StringPrototypeSlice, @@ -1117,21 +1118,22 @@ class Interface extends InterfaceConstructor { this[kRefreshLine](); } - [kMultilineMove](direction, splitLines, { rows, cols }) { - const curr = splitLines[rows]; + [kMultilineMove](direction, splitLines, { logicalRows, logicalCols }) { + const curr = splitLines[logicalRows]; const down = direction === 1; - const adj = splitLines[rows + direction]; - const promptLen = kMultilinePrompt.description.length; + const adj = splitLines[logicalRows + direction]; let amountToMove; - // Clamp distance to end of current + prompt + next/prev line + newline + // Clamp distance to end of current line + newline + end of next/prev line. + // Using logical (\n-based) cols here so terminal wrapping doesn't affect + // which adjacent logical line we land on. const clamp = down ? - curr.length - cols + promptLen + adj.length + 1 : - -cols + 1; - const shouldClamp = cols > adj.length + 1; + curr.length - logicalCols + adj.length + 1 : + -logicalCols - 1; + const shouldClamp = logicalCols > adj.length; if (shouldClamp) { if (this[kPreviousCursorCols] === -1) { - this[kPreviousCursorCols] = cols; + this[kPreviousCursorCols] = logicalCols; } amountToMove = clamp; } else { @@ -1142,7 +1144,7 @@ class Interface extends InterfaceConstructor { } if (this[kPreviousCursorCols] !== -1) { if (this[kPreviousCursorCols] <= adj.length) { - amountToMove += this[kPreviousCursorCols] - cols; + amountToMove += this[kPreviousCursorCols] - logicalCols; this[kPreviousCursorCols] = -1; } else { amountToMove = clamp; @@ -1154,10 +1156,10 @@ class Interface extends InterfaceConstructor { } [kMoveDownOrHistoryNext]() { - const cursorPos = this.getCursorPos(); + const logicalCursorPos = this.getLogicalCursorPos(); const splitLines = StringPrototypeSplit(this.line, '\n'); - if (this[kIsMultiline] && cursorPos.rows < splitLines.length - 1) { - this[kMultilineMove](1, splitLines, cursorPos); + if (this[kIsMultiline] && logicalCursorPos.logicalRows < splitLines.length - 1) { + this[kMultilineMove](1, splitLines, logicalCursorPos); return; } this[kPreviousCursorCols] = -1; @@ -1181,10 +1183,10 @@ class Interface extends InterfaceConstructor { } [kMoveUpOrHistoryPrev]() { - const cursorPos = this.getCursorPos(); - if (this[kIsMultiline] && cursorPos.rows > 0) { + const logicalCursorPos = this.getLogicalCursorPos(); + if (this[kIsMultiline] && logicalCursorPos.logicalRows > 0) { const splitLines = StringPrototypeSplit(this.line, '\n'); - this[kMultilineMove](-1, splitLines, cursorPos); + this[kMultilineMove](-1, splitLines, logicalCursorPos); return; } this[kPreviousCursorCols] = -1; @@ -1252,6 +1254,26 @@ class Interface extends InterfaceConstructor { return this[kGetDisplayPos](strBeforeCursor); } + /** + * Returns the cursor position within the input string in terms of logical + * lines delimited by '\n', independent of terminal column width or wrapping. + * This is used for multiline history navigation so that terminal-wrapped + * lines do not cause an out-of-bounds access on the logical splitLines array. + * @returns {{ + * logicalRows: number; + * logicalCols: number; + * }} + */ + getLogicalCursorPos() { + const strBeforeCursor = StringPrototypeSlice(this.line, 0, this.cursor); + // Number of '\n' chars before the cursor == the logical row index. + const logicalRows = StringPrototypeSplit(strBeforeCursor, '\n').length - 1; + // Characters since the last '\n' (or start of string) == logical column. + const lastNewline = StringPrototypeLastIndexOf(this.line, '\n', this.cursor - 1); + const logicalCols = this.cursor - (lastNewline + 1); + return { logicalRows, logicalCols }; + } + // This function moves cursor dx places to the right // (-dx for left) and refreshes the line if it is needed. [kMoveCursor](dx) { diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 2fd4646c314c..87eaeed497b4 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -35,7 +35,7 @@ const { stripVTControlCharacters } = require('internal/util/inspect'); const { EventEmitter, listenerCount } = require('events'); -const { Writable, Readable } = require('stream'); +const { Writable, Readable, PassThrough } = require('stream'); class FakeInput extends EventEmitter { resume() {} @@ -1483,3 +1483,39 @@ for (let i = 0; i < 12; i++) { code: 'ERR_INVALID_ARG_TYPE' }); } + +// Regression test for https://github.com/nodejs/node/issues/59431 +// [kMultilineMove] used getCursorPos() (visual terminal rows) to index into +// splitLines (logical '\n'-delimited rows). When one logical line wraps across +// multiple terminal-columns, visual rows > logical rows, making +// splitLines[visualRows] undefined and throwing: +// TypeError: Cannot read properties of undefined (reading 'length') +{ + // Simulate the crash scenario: + // 1. Submit a very long single-line entry (no '\n') so it wraps across + // many visual rows in an 80-column terminal. + // 2. Press UP to recall it from history. + // 3. Press UP again — this is where the crash occurred before the fix. + const input = new PassThrough(); + const output = new PassThrough(); + const rl = readline.createInterface({ + input, + output, + terminal: true, + }); + rl.columns = 80; // force narrow terminal so the long line wraps many times + + const longEntry = 'x'.repeat(400); // one logical line, ~5 visual rows + rl.write(longEntry); + rl.write(null, { name: 'return' }); // commit to history + + // First UP: recalls the long entry (enters multiline-history mode). + assert.doesNotThrow(() => rl.write(null, { name: 'up' })); + + // Second UP from inside the recalled entry: previously threw + // TypeError: Cannot read properties of undefined (reading 'length') + // at [_multilineMove] (node:internal/readline/interface:...) + assert.doesNotThrow(() => rl.write(null, { name: 'up' })); + + rl.close(); +}