diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java b/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java index 790ea82c..8e969dd2 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/Terminal.java @@ -275,9 +275,26 @@ public void setClampedCursorPos(final int x, final int y) { } } + /** + * Move the cursor by a relative delta, clamping the delta to the screen extent before the add. + * CSI argument parsing saturates at {@link Integer#MAX_VALUE}, so {@code terminal.x + dx} would + * overflow to a negative int and {@link #setClampedCursorPos} would then clamp that wrapped value + * to 0 (the near edge) instead of the far edge. Bounding the delta first keeps the sum in range; + * {@code setClampedCursorPos} still applies the screen and scroll-region clamp to the result. + * Negative deltas (up/left) are bounded symmetrically. + */ + public void moveCursorBy(final int dx, final int dy) { + setClampedCursorPos(x + Math.clamp(dx, -width, width), + y + Math.clamp(dy, -Terminal.HEIGHT, Terminal.HEIGHT)); + } + public void setRelativeCursorPos(final int x, final int y) { if (currentPrivateModeState.DECOM) { - setCursorPos(x, Math.max(scrollFirst, Math.min(scrollFirst + y, scrollLast))); + // Clamp y into the scroll region (origin-relative under DECOM) BEFORE adding + // scrollFirst: parseArgument saturates at Integer.MAX_VALUE, so scrollFirst + y + // would overflow negative and clamp to scrollFirst (top) instead of scrollLast + // (bottom). Bounding y to the region keeps the sum in range; row 1 = scrollFirst. + setCursorPos(x, scrollFirst + Math.clamp(y, 0, scrollLast - scrollFirst)); } else { setCursorPos(x, y); } diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CNL.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CNL.java index dd5b6320..99ea991a 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CNL.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CNL.java @@ -18,6 +18,10 @@ public int[] defaultParameters(CSIState state) { @Override public void execute(final int[] args, final int argsCount, final CSIState state) { - terminal.setClampedCursorPos(0, terminal.y + args[0]); + // Down by Ps lines and to column 0 (the CSI E form of NEL). moveCursorBy preserves the + // column, so this resets it explicitly and clamps the delta inline: parseArgument saturates + // at Integer.MAX_VALUE, so terminal.y + args[0] would overflow negative to row 0 (top) + // instead of the bottom row. + terminal.setClampedCursorPos(0, terminal.y + Math.clamp(args[0], 0, Terminal.HEIGHT)); } } diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUB.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUB.java index bcaefe82..24e95bef 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUB.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUB.java @@ -14,6 +14,8 @@ public int[] defaultParameters(CSIState state) { @Override public void execute(int[] args, int argsCount, CSIState state) { - terminal.setClampedCursorPos(terminal.x - args[0], terminal.y); + // Left by Ps columns. Subtraction can't overflow, but all four cardinal directions share + // the bounded relative-move primitive (see Terminal.moveCursorBy). + terminal.moveCursorBy(-args[0], 0); } -} \ No newline at end of file +} diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUD.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUD.java index 67263df4..1a118fbb 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUD.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUD.java @@ -14,6 +14,8 @@ public int[] defaultParameters(CSIState state) { @Override public void execute(int[] args, int argsCount, CSIState state) { - terminal.setClampedCursorPos(terminal.x, terminal.y + args[0]); + // Down by Ps rows. Bounded relative move (see Terminal.moveCursorBy): a saturated CSI + // count can't overflow the int sum before setClampedCursorPos clamps. + terminal.moveCursorBy(0, args[0]); } -} \ No newline at end of file +} diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUF.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUF.java index f642ef4b..6c3d0db3 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUF.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUF.java @@ -14,6 +14,8 @@ public int[] defaultParameters(CSIState state) { @Override public void execute(int[] args, int argsCount, CSIState state) { - terminal.setClampedCursorPos(terminal.x + args[0], terminal.y); + // Right by Ps columns. Bounded relative move (see Terminal.moveCursorBy): a saturated + // CSI count can't overflow the int sum before setClampedCursorPos clamps. + terminal.moveCursorBy(args[0], 0); } -} \ No newline at end of file +} diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUU.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUU.java index b67bbccb..89e4fcbe 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUU.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CUU.java @@ -14,6 +14,8 @@ public int[] defaultParameters(CSIState state) { @Override public void execute(int[] args, int argsCount, CSIState state) { - terminal.setClampedCursorPos(terminal.x, terminal.y - args[0]); + // Up by Ps rows. Subtraction can't overflow, but all four cardinal directions share the + // bounded relative-move primitive (see Terminal.moveCursorBy). + terminal.moveCursorBy(0, -args[0]); } -} \ No newline at end of file +} diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/HPR.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/HPR.java index 7f1e4a45..79796716 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/HPR.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/HPR.java @@ -18,6 +18,8 @@ public int[] defaultParameters(CSIState state) { @Override public void execute(final int[] args, final int argsCount, final CSIState state) { - terminal.setClampedCursorPos(terminal.x + args[0], terminal.y); + // Right by Ps columns (relative). Bounded relative move (see Terminal.moveCursorBy): a + // saturated CSI count can't overflow the int sum before setClampedCursorPos clamps. + terminal.moveCursorBy(args[0], 0); } } diff --git a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/VPR.java b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/VPR.java index 9eb25d1a..b502abc9 100644 --- a/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/VPR.java +++ b/src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/VPR.java @@ -18,6 +18,8 @@ public int[] defaultParameters(CSIState state) { @Override public void execute(final int[] args, final int argsCount, final CSIState state) { - terminal.setClampedCursorPos(terminal.x, terminal.y + args[0]); + // Down by Ps rows (relative). Bounded relative move (see Terminal.moveCursorBy): a + // saturated CSI count can't overflow the int sum before setClampedCursorPos clamps. + terminal.moveCursorBy(0, args[0]); } } diff --git a/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java b/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java index 614f4ba0..4bd87707 100644 --- a/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java +++ b/src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java @@ -130,6 +130,111 @@ void vpaMovesRowOnly() { assertEquals(6, terminal.y); } + @Test + void cudMovesCursorDownAndClampsSaturatedCount() { + // parseArgument saturates at Integer.MAX_VALUE; terminal.y + args[0] must not overflow to a + // negative int (which would clamp to row 0) — a huge down-move lands on the bottom row. + write(terminal, CSI + "6;1H"); // row 6 (y=5) + write(terminal, CSI + "3B"); // down 3 -> y=8 + assertEquals(0, terminal.x); + assertEquals(8, terminal.y); + write(terminal, CSI + "2147483647B"); // saturated down -> bottom + assertEquals(Terminal.HEIGHT - 1, terminal.y, "saturated CUD lands on the bottom row, not 0"); + assertEquals(0, terminal.x); + } + + @Test + void cufMovesCursorRightAndClampsSaturatedCount() { + // Mirror of CUD: terminal.x + args[0] must not overflow to column 0; a huge right-move + // lands on the right edge. + write(terminal, CSI + "1;6H"); // col 6 (x=5) + write(terminal, CSI + "3C"); // right 3 -> x=8 + assertEquals(8, terminal.x); + assertEquals(0, terminal.y); + write(terminal, CSI + "2147483647C"); // saturated right -> right edge + assertEquals(Terminal.WIDTH - 1, terminal.x, "saturated CUF lands on the right edge, not 0"); + assertEquals(0, terminal.y); + } + + @Test + void cuuMovesCursorUpAndClampsSaturatedCount() { + // CUU subtracts (no overflow), but shares the bounded moveCursorBy primitive; a huge + // up-move lands on the top row. + write(terminal, CSI + "6;1H"); // row 6 (y=5) + write(terminal, CSI + "3A"); // up 3 -> y=2 + assertEquals(0, terminal.x); + assertEquals(2, terminal.y); + write(terminal, CSI + "2147483647A"); // saturated up -> top + assertEquals(0, terminal.y, "saturated CUU lands on the top row"); + assertEquals(0, terminal.x); + } + + @Test + void cubMovesCursorLeftAndClampsSaturatedCount() { + // CUB subtracts (no overflow), but shares the bounded moveCursorBy primitive; a huge + // left-move lands on column 0. + write(terminal, CSI + "1;6H"); // col 6 (x=5) + write(terminal, CSI + "3D"); // left 3 -> x=2 + assertEquals(2, terminal.x); + assertEquals(0, terminal.y); + write(terminal, CSI + "2147483647D"); // saturated left -> col 0 + assertEquals(0, terminal.x, "saturated CUB lands on column 0"); + assertEquals(0, terminal.y); + } + + @Test + void vprMovesCursorDownAndClampsSaturatedCount() { + // VPR (CSI Ps e) is the relative mirror of VPA; same overflow class as CUD. + write(terminal, CSI + "6;1H"); // row 6 (y=5) + write(terminal, CSI + "3e"); // down 3 -> y=8 + assertEquals(0, terminal.x); + assertEquals(8, terminal.y); + write(terminal, CSI + "2147483647e"); // saturated -> bottom + assertEquals(Terminal.HEIGHT - 1, terminal.y, "saturated VPR lands on the bottom row, not 0"); + assertEquals(0, terminal.x); + } + + @Test + void hprMovesCursorRightAndClampsSaturatedCount() { + // HPR (CSI Ps a) is the relative mirror of HPA; same overflow class as CUF. + write(terminal, CSI + "1;6H"); // col 6 (x=5) + write(terminal, CSI + "3a"); // right 3 -> x=8 + assertEquals(8, terminal.x); + assertEquals(0, terminal.y); + write(terminal, CSI + "2147483647a"); // saturated -> right edge + assertEquals(Terminal.WIDTH - 1, terminal.x, "saturated HPR lands on the right edge, not 0"); + assertEquals(0, terminal.y); + } + + @Test + void cnlMovesToNextLineAndClampsSaturatedCount() { + // CNL (CSI Ps E) moves down Ps lines and to column 0. Same overflow class as CUD on the + // row; the column reset is why it can't share moveCursorBy (which preserves the column). + write(terminal, CSI + "1;6H"); // col 6 (x=5), row 1 (y=0) + write(terminal, CSI + "3E"); // down 3, col 0 -> y=3, x=0 + assertEquals(0, terminal.x); + assertEquals(3, terminal.y); + write(terminal, CSI + "2147483647E"); // saturated -> bottom, col 0 + assertEquals(0, terminal.x); + assertEquals(Terminal.HEIGHT - 1, terminal.y, "saturated CNL lands on the bottom row, not 0"); + } + + @Test + void moveCursorBySaturatedDownInScrollRegionClampsToScrollLast() { + // moveCursorBy bounds the delta to +/- HEIGHT before the add, then setClampedCursorPos + // applies the scroll-region clamp. From inside a region [5..10] (0-indexed 4..9) at y=6, + // a saturated CUD (down) must land on scrollLast (9) — not overflow the int sum to a + // negative value that clamps to scrollFirst (4), and not escape the region to HEIGHT-1. + write(terminal, CSI + "5;10r"); // scroll region rows 5-10 (0-indexed 4..9) + assertEquals(4, terminal.scrollFirst); + assertEquals(9, terminal.scrollLast); + write(terminal, CSI + "7;1H"); // row 7 (y=6), inside the region + assertEquals(6, terminal.y); + write(terminal, CSI + "2147483647B"); // saturated down + assertEquals(9, terminal.y, "saturated CUD inside a scroll region lands on scrollLast, not scrollFirst or HEIGHT-1"); + assertEquals(0, terminal.x); + } + @Test void edClearsFromCursorToEndOfScreen() { write(terminal, SAMPLE_LINE + CSI + "3G" + CSI + "J"); @@ -227,6 +332,19 @@ void decomOriginModeClampsCursorToMargins() { assertEquals(0, terminal.y); } + @Test + void decomSaturatedRowClampsToScrollLastNotOverflow() { + // Under DECOM, CUP's row is origin-relative: absolute = scrollFirst + (row-1). With a + // saturated row, scrollFirst + (MAX_VALUE-1) overflows negative and clamps to scrollFirst + // (top) instead of scrollLast (bottom). The fix bounds the row to the region first. + write(terminal, CSI + "3;8r" + CSI + "?6h"); // scroll region rows 3-8 (0-indexed 2..7), DECOM on + assertEquals(2, terminal.scrollFirst); + assertEquals(7, terminal.scrollLast); + write(terminal, CSI + "2147483647;1H"); // CUP row MAX, col 1 -> bottom of region + assertEquals(7, terminal.y, "saturated DECOM row lands on scrollLast (bottom), not scrollFirst"); + assertEquals(0, terminal.x); + } + @Test void insertLinesShiftsContentDownWithinMargins() { write(terminal, CSI + "2;8r");