From d14847183844e84fb8282ebe5a6c9061f40530e3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 2 Aug 2026 20:46:38 -0700 Subject: [PATCH 1/5] terminal/snapshot: preserve mixed-width pending wrap SCREEN decode clamped the cursor x coordinate to the physical page width, but validated pending wrap against the terminal-wide column count. A lazily reflowed page narrower than the current terminal could therefore lose a valid pending-wrap state at its last physical column. The next write would continue on the same row instead of wrapping. Validate pending wrap against the cursor page width, matching the clamp and the page-local cursor pin. Add a mixed-width decode regression that places the cursor at the narrow page boundary. --- src/terminal/snapshot/screen.zig | 54 +++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/terminal/snapshot/screen.zig b/src/terminal/snapshot/screen.zig index d05ff00ba..defb9ce24 100644 --- a/src/terminal/snapshot/screen.zig +++ b/src/terminal/snapshot/screen.zig @@ -411,7 +411,7 @@ pub fn decode( .y = y, .cursor_style = header.cursor_style, .pending_wrap = header.cursor_flags.pending_wrap and - x == options.cols - 1, + x == row_pin.node.cols() - 1, .protected = header.cursor_flags.protected, .style = header.cursor_pen, .hyperlink_implicit_id = header.hyperlink_implicit_id, @@ -2048,6 +2048,58 @@ test "SCREEN restoration normalizes invalid cursor positions" { } } +test "SCREEN validates pending wrap against a mixed-width cursor page" { + var screen = try TerminalScreen.init( + std.testing.io, + std.testing.allocator, + .{ .cols = 8, .rows = 1, .max_scrollback_bytes = 0 }, + ); + defer screen.deinit(); + + // Model a lazily reflowed active page that is narrower than the terminal. + // Column three is its physical final column even though it is not column + // seven of the current terminal dimensions. + var narrow_page = try terminal_page.Page.init(.{ .cols = 4, .rows = 1 }); + defer narrow_page.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.cursor_x = 3; + header.cursor_y = 0; + header.cursor_flags.pending_wrap = true; + header.saved_cursor_present = false; + + const screen_payload = stream.begin(.screen); + errdefer stream.cancel(); + try header.encode(screen_payload); + try screen_payload.writeByte(0); + try stream.finish(); + try page.encode(&narrow_page, &stream); + + var source: std.Io.Reader = .fixed(destination.written()); + var decoded = try decode( + &source, + std.testing.io, + std.testing.allocator, + .{ .cols = 8, .rows = 1, .max_scrollback_bytes = 0 }, + ); + defer decoded.deinit(); + + try std.testing.expectEqual(@as(u16, 4), decoded.screen.cursor.page_pin.node.cols()); + try std.testing.expectEqual(@as(u16, 3), decoded.screen.cursor.x); + try std.testing.expect(decoded.screen.cursor.pending_wrap); + decoded.screen.assertIntegrity(); +} + test "SCREEN restoration rejects invalid and incomplete sequences" { var screen = try TerminalScreen.init( std.testing.io, From cbc9f360b1d6be9305f25cd6e68c49884fa24187 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 2 Aug 2026 20:48:22 -0700 Subject: [PATCH 2/5] terminal/snapshot: report invalid decoder states Decoder.next treated calls before READY and calls after any prior decode error as unreachable. Network or mux glue that retried after a truncated history record, or invoked next before setup completed, could therefore turn a recoverable protocol misuse into a process panic. Add DecoderNotReady and DecoderFailed to NextError and return them for the start and failed states. Keep finished calls idempotent, and cover both an early call and a retry after FINISH truncation. --- src/terminal/snapshot/snapshot.zig | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/terminal/snapshot/snapshot.zig b/src/terminal/snapshot/snapshot.zig index a6b3146aa..2153802b4 100644 --- a/src/terminal/snapshot/snapshot.zig +++ b/src/terminal/snapshot/snapshot.zig @@ -413,6 +413,12 @@ pub const Decoder = struct { page.DecodeError || Allocator.Error || error{ + /// `next` was called before `ready` completed successfully. + DecoderNotReady, + + /// A prior `next` call failed and invalidated the stream position. + DecoderFailed, + /// A HISTORY names a key not declared by TERMINAL. UnexpectedHistoryKey, @@ -457,7 +463,8 @@ pub const Decoder = struct { switch (self.state) { // `ready` must succeed before history exists to decode, and a // failed decoder no longer knows its stream position. - .start, .failed => unreachable, + .start => return error.DecoderNotReady, + .failed => return error.DecoderFailed, .finished => return null, .history => {}, } @@ -1774,6 +1781,23 @@ test "incremental decode restores a renderable terminal at READY" { try testing.expectEqualStrings(&test_complete_fixture, reencoded.written()); } +test "incremental next reports invalid decoder states" { + const testing = std.testing; + var source: std.Io.Reader = .fixed(""); + var decoder: Decoder = .init(&source); + var terminal_value = try Terminal.init( + testing.io, + testing.allocator, + .{ .cols = 1, .rows = 1 }, + ); + defer terminal_value.deinit(testing.allocator); + + try testing.expectError( + error.DecoderNotReady, + decoder.next(testing.allocator, &terminal_value), + ); +} + test "incremental decode applies history below live PTY output" { const testing = std.testing; @@ -2065,6 +2089,10 @@ test "incremental decode failure leaves applied history usable" { error.EndOfStream, decoder.next(testing.allocator, &restored), ); + try testing.expectError( + error.DecoderFailed, + decoder.next(testing.allocator, &restored), + ); // The transferred terminal was never owned by the decoder: it remains // valid with the contiguous history prefix that did apply, and only From e89ff37aa8a551369a8f5ae9022cc43410b6d59e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 2 Aug 2026 20:49:48 -0700 Subject: [PATCH 3/5] terminal/snapshot: encode screen pages safely SCREEN encoding assumed every page from the active boundary onward was resident. A debug assertion guarded that PageList policy invariant, but release builds immediately used pageAssumeResident. If compression policy ever allowed a SCREEN suffix page to remain compressed, the encoder would read an inactive union field, causing undefined behavior and potentially a crash or corrupt snapshot. Use pagePreservingState for every SCREEN suffix page, as HISTORY already does, and include allocation failure in EncodeError. Resident pages remain a zero-allocation borrow while compressed pages decode into temporary read-only storage without changing the source representation. Exercise the path with an explicitly compressed active suffix page. --- src/terminal/snapshot/screen.zig | 69 +++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/src/terminal/snapshot/screen.zig b/src/terminal/snapshot/screen.zig index defb9ce24..72c79a999 100644 --- a/src/terminal/snapshot/screen.zig +++ b/src/terminal/snapshot/screen.zig @@ -233,7 +233,7 @@ const PayloadEncodeError = hyperlink.EncodeError || error{ }; /// Errors possible while encoding a SCREEN and its complete PAGE sequence. -pub const EncodeError = PayloadEncodeError || page.EncodeError || error{ +pub const EncodeError = Allocator.Error || PayloadEncodeError || page.EncodeError || error{ /// The active area spans more pages than the SCREEN header can declare. PageCountOverflow, }; @@ -273,13 +273,14 @@ pub fn encode( try destination.finish(); } - // PageList never compresses the active-boundary page or any later page. - // Encoding this resident suffix therefore does not restore cold history or - // otherwise mutate the source screen. + // Active pages are resident today, but use the representation-safe access + // path so a future PageList compression-policy change cannot turn this + // wire encoder's optimization invariant into undefined behavior. node = first; while (node) |current| : (node = current.next) { - std.debug.assert(current.pageIfResident() != null); - try page.encode(current.pageAssumeResident(), destination); + var preserved = try current.pagePreservingState(screen.alloc); + defer preserved.deinit(); + try page.encode(preserved.page(), destination); } } @@ -1963,6 +1964,62 @@ test "SCREEN encodes the minimal complete-page active suffix" { try std.testing.expectError(error.EndOfStream, restore_source.takeByte()); } +test "SCREEN encoding preserves a compressed suffix page" { + const testing = std.testing; + const compression = @import("../compress.zig"); + + var screen = try TerminalScreen.init( + testing.io, + testing.allocator, + .{ .cols = 8, .rows = 2, .max_scrollback_bytes = 0 }, + ); + defer screen.deinit(); + screen.pages.getCell(.{ .active = .{} }).?.cell.* = .init('A'); + + // Force the active suffix into the representation that current PageList + // policy normally reserves for history. This models a future policy change + // and makes pageAssumeResident an invalid tagged-union access. + const node = screen.pages.getTopLeft(.active).node; + const resident = node.pageAssumeResident(); + const scratch = try testing.allocator.alloc( + u8, + try compression.Page.requiredScratch(resident.memory.len), + ); + defer testing.allocator.free(scratch); + var table: compression.lz4.HashTable = undefined; + const compressed = (try compression.Page.init( + testing.allocator, + resident, + scratch, + &table, + )).?; + node.data = .{ .compressed = compressed }; + try testing.expectEqual(.compressed, node.storage()); + + var destination: std.Io.Writer.Allocating = .init(testing.allocator); + defer destination.deinit(); + var stream: record.Writer = .init(testing.allocator, &destination.writer); + defer stream.deinit(); + try encode(&screen, .primary, &stream); + + // Encoding borrows or clones through PreservedPage and never changes the + // source node's storage representation. + try testing.expectEqual(.compressed, node.storage()); + + var source: std.Io.Reader = .fixed(destination.written()); + var decoded = try decode( + &source, + testing.io, + testing.allocator, + .{ .cols = 8, .rows = 2, .max_scrollback_bytes = 0 }, + ); + defer decoded.deinit(); + try testing.expectEqual( + @as(u21, 'A'), + decoded.screen.pages.getCell(.{ .active = .{} }).?.cell.codepoint(), + ); +} + test "SCREEN restoration normalizes invalid cursor positions" { var screen = try TerminalScreen.init( std.testing.io, From 9a5279db682832274442ba2471d277b2ca9b4fa4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 2 Aug 2026 20:52:23 -0700 Subject: [PATCH 4/5] terminal/snapshot: release decoded style table refs PAGE decoding inserted every valid style table entry into the native ref-counted set before decoding cells. That insertion contributed one reference in addition to every cell reference, unlike organically built pages where the initial add belongs to the first cell. An unused encoded style therefore remained live with refcount one and was emitted again on every re-encode; used styles were also permanently over-counted. After the grid has installed all cell references, release the temporary table-owned reference once per distinct live style. Unused styles become dead immediately and used styles retain exactly their cell count. Cover used reference counts, unordered sparse IDs, and canonical first re-encoding of an unused entry. --- src/terminal/snapshot/page.zig | 83 ++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 4 deletions(-) diff --git a/src/terminal/snapshot/page.zig b/src/terminal/snapshot/page.zig index 5549de98e..6ffa0d696 100644 --- a/src/terminal/snapshot/page.zig +++ b/src/terminal/snapshot/page.zig @@ -418,6 +418,18 @@ fn decodePayloadBody( &style_remap, &hyperlink_remap, ); + + // A newly inserted table value starts with one reference so grid decoding + // can safely attach it to any number of cells. Unlike organically built + // pages, that initial reference does not itself represent a cell. Release + // it once per distinct live style after every cell reference is installed; + // unused entries then become dead and disappear from canonical re-encoding. + for (1..@as(usize, page.styles.next_id)) |raw_id| { + const id: TerminalStyleId = @intCast(raw_id); + if (page.styles.refCount(page.memory, id) > 0) { + page.styles.release(page.memory, id); + } + } } /// The fixed logical dimensions, table counts, and allocation hints at the @@ -751,6 +763,14 @@ test "framed PAGE encode and decode a sparse native page" { .bg_color = .{ .palette = 42 }, }).eql(decoded_style_b.value_ptr.*)); try std.testing.expectEqual(null, style_it.next()); + try std.testing.expectEqual( + @as(u16, 2), + decoded.styles.refCount(decoded.memory, decoded_style_a.id), + ); + try std.testing.expectEqual( + @as(u16, 1), + decoded.styles.refCount(decoded.memory, decoded_style_b.id), + ); var hyperlink_it = decoded.hyperlink_set.iterator(decoded.memory); const decoded_link_a = hyperlink_it.next().?; @@ -914,7 +934,7 @@ test "decode sparse page rejects every truncation" { test "decode accepts unordered sparse style IDs and ignores zero" { const header: Header = .{ - .columns = 1, + .columns = 2, .rows = 1, .style_count = 2, .hyperlink_count = 0, @@ -927,7 +947,7 @@ test "decode accepts unordered sparse style IDs and ignores zero" { var descending: [ Header.len + 2 * (2 + style.len) + - 7 + 3 + 2 * 8 + 4 ]u8 = undefined; var descending_writer: std.Io.Writer = .fixed(&descending); try header.encode(&descending_writer); @@ -935,8 +955,16 @@ test "decode accepts unordered sparse style IDs and ignores zero" { try style.encode(.{ .flags = .{ .bold = true } }, &descending_writer); try io.writeInt(&descending_writer, TerminalStyleId, 2); try style.encode(.{ .flags = .{ .italic = true } }, &descending_writer); - try descending_writer.writeByte(0); // row flags - try io.writeInt(&descending_writer, u16, 0); // cell count + try descending_writer.writeByte(@bitCast(grid.Row{ .cell_width = .eight })); + try io.writeInt(&descending_writer, u16, 2); // cell count + try io.writeInt(&descending_writer, u64, @bitCast(grid.Cell{ + .content = 'A', + .style_id = 3, + })); + try io.writeInt(&descending_writer, u64, @bitCast(grid.Cell{ + .content = 'B', + .style_id = 2, + })); try io.writeInt(&descending_writer, u32, 0); // grapheme section var descending_reader: std.Io.Reader = .fixed( @@ -948,6 +976,14 @@ test "decode accepts unordered sparse style IDs and ignores zero" { ); defer decoded_descending.deinit(); try std.testing.expectEqual(@as(usize, 2), decoded_descending.styles.count()); + try std.testing.expectEqual( + @as(TerminalStyleId, 1), + decoded_descending.getRowAndCell(0, 0).cell.style_id, + ); + try std.testing.expectEqual( + @as(TerminalStyleId, 2), + decoded_descending.getRowAndCell(1, 0).cell.style_id, + ); const one_header: Header = .{ .columns = 1, @@ -1362,6 +1398,45 @@ test "decode normalizes duplicate default and invalid style entries" { ); } +test "decode releases unused style table references" { + 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, + }; + + var empty_page = try TerminalPage.init(.{ .cols = 1, .rows = 1 }); + defer empty_page.deinit(); + var grid_bytes: [32]u8 = undefined; + var grid_writer: std.Io.Writer = .fixed(&grid_bytes); + try grid.encode(&empty_page, &grid_writer); + + var encoded: std.Io.Writer.Allocating = .init(std.testing.allocator); + defer encoded.deinit(); + try header.encode(&encoded.writer); + try io.writeInt(&encoded.writer, TerminalStyleId, 7); + try style.encode(.{ .flags = .{ .bold = true } }, &encoded.writer); + try encoded.writer.writeAll(grid_writer.buffered()); + + var reader: std.Io.Reader = .fixed(encoded.written()); + var decoded = try decodePayload(&reader, std.testing.allocator); + defer decoded.deinit(); + try std.testing.expectEqual(@as(usize, 0), decoded.styles.count()); + + // The dead table-only entry is absent from the very first re-encode. + var canonical: [64]u8 = undefined; + var canonical_writer: std.Io.Writer = .fixed(&canonical); + try encodePayload(&decoded, &canonical_writer); + var canonical_reader: std.Io.Reader = .fixed(canonical_writer.buffered()); + const canonical_header = try Header.decode(&canonical_reader); + try std.testing.expectEqual(@as(u16, 0), canonical_header.style_count); +} + test "decode reuses duplicate hyperlinks" { const header: Header = .{ .columns = 1, From 418b5d18054a015f25efc30592a412a0ac5c6c68 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 2 Aug 2026 20:53:36 -0700 Subject: [PATCH 5/5] terminal/snapshot: harden grapheme suffix decode Grapheme suffix decoding accepted U+0000 even though zero is the native empty-cell sentinel. It also appended codepoints one at a time and, when page capacity failed after a prefix had been stored, left that truncated prefix attached to the cell. Hostile snapshots could therefore introduce invalid cluster data or render a partial cluster depending on allocator capacity. Ignore NUL alongside invalid scalar values. If any append runs out of native capacity, remove the prefix already attached and consume the rest of the declared suffix without applying it, making delivery atomic at the cluster level. Cover NUL input and a failure after 128 accepted suffix codepoints. --- src/terminal/snapshot/grid.zig | 58 +++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/src/terminal/snapshot/grid.zig b/src/terminal/snapshot/grid.zig index 221d545f8..f9d6f6be7 100644 --- a/src/terminal/snapshot/grid.zig +++ b/src/terminal/snapshot/grid.zig @@ -1067,20 +1067,25 @@ fn decodeGraphemes( break :target .{ .row = row, .cell = cell }; }; - // Always consume every declared codepoint. Invalid scalars and - // suffixes that exceed the native capacity are dropped - // independently without affecting the rest of the grid. + // Always consume every declared codepoint. Invalid scalars and NUL are + // not meaningful grapheme suffix components and are ignored. If native + // capacity is exhausted, remove any prefix already attached so the + // cell never exposes a truncated cluster. var accept = target != null; for (0..cp_count) |_| { const cp = try io.readInt(reader, u32); if (!accept) continue; - if (!validScalar(cp)) continue; + if (cp == 0 or !validScalar(cp)) continue; page.appendGrapheme( target.?.row, target.?.cell, @intCast(cp), ) catch { + if (target.?.cell.hasGrapheme()) { + page.clearGrapheme(target.?.cell); + page.updateRowGraphemeFlag(target.?.row); + } accept = false; }; } @@ -1752,8 +1757,9 @@ test "grid drops undeliverable grapheme entries" { // Valid entry with one invalid scalar dropped from within it. try io.writeInt(&writer, u16, 0); try io.writeInt(&writer, u16, 0); - try io.writeInt(&writer, u16, 3); + try io.writeInt(&writer, u16, 4); try io.writeInt(&writer, u32, 0x0301); + try io.writeInt(&writer, u32, 0); try io.writeInt(&writer, u32, 0xD800); try io.writeInt(&writer, u32, 0x0302); // Duplicate entry for the same cell is consumed and dropped. @@ -1794,6 +1800,48 @@ test "grid drops undeliverable grapheme entries" { try testing.expect(!page.getRowAndCell(3, 0).cell.hasGrapheme()); } +test "grid drops a complete grapheme when capacity fails mid-cluster" { + const testing = std.testing; + var page = try TerminalPage.init(.{ + .cols = 1, + .rows = 1, + .grapheme_bytes = 16, + }); + defer page.deinit(); + var style_remap = try StyleRemap.init(testing.allocator); + defer style_remap.deinit(testing.allocator); + var hyperlink_remap = try HyperlinkRemap.init(testing.allocator); + defer hyperlink_remap.deinit(testing.allocator); + + var payload: [1024]u8 = undefined; + var writer: std.Io.Writer = .fixed(&payload); + try writer.writeByte(@bitCast(Row{ .cell_width = .eight })); + try io.writeInt(&writer, u16, 1); + try io.writeInt(&writer, u64, @bitCast(Cell{ .kind = 1, .content = 'x' })); + try io.writeInt(&writer, u32, 1); + try io.writeInt(&writer, u16, 0); + try io.writeInt(&writer, u16, 0); + // BitmapAllocator rounds this capacity to 64 four-codepoint chunks. The + // 129th suffix needs 33 new chunks while the old 32-chunk slice is still + // live, forcing the append's atomic replacement allocation to fail. + try io.writeInt(&writer, u16, 129); + for (0..129) |i| try io.writeInt( + &writer, + u32, + @intCast(0x0300 + i), + ); + + var reader: std.Io.Reader = .fixed(writer.buffered()); + try decode(&page, &reader, &style_remap, &hyperlink_remap); + try page.verifyIntegrity(testing.allocator); + + const cell = page.getRowAndCell(0, 0); + try testing.expectEqual(@as(u21, 'x'), cell.cell.codepoint()); + try testing.expect(!cell.cell.hasGrapheme()); + try testing.expect(!cell.row.grapheme); + try testing.expectEqual(@as(usize, 0), page.graphemeCount()); +} + test "grid encodes rows at their narrowest width" { const testing = std.testing; var page = try TerminalPage.init(.{