From 43ec9b373b76a8c0e86a093f63aee643e01c69ff Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 30 Jul 2026 13:07:25 -0700 Subject: [PATCH] terminal/snapshot: harden hyperlink decoding, allow invalid hyperlinks for page --- src/terminal/snapshot/hyperlink.zig | 113 ++++++++++++++++------ src/terminal/snapshot/page.zig | 106 +++++++++++++++++---- src/terminal/snapshot/screen.zig | 142 ++++++++++++++++++++++++++++ 3 files changed, 316 insertions(+), 45 deletions(-) diff --git a/src/terminal/snapshot/hyperlink.zig b/src/terminal/snapshot/hyperlink.zig index 71b27558c..8a7a7d920 100644 --- a/src/terminal/snapshot/hyperlink.zig +++ b/src/terminal/snapshot/hyperlink.zig @@ -7,8 +7,11 @@ //! Indexing and ordering are properties of the containing record rather than //! this codec. //! -//! IDs and URIs are arbitrary byte strings. Their lengths are retained on the -//! wire; they are not NUL-terminated and need not contain UTF-8. +//! URIs and explicit IDs are non-empty arbitrary byte strings. Their lengths +//! are retained on the wire; they are not NUL-terminated and need not contain +//! UTF-8. Native page storage requires every allocated hyperlink string to +//! contain at least one byte, so standalone decoding rejects zero lengths and +//! PAGE decoding consumes but ignores the invalid table entry. //! //! All integers are unsigned and little-endian. //! @@ -56,6 +59,12 @@ pub const EncodeError = std.Io.Writer.Error; pub const DecodeError = std.Io.Reader.Error || Allocator.Error || error{ /// The hyperlink kind is not defined by snapshot version 1. InvalidKind, + + /// A native hyperlink URI must contain at least one byte. + InvalidUri, + + /// A native explicit hyperlink ID must contain at least one byte. + InvalidExplicitId, }; /// Errors possible while decoding directly into a native page. @@ -106,6 +115,8 @@ pub fn decode( .implicit => implicit: { const id = try io.readInt(reader, u32); const uri_len: usize = @intCast(try io.readInt(reader, u32)); + if (uri_len == 0) return error.InvalidUri; + const uri = try alloc.alloc(u8, uri_len); errdefer alloc.free(uri); try reader.readSliceAll(uri); @@ -118,11 +129,15 @@ pub fn decode( .explicit => explicit: { const id_len: usize = @intCast(try io.readInt(reader, u32)); + if (id_len == 0) return error.InvalidExplicitId; + const id = try alloc.alloc(u8, id_len); errdefer alloc.free(id); try reader.readSliceAll(id); const uri_len: usize = @intCast(try io.readInt(reader, u32)); + if (uri_len == 0) return error.InvalidUri; + const uri = try alloc.alloc(u8, uri_len); errdefer alloc.free(uri); try reader.readSliceAll(uri); @@ -139,7 +154,9 @@ pub fn decode( /// /// Explicit ID and URI bytes are read into the page string allocator and the /// completed entry is inserted into the page hyperlink set. The returned ID is -/// the native ID assigned by the destination page. +/// the native ID assigned by the destination page. Zero is returned when the +/// encoded URI or explicit ID is empty because native page storage cannot +/// represent empty hyperlink strings. pub fn decodePage( page: *terminal_page.Page, reader: *std.Io.Reader, @@ -152,6 +169,8 @@ pub fn decodePage( .implicit => implicit: { const id = try io.readInt(reader, u32); const uri_len: usize = @intCast(try io.readInt(reader, u32)); + if (uri_len == 0) return 0; + const uri = try decodePageString( page, reader, @@ -166,17 +185,33 @@ pub fn decodePage( .explicit => explicit: { const id_len: usize = @intCast(try io.readInt(reader, u32)); + if (id_len == 0) { + const uri_len: usize = @intCast( + try io.readInt(reader, u32), + ); + try reader.discardAll(uri_len); + return 0; + } + const id = try decodePageString( page, reader, id_len, ); - errdefer if (id.len > 0) page.string_alloc.free( + errdefer page.string_alloc.free( page.memory, id.slice(page.memory), ); const uri_len: usize = @intCast(try io.readInt(reader, u32)); + if (uri_len == 0) { + page.string_alloc.free( + page.memory, + id.slice(page.memory), + ); + return 0; + } + const uri = try decodePageString( page, reader, @@ -214,7 +249,7 @@ fn decodePageString( reader: *std.Io.Reader, len: usize, ) (std.Io.Reader.Error || error{StringsOutOfMemory})!terminal_size.Offset(u8).Slice { - if (len == 0) return .{}; + std.debug.assert(len > 0); // Allocate space for the string and read directly into it. const value = page.string_alloc.alloc( @@ -267,37 +302,57 @@ test "golden explicit encoding" { ); } -test "empty strings round trip" { - const values: [2]terminal_hyperlink.Hyperlink = .{ +test "decode rejects empty strings" { + const cases = [_]struct { + fixture: []const u8, + expected: anyerror, + }{ .{ - .id = .{ .implicit = 0 }, - .uri = "", + .fixture = "\x01\x04\x03\x02\x01\x00\x00\x00\x00", + .expected = error.InvalidUri, }, .{ - .id = .{ .explicit = "" }, - .uri = "", + .fixture = "\x02\x00\x00\x00\x00", + .expected = error.InvalidExplicitId, + }, + .{ + .fixture = "\x02\x02\x00\x00\x00id\x00\x00\x00\x00", + .expected = error.InvalidUri, }, }; - for (values) |value| { - var encoded: [9]u8 = undefined; - var writer: std.Io.Writer = .fixed(&encoded); - try encode(value, &writer); + for (cases) |case| { + var reader: std.Io.Reader = .fixed(case.fixture); + try std.testing.expectError( + case.expected, + decode(&reader, std.testing.allocator), + ); + } +} - var reader: std.Io.Reader = .fixed(writer.buffered()); - const decoded = try decode(&reader, std.testing.allocator); - defer decoded.deinit(std.testing.allocator); - try std.testing.expectEqualStrings("", decoded.uri); - switch (value.id) { - .implicit => |id| try std.testing.expectEqual( - id, - decoded.id.implicit, - ), - .explicit => |id| try std.testing.expectEqualStrings( - id, - decoded.id.explicit, - ), - } +test "decodePage ignores empty strings" { + const fixtures = [_][]const u8{ + "\x01\x04\x03\x02\x01\x00\x00\x00\x00", + "\x02\x00\x00\x00\x00\x03\x00\x00\x00uri", + "\x02\x02\x00\x00\x00id\x00\x00\x00\x00", + }; + + for (fixtures) |fixture| { + var page = try terminal_page.Page.init(.{ + .cols = 1, + .rows = 1, + .hyperlink_bytes = 512, + .string_bytes = 16, + }); + defer page.deinit(); + + var reader: std.Io.Reader = .fixed(fixture); + try std.testing.expectEqual( + @as(terminal_hyperlink.Id, 0), + try decodePage(&page, &reader), + ); + try std.testing.expectEqual(@as(usize, 0), page.hyperlink_set.count()); + try std.testing.expectError(error.EndOfStream, reader.takeByte()); } } diff --git a/src/terminal/snapshot/page.zig b/src/terminal/snapshot/page.zig index f61d1a472..d1207f2a3 100644 --- a/src/terminal/snapshot/page.zig +++ b/src/terminal/snapshot/page.zig @@ -373,6 +373,10 @@ fn decodePayloadBody( error.EndOfStream => return error.EndOfStream, error.ReadFailed => return error.ReadFailed, }; + + // Zero records an ignored table entry. Keeping that mapping preserves + // encoded-ID uniqueness while grid decoding treats every reference to + // the invalid hyperlink as no hyperlink. hyperlink_remap.putAssumeCapacityNoClobber( native_id, decoded_id, @@ -1104,21 +1108,21 @@ test "decode accepts unordered sparse hyperlink IDs" { .style_capacity = 0, .hyperlink_capacity_bytes = 512, .grapheme_capacity_bytes = 0, - .string_capacity_bytes = 0, + .string_capacity_bytes = 6, }; const first: TerminalHyperlink = .{ .id = .{ .implicit = 1 }, - .uri = "", + .uri = "one", }; const second: TerminalHyperlink = .{ .id = .{ .implicit = 2 }, - .uri = "", + .uri = "two", }; var encoded: [ Header.len + - 2 * 11 + + 2 * 14 + 1 + 16 ]u8 = undefined; @@ -1406,7 +1410,7 @@ test "decode rejects duplicate and default style entries" { ); } -test "decode rejects duplicate hyperlinks with empty strings" { +test "decode rejects duplicate hyperlinks" { const header: Header = .{ .columns = 1, .rows = 1, @@ -1415,24 +1419,94 @@ test "decode rejects duplicate hyperlinks with empty strings" { .style_capacity = 0, .hyperlink_capacity_bytes = 512, .grapheme_capacity_bytes = 0, - .string_capacity_bytes = 0, + .string_capacity_bytes = 16, }; const duplicate: TerminalHyperlink = .{ - .id = .{ .explicit = "" }, - .uri = "", + .id = .{ .explicit = "id" }, + .uri = "uri", }; - var encoded: [Header.len + 22]u8 = undefined; - var writer: std.Io.Writer = .fixed(&encoded); - try header.encode(&writer); - try io.writeInt(&writer, TerminalHyperlinkId, 1); - try hyperlink.encode(duplicate, &writer); - try io.writeInt(&writer, TerminalHyperlinkId, 3); - try hyperlink.encode(duplicate, &writer); + var encoded: std.Io.Writer.Allocating = .init(std.testing.allocator); + defer encoded.deinit(); + try header.encode(&encoded.writer); + try io.writeInt(&encoded.writer, TerminalHyperlinkId, 1); + try hyperlink.encode(duplicate, &encoded.writer); + try io.writeInt(&encoded.writer, TerminalHyperlinkId, 3); + try hyperlink.encode(duplicate, &encoded.writer); - var reader: std.Io.Reader = .fixed(writer.buffered()); + var reader: std.Io.Reader = .fixed(encoded.written()); try std.testing.expectError( error.DuplicateHyperlink, decodePayload(&reader, std.testing.allocator), ); } + +test "decode ignores empty hyperlink strings" { + const header: Header = .{ + .columns = 1, + .rows = 1, + .style_count = 0, + .hyperlink_count = 1, + .style_capacity = 0, + .hyperlink_capacity_bytes = 512, + .grapheme_capacity_bytes = 0, + .string_capacity_bytes = 16, + }; + const cases = [_]struct { + value: TerminalHyperlink, + }{ + .{ + .value = .{ + .id = .{ .implicit = 1 }, + .uri = "", + }, + }, + .{ + .value = .{ + .id = .{ .explicit = "" }, + .uri = "uri", + }, + }, + .{ + .value = .{ + .id = .{ .explicit = "id" }, + .uri = "", + }, + }, + }; + + for (cases) |case| { + var encoded: std.Io.Writer.Allocating = .init( + std.testing.allocator, + ); + defer encoded.deinit(); + try header.encode(&encoded.writer); + try io.writeInt(&encoded.writer, TerminalHyperlinkId, 1); + try hyperlink.encode(case.value, &encoded.writer); + + // The cell refers to the ignored table entry. Its absent native + // remapping must degrade to no hyperlink while preserving the cell. + try encoded.writer.writeByte(0); + try encoded.writer.writeAll(&.{ 0, 0, 0, 0 }); + try io.writeInt(&encoded.writer, TerminalStyleId, 0); + try io.writeInt(&encoded.writer, TerminalHyperlinkId, 1); + try io.writeInt(&encoded.writer, u32, 'A'); + try io.writeInt(&encoded.writer, u32, 0); + + 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.hyperlink_set.count(), + ); + const cell = decoded.getRowAndCell(0, 0).cell; + try std.testing.expectEqual(@as(u21, 'A'), cell.codepoint()); + try std.testing.expect(!cell.hyperlink); + try std.testing.expectEqual(null, decoded.lookupHyperlink(cell)); + } +} diff --git a/src/terminal/snapshot/screen.zig b/src/terminal/snapshot/screen.zig index 8b9d1215f..c61191337 100644 --- a/src/terminal/snapshot/screen.zig +++ b/src/terminal/snapshot/screen.zig @@ -2148,6 +2148,34 @@ test "cursor hyperlink rejects invalid kind and every truncation" { ), ); + const empty_cases = [_]struct { + fixture: []const u8, + expected: anyerror, + }{ + .{ + .fixture = "\x01\x04\x03\x02\x01\x00\x00\x00\x00", + .expected = error.InvalidUri, + }, + .{ + .fixture = "\x02\x00\x00\x00\x00", + .expected = error.InvalidExplicitId, + }, + .{ + .fixture = "\x02\x02\x00\x00\x00id\x00\x00\x00\x00", + .expected = error.InvalidUri, + }, + }; + for (empty_cases) |case| { + var reader: std.Io.Reader = .fixed(case.fixture); + try std.testing.expectError( + case.expected, + decodeCursorHyperlink( + &reader, + std.testing.allocator, + ), + ); + } + const fixtures = .{ "\x01\x04\x03\x02\x01\x03\x00\x00\x00uri", "\x02\x02\x00\x00\x00id\x03\x00\x00\x00uri", @@ -2165,3 +2193,117 @@ test "cursor hyperlink rejects invalid kind and every truncation" { } } } + +test "SCREEN decode rejects an empty cursor hyperlink URI" { + var destination: std.Io.Writer.Allocating = .init( + std.testing.allocator, + ); + defer destination.deinit(); + + var header = testHeader(); + header.key = .primary; + header.page_count = 1; + header.cursor_x = 0; + header.cursor_y = 0; + header.cursor_flags.pending_wrap = false; + header.saved_cursor_present = false; + + var record_writer = try record.Writer.init(&destination, .screen); + try header.encode(record_writer.payloadWriter()); + try hyperlink.encode(.{ + .id = .{ .implicit = 1 }, + .uri = "", + }, record_writer.payloadWriter()); + try record_writer.finish(); + + var source: std.Io.Reader = .fixed(destination.written()); + try std.testing.expectError( + error.InvalidUri, + decode( + &source, + std.testing.io, + std.testing.allocator, + .{ .cols = 1, .rows = 1 }, + ), + ); +} + +test "SCREEN decode ignores a PAGE with an empty hyperlink URI" { + var destination: std.Io.Writer.Allocating = .init( + std.testing.allocator, + ); + defer destination.deinit(); + + var header = testHeader(); + header.key = .primary; + header.page_count = 1; + header.cursor_x = 0; + header.cursor_y = 0; + header.cursor_flags.pending_wrap = false; + header.saved_cursor_present = false; + + var screen_writer = try record.Writer.init(&destination, .screen); + try header.encode(screen_writer.payloadWriter()); + try screen_writer.payloadWriter().writeByte(0); + try screen_writer.finish(); + + const page_header: page.Header = .{ + .columns = 1, + .rows = 1, + .style_count = 0, + .hyperlink_count = 1, + .style_capacity = 16, + .hyperlink_capacity_bytes = 512, + .grapheme_capacity_bytes = 0, + .string_capacity_bytes = 0, + }; + var page_writer = try record.Writer.init(&destination, .page); + try page_header.encode(page_writer.payloadWriter()); + try io.writeInt( + page_writer.payloadWriter(), + terminal_hyperlink.Id, + 1, + ); + try hyperlink.encode(.{ + .id = .{ .implicit = 1 }, + .uri = "", + }, page_writer.payloadWriter()); + + // One narrow codepoint cell refers to the hyperlink table entry above. + // Since that entry is ignored, the cell must restore without a hyperlink. + try page_writer.payloadWriter().writeByte(0); + try page_writer.payloadWriter().writeAll(&.{ 0, 0, 0, 0 }); + try io.writeInt( + page_writer.payloadWriter(), + terminal_style.Id, + 0, + ); + try io.writeInt( + page_writer.payloadWriter(), + terminal_hyperlink.Id, + 1, + ); + try io.writeInt(page_writer.payloadWriter(), u32, 'A'); + try io.writeInt(page_writer.payloadWriter(), u32, 0); + try page_writer.finish(); + + var source: std.Io.Reader = .fixed(destination.written()); + var decoded = try decode( + &source, + std.testing.io, + std.testing.allocator, + .{ .cols = 1, .rows = 1 }, + ); + defer decoded.deinit(); + + try std.testing.expectEqual( + @as(u21, 'A'), + decoded.screen.cursor.page_cell.codepoint(), + ); + try std.testing.expect(!decoded.screen.cursor.page_cell.hyperlink); + + // Resizing copies the restored cells into a wider page. The ignored + // hyperlink must not leave a zero-length PageEntry to duplicate later. + try decoded.screen.resize(.{ .cols = 2, .rows = 1 }); + try std.testing.expect(!decoded.screen.cursor.page_cell.hyperlink); +}