diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index dc1686119..429a3633b 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -16643,7 +16643,7 @@ test "PageList resize reflow exceeds grapheme memory forcing capacity increase" const testing = std.testing; const alloc = testing.allocator; - var s = try init(alloc, .{ .cols = 2, .rows = 10, .max_size = 0 }); + var s = try init(alloc, .{ .cols = 4, .rows = 10, .max_size = 0 }); defer s.deinit(); try testing.expectEqual(@as(usize, 1), s.totalPages()); @@ -16665,44 +16665,38 @@ test "PageList resize reflow exceeds grapheme memory forcing capacity increase" try std.testing.expectEqual(s.pages.last.?, s.pages.first.?.next); } - // We use almost all grapheme alloc capacity with a grapheme in the final - // row of the first page, and do the same on the first row of the second - // page. We also mark the row as wrapped so that when we resize with more - // cols the row unwraps and we have a single row that requires almost two - // times the base grapheme alloc capacity. + // We use all grapheme alloc capacity with four maximum-sized graphemes on + // each page. The two rows form one wrapped logical line across the page + // boundary, so resizing wider moves all eight graphemes into one page and + // requires almost two times the base grapheme alloc capacity. // // This forces the reflow to increase capacity. // - // +--+ = PAGE 0 + // +----+ = PAGE 0 // : : - // | X… <- where X is a grapheme which uses almost all the capacity. - // +--+ - // +--+ = PAGE 1 - // …X | <- X here also almost hits grapheme cap. - // +--+ + // |XXXX| <- four capped graphemes in one wrapped row. + // +----+ + // +----+ = PAGE 1 + // |XXXX| <- four more capped graphemes continue the logical line. + // +----+ - // Almost hit grapheme alloc cap in bottom right of first page. - // Mark the final row as wrapped. + const suffixes: [pagepkg.grapheme_max_len]u21 = @splat('a'); + + // Fill the final row of the first page and mark it as wrapped. { const page = s.pages.first.?.page(); - const rac = page.getRowAndCell(page.size.cols - 1, page.size.rows - 1); - rac.row.wrap = true; - rac.cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = .{ .data = 'X' } }, - }; - try page.setGraphemes( - rac.row, - rac.cell, - &@as( - [ - @divFloor( - pagepkg.grapheme_bytes_default - 1, - @sizeOf(u21), - ) - ]u21, - @splat('a'), - ), + const y = page.size.rows - 1; + const row = page.getRow(y); + row.wrap = true; + + for (0..page.size.cols) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .init('X'); + try page.setGraphemes(rac.row, rac.cell, &suffixes); + } + try std.testing.expectEqual( + page.grapheme_alloc.capacityBytes(), + page.grapheme_alloc.usedBytes(page.memory), ); try std.testing.expectError( error.OutOfMemory, @@ -16714,29 +16708,17 @@ test "PageList resize reflow exceeds grapheme memory forcing capacity increase" ); } - // Almost hit grapheme alloc cap in top left of second page. - // Mark the first row as a wrap continuation. + // Fill the first row of the second page and mark it as a continuation. { const page = s.pages.last.?.page(); - const rac = page.getRowAndCell(0, 0); - rac.row.wrap = true; - rac.cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = .{ .data = 'X' } }, - }; - try page.setGraphemes( - rac.row, - rac.cell, - &@as( - [ - @divFloor( - pagepkg.grapheme_bytes_default - 1, - @sizeOf(u21), - ) - ]u21, - @splat('a'), - ), - ); + const row = page.getRow(0); + row.wrap_continuation = true; + + for (0..page.size.cols) |x| { + const rac = page.getRowAndCell(x, 0); + rac.cell.* = .init('X'); + try page.setGraphemes(rac.row, rac.cell, &suffixes); + } try std.testing.expectError( error.OutOfMemory, page.grapheme_alloc.alloc( diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 92bb9e690..36127e874 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -4864,6 +4864,29 @@ test "Terminal: zero-width character attaches to pending wrap cell" { try testing.expectEqualStrings("xå̲", str); } +test "Terminal: caps zero-width codepoints attached to one cell" { + var t = try init(testing.io, testing.allocator, .{ .cols = 2, .rows = 2 }); + defer t.deinit(testing.allocator); + + t.modes.set(.grapheme_cluster, false); + try t.print('A'); + + const initial_capacity = t.screens.active.cursor.page_pin.node.capacity().grapheme_bytes; + for (0..pagepkg.grapheme_max_len * 4) |_| try t.print(0x0301); + + const list_cell = t.screens.active.pages.getCell(.{ + .screen = .{ .x = 0, .y = 0 }, + }).?; + try testing.expectEqual( + @as(usize, pagepkg.grapheme_max_len), + list_cell.node.page().lookupGrapheme(list_cell.cell).?.len, + ); + try testing.expectEqual( + initial_capacity, + list_cell.node.capacity().grapheme_bytes, + ); +} + // https://github.com/mitchellh/ghostty/issues/1400 test "Terminal: print single very long line" { var t = try init(testing.io, testing.allocator, .{ .rows = 5, .cols = 5 }); diff --git a/src/terminal/osc.zig b/src/terminal/osc.zig index e3d323915..618777241 100644 --- a/src/terminal/osc.zig +++ b/src/terminal/osc.zig @@ -300,11 +300,20 @@ pub const Parser = struct { /// Maximum size of a "normal" OSC. pub const MAX_BUF = 2048; + /// Maximum size of an OSC that requires dynamically allocated storage. + /// OSC input is untrusted, so these captures must have a finite bound. + pub const MAX_ALLOCATING_BUF = 8 * 1024 * 1024; + /// Optional allocator used to accept data longer than MAX_BUF. /// This only applies to some commands (e.g. OSC 52) that can /// reasonably exceed MAX_BUF. alloc: ?Allocator, + /// Maximum number of bytes retained by an allocating capture. + /// This is configurable primarily so callers and tests can choose a + /// smaller policy than the default. + max_allocating_bytes: usize, + /// Current state of the parser. state: State, @@ -376,6 +385,7 @@ pub const Parser = struct { pub fn init(alloc: ?Allocator) Parser { var result: Parser = .{ .alloc = alloc, + .max_allocating_bytes = MAX_ALLOCATING_BUF, .state = .start, .capture = null, .command = .invalid, @@ -461,6 +471,7 @@ pub const Parser = struct { const Capture = struct { writer: *std.Io.Writer, backing: Backing, + max_bytes: usize, const Backing = union(enum) { fixed: std.Io.Writer, @@ -476,22 +487,50 @@ pub const Parser = struct { new.* = .{ .backing = .{ .fixed = .fixed(buf) }, .writer = &new.*.?.backing.fixed, + .max_bytes = buf.len, }; } pub inline fn allocating( new: *?Capture, alloc: Allocator, + max_bytes: usize, ) error{OutOfMemory}!void { new.* = .{ .backing = .{ .allocating = try std.Io.Writer.Allocating.initCapacity( alloc, - 2048, + @min(MAX_BUF, max_bytes), ) }, .writer = &new.*.?.backing.allocating.writer, + .max_bytes = max_bytes, }; } + /// Append one byte without permitting the backing allocation to grow + /// beyond max_bytes. Allocating.Writer normally grows super-linearly, + /// so grow it explicitly to keep the allocation itself bounded too. + pub inline fn writeByte(self: *Capture, byte: u8) error{WriteFailed}!void { + if (self.writer.buffered().len >= self.max_bytes) return error.WriteFailed; + + switch (self.backing) { + .fixed => {}, + .allocating => |*w| { + if (w.writer.end >= w.writer.buffer.len) { + const new_capacity = @min( + self.max_bytes, + @max(w.writer.buffer.len *| 2, 1), + ); + w.writer.buffer = w.allocator.realloc( + w.writer.buffer, + new_capacity, + ) catch return error.WriteFailed; + } + }, + } + + try self.writer.writeByte(byte); + } + pub fn deinit(self: *Capture) void { switch (self.backing) { .fixed => {}, @@ -535,6 +574,7 @@ pub const Parser = struct { Capture.allocating( &self.capture, alloc, + self.max_allocating_bytes, ) catch { // The allocator failed for some reason, fall back to a fixed buffer // and hope that it's big enough. @@ -554,7 +594,7 @@ pub const Parser = struct { // If a writer has been initialized, we just accumulate the rest of the // OSC sequence in the writer's buffer and skip the state machine. if (self.capture) |*cap| { - cap.writer.writeByte(c) catch |err| switch (err) { + cap.writeByte(c) catch |err| switch (err) { // We have overflowed our buffer or had some other error, set the // state to invalid so that we discard any further input. error.WriteFailed => self.state = .invalid, @@ -845,3 +885,42 @@ test { _ = parsers; _ = encoding; } + +test "Parser allocating captures have a hard limit" { + const testing = std.testing; + const prefixes = [_][]const u8{ "52;", "66;", "72;", "5522;" }; + const limit = Parser.MAX_BUF + 1; + + for (prefixes) |prefix| { + var p: Parser = .init(testing.allocator); + defer p.deinit(); + p.max_allocating_bytes = limit; + + for (prefix) |ch| p.next(ch); + for (0..limit) |_| p.next('a'); + + const cap = &p.capture.?; + try testing.expectEqual(@as(usize, limit), cap.trailing().len); + try testing.expectEqual(@as(usize, limit), cap.writer.buffer.len); + + p.next('a'); + try testing.expectEqual(Parser.State.invalid, p.state); + try testing.expectEqual(@as(usize, limit), cap.trailing().len); + try testing.expectEqual(@as(usize, limit), cap.writer.buffer.len); + } +} + +test "Parser allocating capture limit includes parser-added bytes" { + const testing = std.testing; + var p: Parser = .init(testing.allocator); + defer p.deinit(); + p.max_allocating_bytes = 4; + + for ("52;abcd") |ch| p.next(ch); + try testing.expect(p.end(null) == null); + try testing.expectEqual(Parser.State.invalid, p.state); + + const cap = &p.capture.?; + try testing.expectEqual(@as(usize, 4), cap.trailing().len); + try testing.expectEqual(@as(usize, 4), cap.writer.buffer.len); +} diff --git a/src/terminal/osc/parsers/clipboard_operation.zig b/src/terminal/osc/parsers/clipboard_operation.zig index 8d149f97c..ec8836fb0 100644 --- a/src/terminal/osc/parsers/clipboard_operation.zig +++ b/src/terminal/osc/parsers/clipboard_operation.zig @@ -12,7 +12,7 @@ pub fn parse(parser: *Parser, _: ?u8) ?*Command { parser.state = .invalid; return null; }; - cap.writer.writeByte(0) catch { + cap.writeByte(0) catch { parser.state = .invalid; return null; }; diff --git a/src/terminal/osc/parsers/kitty_text_sizing.zig b/src/terminal/osc/parsers/kitty_text_sizing.zig index 94f271005..a60ed5d75 100644 --- a/src/terminal/osc/parsers/kitty_text_sizing.zig +++ b/src/terminal/osc/parsers/kitty_text_sizing.zig @@ -77,7 +77,7 @@ pub fn parse(parser: *Parser, _: ?u8) ?*Command { }; // Write a NUL byte to ensure that `text` is NUL-terminated - cap.writer.writeByte(0) catch { + cap.writeByte(0) catch { parser.state = .invalid; return null; }; diff --git a/src/terminal/page.zig b/src/terminal/page.zig index a06a1dae7..c2814ae98 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -89,6 +89,7 @@ const AllocWindows = struct { /// for alignment. const grapheme_chunk_len = 4; const grapheme_chunk = grapheme_chunk_len * @sizeOf(u21); +pub const grapheme_max_len = 64; const GraphemeAlloc = BitmapAllocator(grapheme_chunk); const grapheme_count_default = GraphemeAlloc.bitmap_bit_size; pub const grapheme_bytes_default = grapheme_count_default * grapheme_chunk; @@ -1502,7 +1503,8 @@ pub const Page = struct { } /// Set the graphemes for the given cell. This asserts that the cell - /// has no graphemes set, and only contains a single codepoint. + /// has no graphemes set, and only contains a single codepoint. Input + /// beyond grapheme_max_len is ignored. pub inline fn setGraphemes( self: *Page, row: *Row, @@ -1516,14 +1518,15 @@ pub const Page = struct { const cell_offset = getOffset(Cell, self.memory, cell); var map = self.grapheme_map.map(self.memory); + const stored_cps = cps[0..@min(cps.len, grapheme_max_len)]; - const slice = self.grapheme_alloc.alloc(u21, self.memory, cps.len) catch |e| { + const slice = self.grapheme_alloc.alloc(u21, self.memory, stored_cps.len) catch |e| { comptime assert(@TypeOf(e) == error{OutOfMemory}); // The grapheme alloc capacity needs to be increased. return error.GraphemeAllocOutOfMemory; }; errdefer self.grapheme_alloc.free(self.memory, slice); - @memcpy(slice, cps); + @memcpy(slice, stored_cps); map.putNoClobber(cell_offset, .{ .offset = getOffset(u21, self.memory, @ptrCast(slice.ptr)), @@ -1541,7 +1544,8 @@ pub const Page = struct { return; } - /// Append a codepoint to the given cell as a grapheme. + /// Append a codepoint to the given cell as a grapheme. Once the cell has + /// grapheme_max_len suffix codepoints, additional codepoints are ignored. pub fn appendGrapheme(self: *Page, row: *Row, cell: *Cell, cp: u21) Allocator.Error!void { defer self.assertIntegrity(); @@ -1575,6 +1579,10 @@ pub const Page = struct { const slice = map.getPtr(cell_offset).?; + // Terminal input is untrusted. In addition to bounding memory, this + // prevents repeated chunk growth and copying from becoming quadratic. + if (slice.len >= grapheme_max_len) return; + // If our slice len doesn't divide evenly by the grapheme chunk // length then we can utilize the additional chunk space. if (slice.len % grapheme_chunk_len != 0) { @@ -2831,6 +2839,49 @@ test "Page appendGrapheme larger than chunk" { } } +test "Page appendGrapheme caps codepoints per cell" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .styles = 8, + }); + defer page.deinit(); + + const rac = page.getRowAndCell(0, 0); + rac.cell.* = .init('A'); + + for (0..grapheme_max_len + 16) |i| { + try page.appendGrapheme(rac.row, rac.cell, @intCast(0x0300 + i)); + } + + const cps = page.lookupGrapheme(rac.cell).?; + try testing.expectEqual(@as(usize, grapheme_max_len), cps.len); + for (0..grapheme_max_len) |i| { + try testing.expectEqual(@as(u21, @intCast(0x0300 + i)), cps[i]); + } +} + +test "Page setGraphemes caps codepoints per cell" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .styles = 8, + }); + defer page.deinit(); + + var input: [grapheme_max_len + 16]u21 = undefined; + for (&input, 0..) |*cp, i| cp.* = @intCast(0x0300 + i); + + const rac = page.getRowAndCell(0, 0); + rac.cell.* = .init('A'); + try page.setGraphemes(rac.row, rac.cell, &input); + + try testing.expectEqual( + @as(usize, grapheme_max_len), + page.lookupGrapheme(rac.cell).?.len, + ); +} + test "Page clearGrapheme not all cells" { var page = try Page.init(.{ .cols = 10, diff --git a/src/terminal/snapshot/grid.zig b/src/terminal/snapshot/grid.zig index 4cb226516..fe5d05528 100644 --- a/src/terminal/snapshot/grid.zig +++ b/src/terminal/snapshot/grid.zig @@ -1806,9 +1806,9 @@ test "grid drops undeliverable grapheme entries" { test "grid drops a complete grapheme when capacity fails mid-cluster" { const testing = std.testing; var page = try TerminalPage.init(.{ - .cols = 1, + .cols = 4, .rows = 1, - .grapheme_bytes = 16, + .grapheme_bytes = 512, }); defer page.deinit(); var style_remap = try StyleRemap.init(testing.allocator); @@ -1819,30 +1819,38 @@ test "grid drops a complete grapheme when capacity fails mid-cluster" { 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( + try io.writeInt(&writer, u16, 4); + for (0..4) |i| try io.writeInt( &writer, - u32, - @intCast(0x0300 + i), + u64, + @bitCast(Cell{ .kind = 1, .content = @intCast('x' + i) }), ); + // BitmapAllocator rounds this capacity to 64 four-codepoint chunks. The + // first three cells consume 34 chunks. On the fourth cell, appending the + // 61st suffix needs 16 new chunks while the old 15-chunk slice is still + // live, exceeding capacity and forcing the complete suffix to be dropped. + try io.writeInt(&writer, u32, 4); + for ([_]u16{ 64, 64, 8, 61 }, 0..) |count, x| { + try io.writeInt(&writer, u16, 0); + try io.writeInt(&writer, u16, @intCast(x)); + try io.writeInt(&writer, u16, count); + for (0..count) |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()); + const cell = page.getRowAndCell(3, 0); + try testing.expectEqual(@as(u21, @intCast('x' + 3)), cell.cell.codepoint()); try testing.expect(!cell.cell.hasGrapheme()); - try testing.expect(!cell.row.grapheme); - try testing.expectEqual(@as(usize, 0), page.graphemeCount()); + try testing.expect(cell.row.grapheme); + try testing.expectEqual(@as(usize, 3), page.graphemeCount()); } test "grid encodes rows at their narrowest width" {