mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-06-15 08:03:56 +00:00
fix(terminal): avoid integer overflow in selectPrev with no active matches (#12936)
Also found when test searching. Run Ghostty debug on macOS and follow these steps: 1. Open Ghostty, `cat src/Surface.zig` and start search `self.startClipboardRequest`. 2. Click up button(Press enter) 6 times and click down button (Press shift+enter) 6 times. 3. You should see a panic crash. ### AI Disclosure Claude implemented the fix and the unit test. I reviewed it and tested it myself.
This commit is contained in:
@@ -798,7 +798,7 @@ pub const ScreenSearch = struct {
|
||||
|
||||
const active_len = self.active_results.items.len;
|
||||
const history_len = self.history_results.items.len;
|
||||
const next_idx = if (prev.idx != 0) prev.idx - 1 else active_len - 1 + history_len;
|
||||
const next_idx = if (prev.idx != 0) prev.idx - 1 else active_len + history_len - 1;
|
||||
|
||||
const hl: FlattenedHighlight = if (next_idx < active_len)
|
||||
self.active_results.items[active_len - 1 - next_idx]
|
||||
@@ -1380,6 +1380,72 @@ test "select prev with history" {
|
||||
}
|
||||
}
|
||||
|
||||
test "select prev wraps when all matches are in history" {
|
||||
// Regression test: when every match is in scrollback (the active area
|
||||
// has none, so active_len == 0), selecting prev from index 0 must wrap
|
||||
// to the last result without underflowing `active_len - 1`.
|
||||
const alloc = testing.allocator;
|
||||
var t: Terminal = try .init(alloc, .{
|
||||
.cols = 10,
|
||||
.rows = 2,
|
||||
.max_scrollback = std.math.maxInt(usize),
|
||||
});
|
||||
defer t.deinit(alloc);
|
||||
const list: *PageList = &t.screens.active.pages;
|
||||
|
||||
var s = t.vtStream();
|
||||
defer s.deinit();
|
||||
|
||||
// Put the only match in scrollback, then scroll the active area to all
|
||||
// blank lines so it contains no match (active_len == 0, history_len == 1).
|
||||
s.nextSlice("Fizz\r\n");
|
||||
while (list.totalPages() < 3) s.nextSlice("\r\n");
|
||||
for (0..list.rows) |_| s.nextSlice("\r\n");
|
||||
|
||||
var search: ScreenSearch = try .init(alloc, t.screens.active, "Fizz");
|
||||
defer search.deinit();
|
||||
try search.searchAll();
|
||||
try testing.expectEqual(0, search.active_results.items.len);
|
||||
|
||||
// Select the first match (idx 0), then wrap backwards. This must not
|
||||
// panic and must keep a valid selection.
|
||||
_ = try search.select(.next);
|
||||
_ = try search.select(.prev);
|
||||
try testing.expect(search.selectedMatch() != null);
|
||||
}
|
||||
|
||||
test "select after all matches disappear drops the selection" {
|
||||
// The wrap arithmetic in selectPrev (active_len + history_len - 1) would
|
||||
// underflow if a selection were ever live while both result lists are
|
||||
// empty. This guards the invariant that makes that unreachable: when a
|
||||
// reload/prune empties the results, the selection is dropped, so the next
|
||||
// select() hits the "no matches" guard instead of the wrap arithmetic.
|
||||
const alloc = testing.allocator;
|
||||
var t: Terminal = try .init(alloc, .{ .cols = 10, .rows = 2 });
|
||||
defer t.deinit(alloc);
|
||||
|
||||
var s = t.vtStream();
|
||||
defer s.deinit();
|
||||
s.nextSlice("Fizz");
|
||||
|
||||
var search: ScreenSearch = try .init(alloc, t.screens.active, "Fizz");
|
||||
defer search.deinit();
|
||||
try search.searchAll();
|
||||
try testing.expectEqual(1, search.active_results.items.len);
|
||||
|
||||
// Take a selection, then overwrite the only match so a reload finds none
|
||||
// (active and history both empty).
|
||||
_ = try search.select(.next);
|
||||
try testing.expect(search.selectedMatch() != null);
|
||||
s.nextSlice("\x1b[1;1H ");
|
||||
|
||||
// Must not underflow; the selection is dropped and nothing is selected.
|
||||
_ = try search.select(.prev);
|
||||
try testing.expect(search.selectedMatch() == null);
|
||||
try testing.expectEqual(0, search.active_results.items.len);
|
||||
try testing.expectEqual(0, search.history_results.items.len);
|
||||
}
|
||||
|
||||
test "screen search no scrollback has no history" {
|
||||
const alloc = testing.allocator;
|
||||
var t: Terminal = try .init(alloc, .{
|
||||
|
||||
Reference in New Issue
Block a user