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(); +}