From 04810273afc2c1dba73cf32ebb6d63ceb37df168 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 9 Jul 2026 21:37:26 -0700 Subject: [PATCH] terminal: wrap implicit hyperlink identifiers Implicit OSC 8 hyperlinks incremented a u32 identifier with checked arithmetic even though the cursor contract allows the sequence to wrap. Reaching the maximum identifier therefore caused a runtime safety panic before the link could be installed. Use wrapping arithmetic for both the successful increment and the error rollback. The identifier now returns to zero at the boundary, while failed allocation attempts still restore the original value. --- src/terminal/Screen.zig | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index d8ce4db9a..b47e4e1cd 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2413,13 +2413,13 @@ pub fn startHyperlink( .id = if (id_) |id| .{ .explicit = id, } else implicit: { - defer self.cursor.hyperlink_implicit_id += 1; + defer self.cursor.hyperlink_implicit_id +%= 1; break :implicit .{ .implicit = self.cursor.hyperlink_implicit_id }; }, }; errdefer switch (link.id) { .explicit => {}, - .implicit => self.cursor.hyperlink_implicit_id -= 1, + .implicit => self.cursor.hyperlink_implicit_id -%= 1, }; // Loop until we have enough page memory to add the hyperlink @@ -9996,6 +9996,43 @@ test "Screen: hyperlink start/end" { } } +test "Screen: implicit hyperlink ID wraps" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); + defer s.deinit(); + + s.cursor.hyperlink_implicit_id = std.math.maxInt(size.OffsetInt); + try s.startHyperlink("http://example.com", null); + + try testing.expectEqual(@as(size.OffsetInt, 0), s.cursor.hyperlink_implicit_id); + try testing.expectEqual( + std.math.maxInt(size.OffsetInt), + s.cursor.hyperlink.?.id.implicit, + ); + + // A failed allocation must roll the wrapped counter back to its + // original value as well. + s.endHyperlink(); + s.cursor.hyperlink_implicit_id = std.math.maxInt(size.OffsetInt); + var failing = testing.FailingAllocator.init(alloc, .{}); + failing.fail_index = failing.alloc_index; + { + const original_alloc = s.alloc; + defer s.alloc = original_alloc; + s.alloc = failing.allocator(); + try testing.expectError( + error.OutOfMemory, + s.startHyperlink("http://example.com", null), + ); + } + try testing.expectEqual( + std.math.maxInt(size.OffsetInt), + s.cursor.hyperlink_implicit_id, + ); +} + test "Screen: hyperlink reuse" { const testing = std.testing; const alloc = testing.allocator;