Skip to content

fix: clamp a rowSpan that extends past the last row instead of crashing#359

Open
spokodev wants to merge 1 commit into
cli-table:masterfrom
spokodev:fix-rowspan-overflow-crash
Open

fix: clamp a rowSpan that extends past the last row instead of crashing#359
spokodev wants to merge 1 commit into
cli-table:masterfrom
spokodev:fix-rowspan-overflow-crash

Conversation

@spokodev

Copy link
Copy Markdown

Problem

A cell whose rowSpan reaches beyond the rows below it crashes rendering with an opaque internal error:

const Table = require('cli-table3');
const table = new Table();
table.push([{ content: 'aaaa bbbb', rowSpan: 2 }]); // only one row exists
table.toString();
// TypeError: Cannot read properties of undefined (reading 'length')
//   at insertCell (src/layout-manager.js:121)
//   at addRowSpanCells (src/layout-manager.js:98)

This happens for any rowSpan: N cell with fewer than N rows beneath it, regardless of options.

Cause

addRowSpanCells indexes the target row without checking it exists:

for (let i = 1; i < cell.rowSpan; i++) {
  ...
  insertCell(rowSpanCell, table[rowIndex + i]); // undefined when the span runs past the last row
}

insertCell then reads .length off undefined. The symmetric addColSpanCells mutates the current row in place, so it never indexes a missing row, which is why colSpan past the table width already degrades gracefully while rowSpan crashes.

Fix

Break out of the loop when the target row does not exist, clamping the rowSpan to the table height. This mirrors how colSpan past the width behaves and matches the existing maxHeight = table.length invariant, so out-of-bounds spans render a normal table instead of throwing. In-bounds rendering is byte-for-byte unchanged.

Tests

Added test/issues/rowspan-overflow-test.js asserting that an over-running rowSpan renders instead of throwing. It fails on master (the TypeError) and passes with the fix. Full suite is unchanged (the one pre-existing test custom chars failure is an unrelated no-TTY @colors/colors artifact that also fails on master).

A cell whose `rowSpan` reaches beyond the number of rows below it crashed
rendering with an opaque `TypeError: Cannot read properties of undefined
(reading 'length')`:

    const table = new Table();
    table.push([{ content: 'aaaa bbbb', rowSpan: 2 }]);
    table.toString(); // throws

`addRowSpanCells` indexed `table[rowIndex + i]` without checking it exists,
then `insertCell` read `.length` off `undefined`. A `colSpan` past the
table width already degrades gracefully, and `maxHeight` already caps the
grid at `table.length`, so clamp the rowSpan the same way instead of
throwing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant