terminal/snapshot: more misc bugs (#13573)

Again nothing critical, just some polish around the edges.
This commit is contained in:
Mitchell Hashimoto
2026-08-03 08:20:43 -07:00
committed by GitHub
4 changed files with 244 additions and 14 deletions

View File

@@ -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" {

View File

@@ -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,

View File

@@ -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,

View File

@@ -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| {