diff --git a/src/terminal/snapshot/page.zig b/src/terminal/snapshot/page.zig index 6ffa0d696..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. @@ -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" { @@ -1261,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, @@ -1518,10 +1642,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" { diff --git a/src/terminal/snapshot/screen.zig b/src/terminal/snapshot/screen.zig index 72c79a999..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, }; @@ -847,7 +852,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 +1013,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. @@ -1783,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); @@ -2157,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, 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, 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| {