From b5290e74c42c2fc5f891eb20f08d0ac0c7c634f8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Aug 2026 07:33:41 -0700 Subject: [PATCH 1/4] terminal/snapshot: release decoded hyperlink table refs Decoded hyperlink table entries retained their insertion reference after the grid added its per-cell references. Overwriting all linked cells could therefore leave unused entries alive indefinitely. Release each accepted wire table entry after grid decoding, including duplicate values that map to one native ID. Regression coverage verifies exact cell ownership and reaping after overwrite. --- src/terminal/snapshot/page.zig | 66 +++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/src/terminal/snapshot/page.zig b/src/terminal/snapshot/page.zig index 6ffa0d696..3e1f618eb 100644 --- a/src/terminal/snapshot/page.zig +++ b/src/terminal/snapshot/page.zig @@ -430,6 +430,16 @@ fn decodePayloadBody( page.styles.release(page.memory, id); } } + + // Hyperlink insertion likewise creates one temporary reference for every + // accepted wire table entry. Release through the encoded-ID remap rather + // than the native set so duplicate values which deduplicated to the same + // native ID each surrender their own reference. + var hyperlink_it = hyperlink_remap.seen.iterator(.{}); + while (hyperlink_it.next()) |encoded_id| { + const id = hyperlink_remap.entries[encoded_id]; + if (id != 0) page.hyperlink_set.release(page.memory, id); + } } /// The fixed logical dimensions, table counts, and allocation hints at the @@ -1017,7 +1027,7 @@ test "decode accepts unordered sparse style IDs and ignores zero" { test "decode accepts unordered sparse hyperlink IDs" { const header: Header = .{ - .columns = 1, + .columns = 2, .rows = 1, .style_count = 0, .hyperlink_count = 2, @@ -1039,7 +1049,7 @@ test "decode accepts unordered sparse hyperlink IDs" { var encoded: [ Header.len + 2 * 14 + - 7 + 3 + 2 * 8 + 4 ]u8 = undefined; var writer: std.Io.Writer = .fixed(&encoded); try header.encode(&writer); @@ -1047,8 +1057,18 @@ test "decode accepts unordered sparse hyperlink IDs" { try hyperlink.encode(first, &writer); try io.writeInt(&writer, TerminalHyperlinkId, 2); try hyperlink.encode(second, &writer); - try writer.writeByte(0); // row flags - try io.writeInt(&writer, u16, 0); // cell count + try writer.writeByte(@bitCast(grid.Row{ .cell_width = .eight })); + try io.writeInt(&writer, u16, 2); // cell count + try io.writeInt(&writer, u64, @bitCast(grid.Cell{ + .content = 'A', + .hyperlink = true, + .hyperlink_id = 3, + })); + try io.writeInt(&writer, u64, @bitCast(grid.Cell{ + .content = 'B', + .hyperlink = true, + .hyperlink_id = 2, + })); try io.writeInt(&writer, u32, 0); // grapheme section var reader: std.Io.Reader = .fixed(writer.buffered()); @@ -1061,6 +1081,34 @@ test "decode accepts unordered sparse hyperlink IDs" { @as(usize, 2), decoded.hyperlink_set.count(), ); + const first_id = decoded.lookupHyperlink( + decoded.getRowAndCell(0, 0).cell, + ).?; + const second_id = decoded.lookupHyperlink( + decoded.getRowAndCell(1, 0).cell, + ).?; + try std.testing.expectEqual( + @as(u16, 1), + decoded.hyperlink_set.refCount(decoded.memory, first_id), + ); + try std.testing.expectEqual( + @as(u16, 1), + decoded.hyperlink_set.refCount(decoded.memory, second_id), + ); + try std.testing.expectEqualStrings( + "one", + decoded.hyperlink_set.get( + decoded.memory, + first_id, + ).uri.slice(decoded.memory), + ); + try std.testing.expectEqualStrings( + "two", + decoded.hyperlink_set.get( + decoded.memory, + second_id, + ).uri.slice(decoded.memory), + ); } test "decode defaults missing sparse cell references" { @@ -1518,10 +1566,20 @@ test "decode reuses duplicate hyperlinks" { try std.testing.expect(cell.hyperlink); const id = decoded.lookupHyperlink(cell).?; const entry = decoded.hyperlink_set.get(decoded.memory, id); + try std.testing.expectEqual( + @as(u16, 1), + decoded.hyperlink_set.refCount(decoded.memory, id), + ); try std.testing.expectEqualStrings( "uri", entry.uri.slice(decoded.memory), ); + + // Overwriting the sole linked cell must make the deduplicated entry dead. + // Any surviving reference belongs to the decode-time wire table, not the + // native page. + decoded.clearCells(decoded.getRow(0), 0, 1); + try std.testing.expectEqual(@as(usize, 0), decoded.hyperlink_set.count()); } test "discard consumes exactly one PAGE record and keeps digest coverage" { From f99896bf8c2438e2edbc2a5779a1f2fafa9bcc6d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Aug 2026 07:37:08 -0700 Subject: [PATCH 2/4] terminal/snapshot: preserve style reader errors Lenient style decoding previously caught every error, so PAGE and SCREEN could treat truncation or an I/O failure as an invalid semantic style and continue from a corrupted stream position. Add a nullable decoder that discards only invalid style contents while propagating reader failures. Update snapshot callers and cover both semantic fallback and structural failure behavior. --- src/terminal/snapshot/page.zig | 78 +++++++++++++++++++++++++++++++- src/terminal/snapshot/screen.zig | 5 +- src/terminal/snapshot/style.zig | 31 +++++++++++++ 3 files changed, 111 insertions(+), 3 deletions(-) diff --git a/src/terminal/snapshot/page.zig b/src/terminal/snapshot/page.zig index 3e1f618eb..97a9c932a 100644 --- a/src/terminal/snapshot/page.zig +++ b/src/terminal/snapshot/page.zig @@ -370,7 +370,7 @@ fn decodePayloadBody( // Styles for (0..header.style_count) |_| { const native_id = try io.readInt(reader, TerminalStyleId); - const value: ?TerminalStyle = style.decodeOrDiscard(reader) catch null; + const value = try style.decodeOrNull(reader); // Zero is reserved for the implicit default. For a duplicate encoded // ID, the first entry wins and this complete entry is simply ignored. @@ -1309,6 +1309,82 @@ test "decode validates dimensions" { } } +test "decode propagates a style table read failure" { + const FailOnceReader = struct { + source: std.Io.Reader, + interface: std.Io.Reader, + bytes_before_failure: usize, + failed: bool = false, + + fn init(bytes: []const u8, bytes_before_failure: usize) @This() { + return .{ + .source = .fixed(bytes), + .interface = .{ + .vtable = &.{ .stream = stream }, + .buffer = &.{}, + .seek = 0, + .end = 0, + }, + .bytes_before_failure = bytes_before_failure, + }; + } + + fn stream( + reader: *std.Io.Reader, + writer: *std.Io.Writer, + limit: std.Io.Limit, + ) std.Io.Reader.StreamError!usize { + const self: *@This() = @fieldParentPtr("interface", reader); + if (self.bytes_before_failure == 0 and !self.failed) { + self.failed = true; + return error.ReadFailed; + } + + const read_limit = if (self.failed) + limit + else + limit.min(.limited(self.bytes_before_failure)); + const n = try self.source.stream(writer, read_limit); + if (!self.failed) self.bytes_before_failure -= n; + return n; + } + }; + + const header: Header = .{ + .columns = 1, + .rows = 1, + .style_count = 1, + .hyperlink_count = 0, + .style_capacity = 8, + .hyperlink_capacity_bytes = 0, + .grapheme_capacity_bytes = 0, + .string_capacity_bytes = 0, + }; + + // If the style read error is swallowed, its sixteen zero bytes are then + // misread as a valid empty grid and the unframed payload appears to decode. + var encoded: [Header.len + @sizeOf(TerminalStyleId) + style.len]u8 = + @splat(0); + var writer: std.Io.Writer = .fixed(&encoded); + try header.encode(&writer); + try io.writeInt(&writer, TerminalStyleId, 1); + try writer.splatByteAll(0, style.len); + + var source = FailOnceReader.init( + writer.buffered(), + Header.len + @sizeOf(TerminalStyleId), + ); + var decoded = decodePayload( + &source.interface, + std.testing.allocator, + ) catch |err| { + try std.testing.expectEqual(error.ReadFailed, err); + return; + }; + defer decoded.deinit(); + try std.testing.expect(false); +} + test "decode normalizes duplicate default and invalid style entries" { const header: Header = .{ .columns = 1, diff --git a/src/terminal/snapshot/screen.zig b/src/terminal/snapshot/screen.zig index 72c79a999..20bc24a8e 100644 --- a/src/terminal/snapshot/screen.zig +++ b/src/terminal/snapshot/screen.zig @@ -847,7 +847,7 @@ pub const SavedCursor = struct { return .{ .x = try io.readInt(reader, u16), .y = try io.readInt(reader, u16), - .pen = style.decodeOrDiscard(reader) catch .{}, + .pen = (try style.decodeOrNull(reader)) orelse .{}, .flags = try Flags.decode(reader), .charset = decodeCharsetState( try io.readInt(reader, u16), @@ -1008,7 +1008,8 @@ pub const Header = struct { try reader.takeByte(), ) orelse .block; const cursor_flags = try CursorFlags.decode(reader); - const cursor_pen: TerminalStyle = style.decodeOrDiscard(reader) catch .{}; + const cursor_pen: TerminalStyle = + (try style.decodeOrNull(reader)) orelse .{}; const hyperlink_implicit_id = try io.readInt(reader, u32); // Charset and selective-erase state. diff --git a/src/terminal/snapshot/style.zig b/src/terminal/snapshot/style.zig index 6df8ba03c..176190b75 100644 --- a/src/terminal/snapshot/style.zig +++ b/src/terminal/snapshot/style.zig @@ -172,6 +172,27 @@ pub fn decodeOrDiscard( return decode(&source); } +/// Decode one complete entry, returning null for invalid semantic contents. +/// +/// Reader errors remain structural and are always propagated. This is the +/// lenient entry point for enclosing codecs which can safely replace an +/// invalid fixed-size style without treating truncation as invalid styling. +pub fn decodeOrNull( + reader: *std.Io.Reader, +) std.Io.Reader.Error!?terminal_style.Style { + return decodeOrDiscard(reader) catch |err| switch (err) { + error.ReadFailed => return error.ReadFailed, + error.EndOfStream => return error.EndOfStream, + + error.InvalidColorKind, + error.InvalidColor, + error.InvalidUnderline, + error.InvalidFlags, + error.InvalidReserved, + => null, + }; +} + fn encodeColor( value: terminal_style.Style.Color, writer: *std.Io.Writer, @@ -383,6 +404,16 @@ test "decodeOrDiscard preserves the next entry boundary" { try std.testing.expectEqual(@as(u8, 0xFF), try reader.takeByte()); } +test "decodeOrNull distinguishes semantic errors from truncation" { + var invalid: [len]u8 = @splat(0); + invalid[0] = 3; + var invalid_reader: std.Io.Reader = .fixed(&invalid); + try std.testing.expectEqual(null, try decodeOrNull(&invalid_reader)); + + var truncated: std.Io.Reader = .fixed(invalid[0 .. len - 1]); + try std.testing.expectError(error.EndOfStream, decodeOrNull(&truncated)); +} + test "reject every truncation" { const fixture = [_]u8{0} ** len; for (0..len) |fixture_len| { From cafe7d5da43fb19020117e1ab5cca06751f8d0c7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Aug 2026 07:38:00 -0700 Subject: [PATCH 3/4] terminal/snapshot: clamp decoded saved cursors SCREEN decoding restored saved cursor coordinates directly from the wire even when they exceeded the current terminal dimensions, unlike the live cursor restoration path. --- src/terminal/snapshot/screen.zig | 70 ++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 7 deletions(-) diff --git a/src/terminal/snapshot/screen.zig b/src/terminal/snapshot/screen.zig index 20bc24a8e..0b477ee8e 100644 --- a/src/terminal/snapshot/screen.zig +++ b/src/terminal/snapshot/screen.zig @@ -435,7 +435,7 @@ pub fn decode( options.max_scrollback_bytes == 0, .cursor = cursor, .saved_cursor = if (saved_cursor) |value| - value.terminal() + value.terminal(options.cols, options.rows) else null, .charset = header.charset, @@ -813,13 +813,18 @@ pub const SavedCursor = struct { }; } - fn terminal(self: SavedCursor) TerminalScreen.SavedCursor { + fn terminal( + self: SavedCursor, + cols: u16, + rows: u16, + ) TerminalScreen.SavedCursor { + const x = @min(self.x, cols - 1); return .{ - .x = self.x, - .y = self.y, + .x = x, + .y = @min(self.y, rows - 1), .style = self.pen, .protected = self.flags.protected, - .pending_wrap = self.flags.pending_wrap, + .pending_wrap = self.flags.pending_wrap and x == cols - 1, .origin = self.flags.origin, .charset = self.charset, }; @@ -1784,8 +1789,8 @@ test "framed native SCREEN and PAGE sequence" { } const restored_saved = restored.saved_cursor.?; - try std.testing.expectEqual(@as(u16, 0x0102), restored_saved.x); - try std.testing.expectEqual(@as(u16, 0x0304), restored_saved.y); + try std.testing.expectEqual(@as(u16, 7), restored_saved.x); + try std.testing.expectEqual(@as(u16, 7), restored_saved.y); try std.testing.expect(restored_saved.protected); try std.testing.expect(restored_saved.pending_wrap); try std.testing.expect(restored_saved.origin); @@ -2158,6 +2163,57 @@ test "SCREEN validates pending wrap against a mixed-width cursor page" { decoded.screen.assertIntegrity(); } +test "SCREEN clamps a decoded saved cursor to terminal dimensions" { + var screen = try TerminalScreen.init( + std.testing.io, + std.testing.allocator, + .{ .cols = 2, .rows = 2, .max_scrollback_bytes = 0 }, + ); + defer screen.deinit(); + + var destination: std.Io.Writer.Allocating = .init( + std.testing.allocator, + ); + defer destination.deinit(); + var stream: record.Writer = .init( + std.testing.allocator, + &destination.writer, + ); + defer stream.deinit(); + + var header = Header.init(&screen, .primary, 1); + header.saved_cursor_present = true; + const screen_payload = stream.begin(.screen); + try header.encode(screen_payload); + try (SavedCursor{ + .x = std.math.maxInt(u16), + .y = std.math.maxInt(u16), + .pen = .{}, + .flags = .{ .pending_wrap = true }, + .charset = .{}, + }).encode(screen_payload); + try screen_payload.writeByte(0); + try stream.finish(); + try page.encode( + screen.pages.getTopLeft(.active).node.pageAssumeResident(), + &stream, + ); + + var source: std.Io.Reader = .fixed(destination.written()); + var decoded = try decode( + &source, + std.testing.io, + std.testing.allocator, + .{ .cols = 2, .rows = 2, .max_scrollback_bytes = 0 }, + ); + defer decoded.deinit(); + + const saved = decoded.screen.saved_cursor.?; + try std.testing.expectEqual(@as(u16, 1), saved.x); + try std.testing.expectEqual(@as(u16, 1), saved.y); + try std.testing.expect(saved.pending_wrap); +} + test "SCREEN restoration rejects invalid and incomplete sequences" { var screen = try TerminalScreen.init( std.testing.io, From 7d9aaa29703750c4f129790314a54c9a6cd5c7c5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 3 Aug 2026 07:38:46 -0700 Subject: [PATCH 4/4] terminal/snapshot: clarify incremental history errors Document why incremental history decoding exposes native page finalization errors and intentionally bypasses the one-shot ExistingHistory guard after READY. --- src/terminal/snapshot/snapshot.zig | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/terminal/snapshot/snapshot.zig b/src/terminal/snapshot/snapshot.zig index 2153802b4..f3adda8cb 100644 --- a/src/terminal/snapshot/snapshot.zig +++ b/src/terminal/snapshot/snapshot.zig @@ -426,6 +426,8 @@ pub const Decoder = struct { DuplicateHistory, /// A history PAGE cannot join a native PageList. + /// These propagate from `nextPage`'s non-limit finalize errors; + /// only limit failures are deliberately converted into drops. InvalidPageDimensions, RowCountOverflow, PageSizeOverflow, @@ -553,6 +555,12 @@ pub const Decoder = struct { try page.discard(self.stream.reader()); break :rows 0; }; + // The one-shot history decoder rejects a Screen which already has + // complete history. This incremental path intentionally bypasses + // that guard: the terminal has been live since READY and may have + // accumulated new history. Generation and width checks above keep + // the destination compatible, while per-page finalization enforces + // its current scrollback limits. break :rows history.decodePage( self.stream.reader(), alloc,