From e8e56e782c7cfa277839b489d5b13396f8bbe24b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 28 Jul 2026 07:28:44 -0700 Subject: [PATCH] terminal/snapshot: small edits --- src/terminal/snapshot/envelope.zig | 29 ++++++++----- src/terminal/snapshot/grid.zig | 32 +++++++------- src/terminal/snapshot/hyperlink.zig | 38 ++++------------- src/terminal/snapshot/io.zig | 2 + src/terminal/snapshot/main.zig | 2 +- src/terminal/snapshot/page.zig | 65 ++++++++++++----------------- src/terminal/snapshot/record.zig | 18 +------- src/terminal/snapshot/style.zig | 8 ++-- 8 files changed, 77 insertions(+), 117 deletions(-) diff --git a/src/terminal/snapshot/envelope.zig b/src/terminal/snapshot/envelope.zig index 7a3496676..726167391 100644 --- a/src/terminal/snapshot/envelope.zig +++ b/src/terminal/snapshot/envelope.zig @@ -13,22 +13,22 @@ //! //! | Offset | Size | Field | //! | -----: | ---: | :------------------- | -//! | 0 | 8 | Magic (`BOOSNAP\0`) | +//! | 0 | 8 | Magic (`GHOSTSNP`) | //! | 8 | 2 | Version (`u16`) | const std = @import("std"); const io = @import("io.zig"); /// Identifies a Ghostty terminal snapshot and rejects unrelated input before -/// any record decoding begins. The trailing NUL is part of the wire value. -pub const magic = "BOOSNAP\x00"; +/// any record decoding begins. All eight bytes are part of the wire value. +pub const magic = "GHOSTSNP"; /// The complete compatibility boundary for snapshot layout and behavior. -/// Version 0 readers require this value to match exactly. -pub const version: u16 = 0; +/// Version 1 readers require this value to match exactly. +pub const version: u16 = 1; /// Number of bytes in the fixed envelope: magic followed by version. -pub const encoded_len = magic.len + @sizeOf(@TypeOf(version)); +pub const encoded_len = computeLen(); comptime { // We expect this so if it changes we should think carefully. @@ -56,22 +56,31 @@ pub fn decode(reader: *std.Io.Reader) DecodeError!void { if (actual_version != version) return error.UnsupportedVersion; } +fn computeLen() usize { + comptime { + var buf: [128]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buf); + encode(&writer) catch unreachable; + return writer.end; + } +} + test "golden encoding" { var buf: [encoded_len]u8 = undefined; var writer: std.Io.Writer = .fixed(&buf); try encode(&writer); try std.testing.expectEqualStrings( - "BOOSNAP\x00\x00\x00", + "GHOSTSNP\x01\x00", writer.buffered(), ); } test "reject invalid magic and version" { - var invalid_magic: std.Io.Reader = .fixed("BOOSNAX\x00\x00\x00"); + var invalid_magic: std.Io.Reader = .fixed("GHOSTSNX\x01\x00"); try std.testing.expectError(error.InvalidMagic, decode(&invalid_magic)); - var invalid_version: std.Io.Reader = .fixed("BOOSNAP\x00\x01\x00"); + var invalid_version: std.Io.Reader = .fixed("GHOSTSNP\x00\x00"); try std.testing.expectError( error.UnsupportedVersion, decode(&invalid_version), @@ -79,7 +88,7 @@ test "reject invalid magic and version" { } test "reject every truncation" { - const fixture = "BOOSNAP\x00\x00\x00"; + const fixture = "GHOSTSNP\x01\x00"; for (0..encoded_len) |len| { var reader: std.Io.Reader = .fixed(fixture[0..len]); try std.testing.expectError(error.EndOfStream, decode(&reader)); diff --git a/src/terminal/snapshot/grid.zig b/src/terminal/snapshot/grid.zig index 18d45b838..4f5604fe2 100644 --- a/src/terminal/snapshot/grid.zig +++ b/src/terminal/snapshot/grid.zig @@ -38,7 +38,7 @@ //! | 1 | Prompt | //! | 2 | Prompt continuation | //! -//! Value 3 is invalid in snapshot version 0. +//! Value 3 is invalid in snapshot version 1. //! //! ## Cell //! @@ -94,7 +94,7 @@ //! | 1 | Input | //! | 2 | Prompt | //! -//! Value 3 is invalid in snapshot version 0. +//! Value 3 is invalid in snapshot version 1. //! //! Style and hyperlink ID zero mean no style and no hyperlink. Other IDs //! refer to entries in the containing record's separate style and hyperlink @@ -122,20 +122,14 @@ const TerminalStyleId = terminal_style.Id; /// Build this by inserting each decoded style into the page, then recording /// the encoded ID and the ID returned by the page's style set. Style ID zero is /// implicit and does not need an entry. -pub const StyleRemap = std.AutoHashMap( - TerminalStyleId, - TerminalStyleId, -); +pub const StyleRemap = std.AutoHashMap(TerminalStyleId, TerminalStyleId); /// Maps encoded hyperlink table IDs to IDs assigned by the destination page. /// /// Build this by inserting each decoded hyperlink into the page, then /// recording the encoded ID and the ID returned by the page's hyperlink set. /// Hyperlink ID zero is implicit and does not need an entry. -pub const HyperlinkRemap = std.AutoHashMap( - TerminalHyperlinkId, - TerminalHyperlinkId, -); +pub const HyperlinkRemap = std.AutoHashMap(TerminalHyperlinkId, TerminalHyperlinkId); pub const EncodeError = std.Io.Writer.Error; @@ -306,7 +300,11 @@ pub fn decode( .codepoint => { const cp = std.math.cast(u21, header.value.codepoint) orelse return error.InvalidCodepoint; - if (!validCodepoint(cp)) return error.InvalidCodepoint; + if (cp > 0x10FFFF or + (cp >= 0xD800 and cp <= 0xDFFF)) + { + return error.InvalidCodepoint; + } if (cp == kitty.graphics.unicode.placeholder) { return error.UnsupportedKittyGraphics; } @@ -367,7 +365,11 @@ pub fn decode( const suffix_raw = try io.readInt(reader, u32); const suffix = std.math.cast(u21, suffix_raw) orelse return error.InvalidCodepoint; - if (!validCodepoint(suffix)) return error.InvalidCodepoint; + if (suffix > 0x10FFFF or + (suffix >= 0xD800 and suffix <= 0xDFFF)) + { + return error.InvalidCodepoint; + } if (suffix == kitty.graphics.unicode.placeholder) { return error.UnsupportedKittyGraphics; } @@ -406,7 +408,7 @@ const RowHeader = packed struct(u8) { }; /// The fixed fields that precede a cell's grapheme suffix codepoints. -pub const CellHeader = struct { +const CellHeader = struct { /// Number of bytes written by `encode`, calculated using the encoder itself /// so this remains synchronized with the field-by-field wire format. pub const len = computeLen(); @@ -556,7 +558,3 @@ pub const CellHeader = struct { }; }; }; - -fn validCodepoint(cp: u21) bool { - return cp <= 0x10FFFF and (cp < 0xD800 or cp > 0xDFFF); -} diff --git a/src/terminal/snapshot/hyperlink.zig b/src/terminal/snapshot/hyperlink.zig index 5f2503c54..71b27558c 100644 --- a/src/terminal/snapshot/hyperlink.zig +++ b/src/terminal/snapshot/hyperlink.zig @@ -54,7 +54,7 @@ pub const EncodeError = std.Io.Writer.Error; /// Errors possible while decoding one allocator-owned hyperlink entry. pub const DecodeError = std.Io.Reader.Error || Allocator.Error || error{ - /// The hyperlink kind is not defined by snapshot version 0. + /// The hyperlink kind is not defined by snapshot version 1. InvalidKind, }; @@ -62,7 +62,7 @@ pub const DecodeError = std.Io.Reader.Error || Allocator.Error || error{ pub const DecodePageError = std.Io.Reader.Error || terminal_page.Page.InsertHyperlinkError || error{ - /// The hyperlink kind is not defined by snapshot version 0. + /// The hyperlink kind is not defined by snapshot version 1. InvalidKind, /// The hyperlink value already exists in the page. @@ -268,18 +268,18 @@ test "golden explicit encoding" { } test "empty strings round trip" { - const values = .{ - terminal_hyperlink.Hyperlink{ + const values: [2]terminal_hyperlink.Hyperlink = .{ + .{ .id = .{ .implicit = 0 }, .uri = "", }, - terminal_hyperlink.Hyperlink{ + .{ .id = .{ .explicit = "" }, .uri = "", }, }; - inline for (values) |value| { + for (values) |value| { var encoded: [9]u8 = undefined; var writer: std.Io.Writer = .fixed(&encoded); try encode(value, &writer); @@ -301,28 +301,6 @@ test "empty strings round trip" { } } -test "decode with a one-byte reader buffer" { - const fixture = - "\x01\x04\x03\x02\x01\x03\x00\x00\x00uri" ++ - "\x02\x02\x00\x00\x00id\x03\x00\x00\x00uri"; - var source: std.Io.Reader = .fixed(fixture); - var read_buf: [1]u8 = undefined; - var limited = source.limited(.unlimited, &read_buf); - - const implicit = try decode(&limited.interface, std.testing.allocator); - defer implicit.deinit(std.testing.allocator); - try std.testing.expectEqualStrings("uri", implicit.uri); - try std.testing.expectEqual( - @as(u32, 0x01020304), - implicit.id.implicit, - ); - - const explicit = try decode(&limited.interface, std.testing.allocator); - defer explicit.deinit(std.testing.allocator); - try std.testing.expectEqualStrings("id", explicit.id.explicit); - try std.testing.expectEqualStrings("uri", explicit.uri); -} - test "reject invalid kinds" { inline for (.{ 0, 3, std.math.maxInt(u8) }) |kind| { var fixture: [1]u8 = .{kind}; @@ -365,12 +343,12 @@ test "decode allocation failure" { } test "reject every truncation" { - const fixtures = .{ + const fixtures: [2][]const u8 = .{ "\x01\x04\x03\x02\x01\x03\x00\x00\x00uri", "\x02\x02\x00\x00\x00id\x03\x00\x00\x00uri", }; - inline for (fixtures) |fixture| { + for (fixtures) |fixture| { for (0..fixture.len) |fixture_len| { var reader: std.Io.Reader = .fixed(fixture[0..fixture_len]); try std.testing.expectError( diff --git a/src/terminal/snapshot/io.zig b/src/terminal/snapshot/io.zig index 77c1c5926..d47f96ccb 100644 --- a/src/terminal/snapshot/io.zig +++ b/src/terminal/snapshot/io.zig @@ -38,6 +38,8 @@ test "integer round trip with a one-byte reader buffer" { try writeInt(&writer, u16, 0x1234); try writeInt(&writer, u32, 0x56789abc); + // Keep the reader buffer smaller than either integer to verify readInt + // does not inherit std.Io.Reader.takeInt's buffer-size requirement. var source: std.Io.Reader = .fixed(&encoded); var buf: [1]u8 = undefined; var limited = source.limited(.unlimited, &buf); diff --git a/src/terminal/snapshot/main.zig b/src/terminal/snapshot/main.zig index 20bc2d41c..93230ef09 100644 --- a/src/terminal/snapshot/main.zig +++ b/src/terminal/snapshot/main.zig @@ -15,7 +15,7 @@ //! //! ## Snapshot Format //! -//! This documents snapshot format 0. Version 0 is the work-in-progress +//! This documents snapshot format 1. Version 1 is the work-in-progress //! format that we intended to continue to break until we can promise //! binary compatibility. //! diff --git a/src/terminal/snapshot/page.zig b/src/terminal/snapshot/page.zig index b6324860d..73e865caa 100644 --- a/src/terminal/snapshot/page.zig +++ b/src/terminal/snapshot/page.zig @@ -123,7 +123,7 @@ const PayloadDecodeError = style.DecodeError || grid.DecodeError || Header.CapacityError || error{ - /// The hyperlink kind is not defined by snapshot version 0. + /// The hyperlink kind is not defined by snapshot version 1. InvalidKind, /// The advertised string capacity cannot hold the encoded hyperlinks. @@ -478,6 +478,9 @@ fn pageHyperlink( }; } +// Regenerate these after a wire-format change from `writer.buffered()` in the +// sparse-page test and `destination.written()` in the empty-record test below. +// Format those byte slices as Zig-escaped strings before replacing the literals. const test_page_fixture = "\x03\x00\x02\x00\x02\x00\x02\x00\x08\x00\x00\x02\x80\x00\x00\x00" ++ "\x00\x01\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" ++ @@ -576,6 +579,8 @@ test "framed PAGE encode and decode a sparse native page" { var page = try TerminalPage.init(capacity); defer page.deinit(); + // Create holes in both source tables so the encoded IDs exercise sparse + // table remapping rather than coincidentally matching decoded IDs. const style_a = try page.styles.add(page.memory, .{ .flags = .{ .bold = true }, }); @@ -619,6 +624,8 @@ test "framed PAGE encode and decode a sparse native page" { try page.setHyperlink(second.row, second.cell, hyperlink_b); try page.setHyperlink(third.row, third.cell, hyperlink_a); + // Cover graphemes, wide-cell pairs, colors, protection, and semantic + // metadata in one compact two-row page. const grapheme = page.getRowAndCell(0, 1); grapheme.cell.* = .init('x'); try page.setGraphemes( @@ -666,6 +673,8 @@ test "framed PAGE encode and decode a sparse native page" { }; try std.testing.expectEqual(header, Header.init(&page)); + // Lock the payload bytes and independently verify the counting writer sees + // the same encoded length. var counter: std.Io.Writer.Discarding = .init(&.{}); try encodePayload(&page, &counter.writer); @@ -677,6 +686,7 @@ test "framed PAGE encode and decode a sparse native page" { try std.testing.expectEqualStrings(fixture, writer.buffered()); try std.testing.expectEqual(@as(u64, fixture.len), counter.count); + // A one-byte backing buffer exercises streaming reads across every field. var source: std.Io.Reader = .fixed(writer.buffered()); var read_buf: [1]u8 = undefined; var limited = source.limited(.unlimited, &read_buf); @@ -689,6 +699,8 @@ test "framed PAGE encode and decode a sparse native page" { try std.testing.expectEqual(header, Header.init(&decoded)); try decoded.verifyIntegrity(std.testing.allocator); + // Decoding compacts the sparse source IDs while preserving table values + // and all cell references through the remap tables. var style_it = decoded.styles.iterator(decoded.memory); const decoded_style_a = style_it.next().?; try std.testing.expectEqual(@as(TerminalStyleId, 1), decoded_style_a.id); @@ -763,6 +775,8 @@ test "framed PAGE encode and decode a sparse native page" { decoded_spacer_head.row.semantic_prompt, ); + // The first re-encode reflects compacted IDs; subsequent round trips must + // stabilize byte-for-byte. var reencoded: [512]u8 = undefined; var rewriter: std.Io.Writer = .fixed(&reencoded); try encodePayload(&decoded, &rewriter); @@ -855,6 +869,7 @@ test "framed PAGE golden empty record" { test "framed PAGE validates tag length checksum and exhaustion" { { + // A valid non-PAGE tag is rejected before payload decoding. var wrong_tag = test_empty_framed_page_fixture.*; std.mem.writeInt(u16, wrong_tag[0..2], @intFromEnum(record.Tag.screen), .little); var reader: std.Io.Reader = .fixed(&wrong_tag); @@ -865,6 +880,7 @@ test "framed PAGE validates tag length checksum and exhaustion" { } { + // Corrupt only the stored checksum. var invalid_checksum = test_empty_framed_page_fixture.*; invalid_checksum[6] ^= 1; var reader: std.Io.Reader = .fixed(&invalid_checksum); @@ -875,6 +891,7 @@ test "framed PAGE validates tag length checksum and exhaustion" { } { + // Mutate a cell without updating the checksum. var invalid_payload = test_empty_framed_page_fixture.*; const value_offset = record.Header.len + Header.len + @@ -889,6 +906,7 @@ test "framed PAGE validates tag length checksum and exhaustion" { } { + // Advertise one byte less than the PAGE decoder requires. var short_payload = test_empty_framed_page_fixture.*; std.mem.writeInt(u32, short_payload[2..6], 36, .little); var reader: std.Io.Reader = .fixed(&short_payload); @@ -899,6 +917,8 @@ test "framed PAGE validates tag length checksum and exhaustion" { } { + // Add a payload byte and recompute the checksum so exhaustion, rather + // than checksum validation, is what fails. var trailing: [test_empty_framed_page_fixture.len + 1]u8 = undefined; @memcpy( trailing[0..test_empty_framed_page_fixture.len], @@ -965,7 +985,7 @@ test "decode accepts unordered sparse style IDs and rejects zero" { Header.len + 2 * (2 + style.len) + 1 + - grid.CellHeader.len + 16 ]u8 = undefined; var descending_writer: std.Io.Writer = .fixed(&descending); try header.encode(&descending_writer); @@ -974,7 +994,7 @@ test "decode accepts unordered sparse style IDs and rejects zero" { try io.writeInt(&descending_writer, TerminalStyleId, 2); try style.encode(.{ .flags = .{ .italic = true } }, &descending_writer); try descending_writer.writeByte(0); - try grid.CellHeader.encode(.{}, &descending_writer); + try descending_writer.splatByteAll(0, 16); var descending_reader: std.Io.Reader = .fixed( descending_writer.buffered(), @@ -1035,7 +1055,7 @@ test "decode accepts unordered sparse hyperlink IDs" { Header.len + 2 * 11 + 1 + - grid.CellHeader.len + 16 ]u8 = undefined; var writer: std.Io.Writer = .fixed(&encoded); try header.encode(&writer); @@ -1044,7 +1064,7 @@ test "decode accepts unordered sparse hyperlink IDs" { try io.writeInt(&writer, TerminalHyperlinkId, 2); try hyperlink.encode(second, &writer); try writer.writeByte(0); - try grid.CellHeader.encode(.{}, &writer); + try writer.splatByteAll(0, 16); var reader: std.Io.Reader = .fixed(writer.buffered()); var decoded = try decodePayload( @@ -1059,6 +1079,7 @@ test "decode accepts unordered sparse hyperlink IDs" { } test "decode defaults missing sparse cell references" { + // An unknown style ID falls back to the default style. const style_header: Header = .{ .columns = 1, .rows = 1, @@ -1091,6 +1112,7 @@ test "decode defaults missing sparse cell references" { style_page.getRowAndCell(0, 0).cell.style_id, ); + // An unknown hyperlink ID likewise falls back to no hyperlink. const hyperlink_header: Header = .{ .columns = 1, .rows = 1, @@ -1349,36 +1371,3 @@ test "decode rejects duplicate hyperlinks with empty strings" { decodePayload(&reader, std.testing.allocator), ); } - -test "decode reads hyperlink strings into page storage" { - const link: TerminalHyperlink = .{ - .id = .{ .explicit = "id" }, - .uri = "uri", - }; - const hyperlink_capacity: u16 = @intCast( - TerminalHyperlinkSet.capacityForCount(1) * - @sizeOf(TerminalHyperlinkSet.Item), - ); - const header: Header = .{ - .columns = 1, - .rows = 1, - .style_count = 0, - .hyperlink_count = 1, - .style_capacity = 0, - .hyperlink_capacity_bytes = hyperlink_capacity, - .grapheme_capacity_bytes = 0, - .string_capacity_bytes = 0, - }; - - var encoded: [Header.len + 16]u8 = undefined; - var writer: std.Io.Writer = .fixed(&encoded); - try header.encode(&writer); - try io.writeInt(&writer, TerminalHyperlinkId, 1); - try hyperlink.encode(link, &writer); - - var reader: std.Io.Reader = .fixed(writer.buffered()); - try std.testing.expectError( - error.InvalidStringCapacity, - decodePayload(&reader, std.testing.allocator), - ); -} diff --git a/src/terminal/snapshot/record.zig b/src/terminal/snapshot/record.zig index 5a99f8607..3694173f1 100644 --- a/src/terminal/snapshot/record.zig +++ b/src/terminal/snapshot/record.zig @@ -320,24 +320,8 @@ test "golden PAGE record header and checksum" { ); } -test "decode header with a one-byte reader buffer" { - const fixture = "\x03\x00\x18\x00\x00\x00\x1b\x44\x78\x71"; - var source: std.Io.Reader = .fixed(fixture); - var buf: [1]u8 = undefined; - var limited = source.limited(.unlimited, &buf); - - try std.testing.expectEqual( - Header{ - .tag = .page, - .payload_len = 24, - .crc32c = 0x7178441b, - }, - try Header.decode(&limited.interface), - ); -} - test "reject invalid tags" { - inline for (.{ 0, 7, std.math.maxInt(u16) }) |tag| { + for ([_]u16{ 0, 7, std.math.maxInt(u16) }) |tag| { var fixture = [_]u8{0} ** Header.len; std.mem.writeInt(u16, fixture[0..2], tag, .little); var reader: std.Io.Reader = .fixed(&fixture); diff --git a/src/terminal/snapshot/style.zig b/src/terminal/snapshot/style.zig index 87bc3f5be..78c905dff 100644 --- a/src/terminal/snapshot/style.zig +++ b/src/terminal/snapshot/style.zig @@ -43,7 +43,7 @@ //! | 4 | Dotted | //! | 5 | Dashed | //! -//! Underline values 6 and 7 are invalid in snapshot version 0. +//! Underline values 6 and 7 are invalid in snapshot version 1. const std = @import("std"); const io = @import("io.zig"); @@ -81,10 +81,10 @@ const ColorKind = enum(u8) { /// Errors possible while decoding one style entry. pub const DecodeError = std.Io.Reader.Error || error{ - /// A color kind is not defined by snapshot version 0. + /// A color kind is not defined by snapshot version 1. InvalidColorKind, - /// The encoded underline kind is not defined by snapshot version 0. + /// The encoded underline kind is not defined by snapshot version 1. InvalidUnderline, /// One or more reserved style flag bits are set. @@ -279,7 +279,7 @@ test "flag bit layout" { } } -test "decode with a one-byte reader buffer" { +test "decoding" { const fixture = "\x00\x00\x00\x00" ++ "\x01\x7f\x00\x00" ++