diff --git a/include/ghostty/vt/terminal.h b/include/ghostty/vt/terminal.h index e015369e3..172674bf5 100644 --- a/include/ghostty/vt/terminal.h +++ b/include/ghostty/vt/terminal.h @@ -1529,16 +1529,15 @@ typedef enum GHOSTTY_ENUM_TYPED { * when a transaction begins; an in-flight transaction keeps the limit * it started with. * - * Text data (text/* MIME types and the legacy X11 text names) beyond - * the limit is truncated at a UTF-8 boundary and the write still - * completes with DONE. Non-text data beyond the limit fails the whole - * transaction with EFBIG and nothing reaches the clipboard write - * callback, since binary formats are corrupted by truncation. + * Data beyond the limit fails the whole transaction with EFBIG. The + * transaction is discarded, later write-related packets are ignored + * until a new write begins, and nothing reaches the clipboard write + * callback. * * Transactions are buffered in memory, so this limit bounds how much * memory a single write can make the terminal allocate. Pass SIZE_MAX * to remove the limit. A NULL value pointer reverts to the built-in - * default of 32MiB. + * default of 64MiB, the minimum required by the protocol. * * This limit doesn't apply to OSC 52 writes, which are bounded by the * maximum length of an escape sequence instead. diff --git a/src/config/Config.zig b/src/config/Config.zig index 51a47c994..f40614a39 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2459,29 +2459,26 @@ keybind: Keybinds = .{}, /// This doesn't apply to OSC 52, which is limited by the maximum length /// of an escape sequence hardcoded into Ghostty for now. /// -/// Text data (`text/*` MIME types) beyond this limit is truncated at -/// a UTF-8 boundary and the write still completes successfully with -/// the clipped text. -/// -/// Non-text data beyond the limit instead fails the entire write with an -/// `EFBIG` status and leaves the clipboard untouched, since binary formats -/// such as images are corrupted by truncation. +/// Data beyond the limit fails the entire write with an `EFBIG` status, +/// discards the transaction, and leaves the clipboard untouched. Later +/// write-related packets are ignored until a new write begins. /// /// The data is buffered in memory while the write is in progress, so /// this limit bounds how much memory a program can make Ghostty /// allocate per write. A future improvement will attempt to spool large /// writes to disk. /// -/// Set this to `unlimited` to remove the limit, allowing writes bounded only -/// by available memory. A value of `0` truncates every text write to empty -/// contents and rejects every non-text write. To reject clipboard writes -/// entirely, use `clipboard-write = deny` instead. +/// The default is 64 MiB, the minimum a conforming implementation must +/// accept. Set this to `unlimited` to remove the limit, allowing writes +/// bounded only by available memory. A value of `0` rejects every non-empty +/// write. To reject clipboard writes entirely, use `clipboard-write = deny` +/// instead. /// /// This can be changed at runtime and applies to writes that begin /// after the change. /// /// Available since: 1.4.0 -@"clipboard-write-limit-bytes": Limit(usize, 32 * 1024 * 1024) = .default, +@"clipboard-write-limit-bytes": Limit(usize, 64 * 1024 * 1024) = .default, /// Trims trailing whitespace on data that is copied to the clipboard. This does /// not affect data sent to the clipboard via `clipboard-write`. This only @@ -11072,7 +11069,7 @@ test "clipboard write limit" { var cfg = try Config.default(alloc); defer cfg.deinit(); try testing.expectEqual( - @as(usize, 32 * 1024 * 1024), + @as(usize, 64 * 1024 * 1024), cfg.@"clipboard-write-limit-bytes".value, ); diff --git a/src/terminal/c/terminal.zig b/src/terminal/c/terminal.zig index 00bb3b060..70d7ab849 100644 --- a/src/terminal/c/terminal.zig +++ b/src/terminal/c/terminal.zig @@ -5106,8 +5106,8 @@ test "set clipboard write max bytes" { try testing.expectEqual(Result.success, get(t, .clipboard_write_max_bytes, @ptrCast(&max))); try testing.expectEqual(@as(usize, kitty_clipboard.max_write_size), max); - // Set a tiny limit; an oversized non-text write fails with EFBIG - // and never reaches the callback. + // Set a tiny limit; an oversized text write fails with EFBIG and + // never reaches the callback. const limit: usize = 4; try testing.expectEqual(Result.success, set(t, .clipboard_write_max_bytes, @ptrCast(&limit))); try testing.expectEqual(Result.success, get(t, .clipboard_write_max_bytes, @ptrCast(&max))); @@ -5115,7 +5115,8 @@ test "set clipboard write max bytes" { const seqs = [_][]const u8{ "\x1B]5522;type=write:id=c1\x1B\\", - "\x1B]5522;type=wdata:mime=aW1hZ2UvcG5n;SGVsbG9Xb3JsZA==\x1B\\", // "HelloWorld" + "\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;SGVsbA==\x1B\\", // "Hell" + "\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;bw==\x1B\\", // "o" "\x1B]5522;type=wdata\x1B\\", }; for (seqs) |seq| vt_write(t, seq.ptr, seq.len); diff --git a/src/terminal/kitty/clipboard.zig b/src/terminal/kitty/clipboard.zig index 55c42fe8e..0a8aab4f2 100644 --- a/src/terminal/kitty/clipboard.zig +++ b/src/terminal/kitty/clipboard.zig @@ -21,11 +21,6 @@ //! * A `type=write` silently replaces any in-flight transaction. A //! commit (`type=wdata` without a MIME type) with no in-flight //! transaction is silently ignored. -//! * Oversized writes of text data are truncated and still complete -//! with DONE, matching kitty. Unlike kitty, oversized non-text data -//! fails the whole transaction with EFBIG instead: a truncated -//! image or other binary payload is corrupt, and the protocol has -//! no way to report partial success. //! * Responses never send a payload section for an empty payload, //! except the targets ('.') listing DATA packet which is always sent. //! diff --git a/src/terminal/kitty/clipboard_write.zig b/src/terminal/kitty/clipboard_write.zig index 7834ed631..3484d3cfc 100644 --- a/src/terminal/kitty/clipboard_write.zig +++ b/src/terminal/kitty/clipboard_write.zig @@ -18,7 +18,7 @@ const log = std.log.scoped(.kitty_clipboard); /// transaction. Embedders can override this per-transaction via /// WriteState.Options (Ghostty exposes it as the /// `clipboard-write-limit-bytes` configuration). -pub const max_write_size = 32 * 1024 * 1024; +pub const max_write_size = 64 * 1024 * 1024; /// Maximum MIME types and aliases per write transaction. pub const max_write_mimes = 64; @@ -50,11 +50,6 @@ pub const WriteState = struct { /// Index into entries currently receiving data. current: ?usize = null, - /// Set when max_size was exceeded on text data; excess data is - /// dropped but the write still completes. Exceeding the limit - /// with non-text data fails the transaction instead (see data). - truncated: bool = false, - pub const Options = struct { /// Maximum total decoded bytes accumulated by the transaction. max_size: usize = max_write_size, @@ -105,10 +100,9 @@ pub const WriteState = struct { /// Accumulate one wdata chunk carrying data for meta.mime (which /// must be non-empty; an empty mime is a commit, not data). /// - /// Returns error.TooLarge when non-text data exceeds max_size: - /// unlike text, a truncated binary payload is corrupt, and the - /// protocol has no partial-success status, so the caller must - /// fail the whole transaction (EFBIG) and abort it. + /// Returns error.TooLarge when the transaction exceeds max_size. + /// The caller must fail the whole transaction with EFBIG and abort + /// it, as required by the protocol. pub fn data( self: *WriteState, alloc: Allocator, @@ -176,57 +170,12 @@ pub const WriteState = struct { // Empty slice, do nothing. if (decoded.data.len == 0) return; - // Make sure it fits in our max size. If it does, easy append. + // The limit covers all decoded data in the transaction. Going + // over it aborts the entire write; partial clipboard contents + // must never reach the embedder. const remaining = self.max_size -| self.spool.items.len; - if (decoded.data.len <= remaining and !self.truncated) { - try self.spool.appendSlice(alloc, decoded.data); - return; - } - - // Bytes must be dropped. Only text (text/* plus the legacy X11 - // text names) remains usable after an arbitrary cut; non-text - // data would be corrupted, so the whole transaction fails. - truncatable: { - if (std.mem.startsWith(u8, meta.mime, "text/")) break :truncatable; - if (clipboard.isTextMime(meta.mime)) break :truncatable; - return error.TooLarge; - } - - // Text is truncated at a UTF-8 boundary. Once truncated, all - // further data is dropped entirely: the spool may sit below - // max_size after the boundary trim, and appending later chunks - // there would splice disjoint parts of the stream together. - if (!self.truncated) { - try self.spool.appendSlice(alloc, decoded.data[0..remaining]); - self.trimPartialSequence(); - self.truncated = true; - logTruncatedOnce(self.max_size); - } - } - - /// Trim a split multi-byte UTF-8 sequence off the spool tail so - /// truncated text never ends in a partial codepoint. - fn trimPartialSequence(self: *WriteState) void { - const idx = self.current orelse return; - const start = self.entries.items[idx].start; - const items = self.spool.items; - - // Skip trailing continuation bytes (0b10xxxxxx) to find the - // sequence's first byte; valid UTF-8 has at most 3. - var i = items.len; - while (i > start and - items.len - i < 3 and - (items[i - 1] & 0xC0) == 0x80) i -= 1; - if (i == start) return; - const lead = items[i - 1]; - if ((lead & 0xC0) == 0x80) return; - - const seq_len = std.unicode.utf8ByteSequenceLength(lead) catch return; - // Equal means the tail is a complete sequence; greater means - // invalid input (e.g. a continuation after ASCII). - if (items.len - (i - 1) < seq_len) { - self.spool.shrinkRetainingCapacity(i - 1); - } + if (decoded.data.len > remaining) return error.TooLarge; + try self.spool.appendSlice(alloc, decoded.data); } /// Register aliases from a walias packet: meta.mime is the target @@ -291,7 +240,6 @@ pub const WriteState = struct { id: []const u8, pw: []const u8, name: []const u8, - truncated: bool, contents: []const Content, pub fn deinit(self: *const Committed, alloc: Allocator) void { @@ -360,25 +308,9 @@ pub const WriteState = struct { .id = self.id, .pw = self.pw, .name = self.name, - .truncated = self.truncated, .contents = try contents.toOwnedSlice(alloc), }; } - - fn logTruncatedOnce(limit: usize) void { - // Log spam protection: this can be hit for every chunk of an - // oversized write. - const S = struct { - var logged: bool = false; - }; - if (!S.logged) { - S.logged = true; - log.warn( - "clipboard write exceeds {} bytes, truncating", - .{limit}, - ); - } - } }; test "write: basic transaction" { @@ -506,7 +438,7 @@ test "write: default limit when unset" { try testing.expectEqual(@as(usize, max_write_size), state.max_size); } -test "write: custom limit truncates but commit succeeds" { +test "write: text over limit rejects transaction" { const testing = std.testing; const alloc = testing.allocator; @@ -514,19 +446,30 @@ test "write: custom limit truncates but commit succeeds" { var state: WriteState = try .init(alloc, &begin_meta, .{ .max_size = 8 }); defer state.deinit(alloc); - // First chunk crosses the limit mid-chunk, second is entirely - // beyond it and fully dropped. - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG9Xb3JsZA=="); // "HelloWorld" - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "IQ=="); // "!" - - const committed = try state.commit(alloc); - defer committed.deinit(alloc); - try testing.expect(committed.truncated); - try testing.expectEqual(@as(usize, 1), committed.contents.len); - try testing.expectEqualStrings("HelloWor", committed.contents[0].data); + try testing.expectError(error.TooLarge, state.data( + alloc, + &.{ .op = .wdata, .mime = "text/plain" }, + "SGVsbG9Xb3JsZA==", // "HelloWorld" + )); } -test "write: truncation cuts at utf8 boundary" { +test "write: text crossing limit across chunks rejects transaction" { + const testing = std.testing; + const alloc = testing.allocator; + + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{ .max_size = 8 }); + defer state.deinit(alloc); + + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG8="); // "Hello" + try testing.expectError(error.TooLarge, state.data( + alloc, + &.{ .op = .wdata, .mime = "text/plain" }, + "V29ybGQ=", // "World" + )); +} + +test "write: data exactly at limit is accepted" { const testing = std.testing; const alloc = testing.allocator; @@ -534,33 +477,11 @@ test "write: truncation cuts at utf8 boundary" { var state: WriteState = try .init(alloc, &begin_meta, .{ .max_size = 5 }); defer state.deinit(alloc); - // "abcdé": the raw cut at 5 bytes would split é (0xC3 0xA9). - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "YWJjZMOp"); + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG8="); // "Hello" const committed = try state.commit(alloc); defer committed.deinit(alloc); - try testing.expect(committed.truncated); - try testing.expectEqualStrings("abcd", committed.contents[0].data); -} - -test "write: truncation cuts utf8 sequence split across chunks" { - const testing = std.testing; - const alloc = testing.allocator; - - const begin_meta: Metadata = .{ .op = .write }; - var state: WriteState = try .init(alloc, &begin_meta, .{ .max_size = 5 }); - defer state.deinit(alloc); - - // "abcd" + 0xC3 exactly fills the limit with a dangling lead - // byte... - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "YWJjZMM="); - // ...whose continuation (0xA9) arrives in the next chunk. - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "qSE="); - - const committed = try state.commit(alloc); - defer committed.deinit(alloc); - try testing.expect(committed.truncated); - try testing.expectEqualStrings("abcd", committed.contents[0].data); + try testing.expectEqualStrings("Hello", committed.contents[0].data); } test "write: non-text under limit is unaffected" { @@ -575,7 +496,6 @@ test "write: non-text under limit is unaffected" { const committed = try state.commit(alloc); defer committed.deinit(alloc); - try testing.expect(!committed.truncated); try testing.expectEqualStrings("\x89PNG", committed.contents[0].data); } @@ -594,7 +514,7 @@ test "write: non-text over limit rejects transaction" { )); } -test "write: non-text after text truncation rejects transaction" { +test "write: total data across MIME types is limited" { const testing = std.testing; const alloc = testing.allocator; @@ -602,15 +522,12 @@ test "write: non-text after text truncation rejects transaction" { var state: WriteState = try .init(alloc, &begin_meta, .{ .max_size = 8 }); defer state.deinit(alloc); - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG9Xb3JsZA=="); // "HelloWorld" - - // An empty chunk drops no bytes so it doesn't reject. - try state.data(alloc, &.{ .op = .wdata, .mime = "image/png" }, ""); + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG8="); // "Hello" try testing.expectError(error.TooLarge, state.data( alloc, &.{ .op = .wdata, .mime = "image/png" }, - "iVBORw==", + "V29ybGQ=", // "World" )); } diff --git a/src/terminal/stream_terminal.zig b/src/terminal/stream_terminal.zig index 3b3cf7eb2..edb67abf0 100644 --- a/src/terminal/stream_terminal.zig +++ b/src/terminal/stream_terminal.zig @@ -87,8 +87,8 @@ pub const Handler = struct { /// Maximum total decoded bytes accumulated by one Kitty clipboard /// protocol (OSC 5522) write transaction, captured when the - /// transaction begins. Text data beyond the limit is truncated; - /// non-text data fails the transaction with EFBIG. + /// transaction begins. Data beyond the limit fails the transaction + /// with EFBIG. kitty_clipboard_write_max_bytes: usize = kitty_clipboard.max_write_size, /// Called for sequence identifiers not supported by this library. @@ -1053,8 +1053,8 @@ pub const Handler = struct { return error.OutOfMemory; }, - // Non-text data over the write limit aborts the - // transaction: truncated binary data would be corrupt. + // Data over the write limit aborts the transaction and is + // reported to the client. error.TooLarge => self.kittyClipboardFinish( state, .EFBIG, @@ -4267,7 +4267,7 @@ test "kitty clipboard invalid walias payload aborts with EINVAL" { ); } -test "kitty clipboard oversized non-text write aborts with EFBIG" { +test "kitty clipboard oversized text write aborts with EFBIG" { var t: Terminal = try .init(testing.io, testing.allocator, .{ .cols = 80, .rows = 24 }); defer t.deinit(testing.allocator); @@ -4281,18 +4281,21 @@ test "kitty clipboard oversized non-text write aborts with EFBIG" { defer s.deinit(); // Shrink the limit so the test doesn't have to stream the - // default 32MiB. + // default 64MiB. s.handler.kitty_clipboard_write_max_bytes = 4; s.nextSlice("\x1B]5522;type=write:id=w\x1B\\"); - s.nextSlice("\x1B]5522;type=wdata:mime=aW1hZ2UvcG5n;SGVsbG9Xb3JsZA==\x1B\\"); + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;SGVsbA==\x1B\\"); // "Hell" + try testing.expectEqual(@as(usize, 0), S.responses_len); + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;bw==\x1B\\"); // "o" try testing.expectEqualStrings( "\x1B]5522;type=write:status=EFBIG:id=w\x1B\\", S.responseSlice(), ); try testing.expect(!s.handler.semantic_failure); - // The transaction is gone: a commit does nothing further. + // The transaction is gone: later data and the commit do nothing. + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;IQ==\x1B\\"); s.nextSlice("\x1B]5522;type=wdata\x1B\\"); try testing.expectEqual(@as(usize, 0), S.write_count); try testing.expectEqualStrings( diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 356ef275e..c0e9e6113 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -52,7 +52,7 @@ pub const StreamHandler = struct { clipboard_write: configpkg.ClipboardAccess, /// Maximum total decoded bytes per Kitty clipboard protocol - /// (OSC 5522) write transaction; data beyond this is truncated. + /// (OSC 5522) write transaction; exceeding it aborts with EFBIG. clipboard_write_limit: usize, //--------------------------------------------------------------- @@ -1218,8 +1218,8 @@ pub const StreamHandler = struct { return error.OutOfMemory; }, - // Non-text data over the write limit aborts the - // transaction: truncated binary data would be corrupt. + // Data over the write limit aborts the transaction and is + // reported to the client. error.TooLarge => try self.kittyClipboardWriteFinish( state, .EFBIG, @@ -1870,3 +1870,62 @@ test "kitty clipboard read: targets-only never consumes a one-time grant" { try testing.expect(handler.kittyClipboardReadGranted("otp", 1)); try testing.expect(!handler.kittyClipboardReadGranted("otp", 1)); } + +test "kitty clipboard write: oversized text replies EFBIG" { + const testing = std.testing; + + var mailbox = try termio.Mailbox.initSPSC(testing.allocator); + defer mailbox.deinit(testing.allocator); + + var mutex: std.Io.Mutex = .init; + mutex.lockUncancelable(global.io()); + defer mutex.unlock(global.io()); + + var renderer_state: renderer.State = .{ + .mutex = &mutex, + .terminal = undefined, + }; + var handler: StreamHandler = undefined; + handler.alloc = testing.allocator; + handler.termio_mailbox = &mailbox; + handler.renderer_state = &renderer_state; + handler.clipboard_write = .allow; + handler.clipboard_write_limit = 4; + handler.kitty_clipboard_write = null; + defer handler.kittyClipboardWriteAbort(); + + const begin: terminal.kitty.clipboard.Metadata = .{ + .op = .write, + .id = "macos", + }; + try handler.kittyClipboardWriteBegin(&begin, .st); + const state = handler.kitty_clipboard_write.?; + try state.data( + testing.allocator, + &.{ .op = .wdata, .mime = "text/plain" }, + "SGVsbA==", // "Hell" + ); + try testing.expectError(error.TooLarge, state.data( + testing.allocator, + &.{ .op = .wdata, .mime = "text/plain" }, + "bw==", // "o" + )); + try handler.kittyClipboardWriteFinish(state, .EFBIG, .st); + try testing.expect(handler.kitty_clipboard_write == null); + + const response = mailbox.spsc.queue.pop(global.io()); + try testing.expect(response != null); + const msg = response.?; + defer msg.deinit(); + switch (msg) { + .write_alloc => |v| try testing.expectEqualStrings( + "\x1B]5522;type=write:status=EFBIG:id=macos\x1B\\", + v.data, + ), + else => try testing.expect(false), + } + + // Teardown leaves no transaction that could be committed and + // forwarded to the macOS clipboard path. + try testing.expect(mailbox.spsc.queue.pop(global.io()) == null); +}