diff --git a/src/Surface.zig b/src/Surface.zig index fefd7accd..8eddc14c8 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -22,6 +22,7 @@ const Allocator = std.mem.Allocator; const ArenaAllocator = std.heap.ArenaAllocator; const global = @import("global.zig"); const oni = @import("oniguruma"); +const simd = @import("simd/main.zig"); const crash = @import("crash/main.zig"); const unicode = @import("unicode/main.zig"); const rendererpkg = @import("renderer.zig"); @@ -2194,34 +2195,18 @@ fn clipboardWrite(self: *const Surface, data: []const u8, loc: apprt.Clipboard) return; } - const dec = std.base64.standard.Decoder; - - // Build buffer - const size = dec.calcSizeForSlice(data) catch |err| switch (err) { - error.InvalidPadding => { - log.info("application sent invalid base64 data for OSC 52", .{}); - return; - }, - - // Should not be reachable but don't want to risk it. - else => return, - }; - var buf = try self.alloc.allocSentinel(u8, size, 0); + // Decode with the SIMD decoder, strict per the Kitty clipboard + // spec that also governs OSC 52 base64 handling: a request with + // characters outside the base64 alphabet is discarded entirely + // (never partially decoded), while a missing-padding tail is + // tolerated since OSC 52 has no way to report errors. + var buf = try self.alloc.allocSentinel(u8, simd.base64.maxLen(data), 0); defer self.alloc.free(buf); - buf[buf.len] = 0; - - // Decode - dec.decode(buf, data) catch |err| switch (err) { - // Ignore this. It is possible to actually have valid data and - // get this error, so we allow it. - error.InvalidPadding => {}, - - else => { - log.info("application sent invalid base64 data for OSC 52", .{}); - return; - }, + const decoded = simd.base64.decodeStrict(data, buf, .optional) catch { + log.info("application sent invalid base64 data for OSC 52", .{}); + return; }; - assert(buf[buf.len] == 0); + buf[decoded.len] = 0; // When clipboard-write is "ask" a prompt is displayed to the user asking // them to confirm the clipboard access. Each app runtime handles this @@ -2229,7 +2214,7 @@ fn clipboardWrite(self: *const Surface, data: []const u8, loc: apprt.Clipboard) const confirm = self.config.clipboard_write == .ask; self.rt_surface.setClipboard(loc, &.{.{ .mime = "text/plain", - .data = buf, + .data = buf[0..decoded.len :0], }}, confirm) catch |err| { log.err("error setting clipboard string err={}", .{err}); return; diff --git a/src/simd/base64.zig b/src/simd/base64.zig index a0ba45ea9..b86b71a65 100644 --- a/src/simd/base64.zig +++ b/src/simd/base64.zig @@ -1,7 +1,8 @@ const std = @import("std"); const options = @import("build_options"); const assert = @import("../quirks.zig").inlineAssert; -const scalar_decoder = @import("base64_scalar.zig").scalar_decoder; +const base64_scalar = @import("base64_scalar.zig"); +const scalar_decoder = base64_scalar.scalar_decoder; const log = std.log.scoped(.simd_base64); @@ -58,6 +59,165 @@ fn scalarInput(input: []const u8) []const u8 { return input[0..end]; } +/// Whether strict decoding requires the input to be padded to a +/// multiple of four bytes (RFC 4648 section 3.2). The Kitty clipboard +/// protocol requires padding; the legacy OSC 52 protocol tolerates a +/// missing-padding tail because it has no way to report errors to the +/// client. +pub const Padding = enum { required, optional }; + +/// Decode strict RFC 4648 standard-alphabet base64: characters outside +/// the alphabet (including whitespace) and misplaced padding are +/// errors rather than being skipped, and padding is validated per the +/// given requirement. This is the decoding the Kitty clipboard +/// protocol specifies: +/// https://sw.kovidgoyal.net/kitty/clipboard/#encoding-of-payloads +/// +/// The output must be at least maxLen(input) bytes. +pub fn decodeStrict( + input: []const u8, + output: []u8, + padding: Padding, +) error{Base64Invalid}![]const u8 { + // Padding can only be a suffix of at most two bytes; any '=' + // elsewhere is rejected by the underlying decode. + var pad: usize = 0; + if (input.len > 0 and input[input.len - 1] == '=') pad += 1; + if (input.len > 1 and input[input.len - 2] == '=') pad += 1; + + switch (padding) { + .required => if (input.len % 4 != 0) return error.Base64Invalid, + // Present padding must still complete a four byte group; only + // fully absent padding is tolerated. A single leftover byte + // can never carry a decodable value. + .optional => if (pad > 0) { + if (input.len % 4 != 0) return error.Base64Invalid; + } else if (input.len % 4 == 1) return error.Base64Invalid, + } + + // The permissive decode already rejects every invalid character + // except whitespace, which simdutf silently skips. Skipped + // characters make the decoded length fall short of the exact + // length the input length implies, so comparing the two rejects + // whitespace without a separate validation pass over the input. + const decoded = decode(input, output) catch return error.Base64Invalid; + const expected = switch (input.len % 4) { + 0 => input.len / 4 * 3 - pad, + 2 => input.len / 4 * 3 + 1, + 3 => input.len / 4 * 3 + 2, + else => unreachable, // rejected above + }; + if (decoded.len != expected) return error.Base64Invalid; + return decoded; +} + +/// A streaming strict base64 decoder for one logical payload split +/// across multiple chunks at arbitrary byte boundaries (e.g. the Kitty +/// clipboard protocol's wdata packets): the concatenation of the fed +/// chunks must be valid RFC 4648 standard-alphabet base64. +/// +/// Padding is terminal within a feed: a padded group followed by more +/// data in the same feed is an error. A feed that ends exactly at +/// terminal padding resets the decoder so the next feed starts a +/// fresh stream, exactly like the kitty reference implementation +/// (which resets its aklomp streaming decoder on EOF); this keeps +/// clients that pad each chunk independently working. +pub const Streaming = struct { + /// Partial group carried between feeds; a group only decodes once + /// all four of its characters have arrived. + carry: [4]u8 = undefined, + carry_len: u3 = 0, + + /// Maximum decoded bytes one feed of input can produce. + pub fn maxLen(self: *const Streaming, input: []const u8) usize { + return (self.carry_len + input.len) / 4 * 3; + } + + /// Decode the complete groups of input (with any carried bytes + /// prepended) into output, which must be at least maxLen(input) + /// bytes, and carry the remainder for the next feed. + pub fn feed( + self: *Streaming, + input: []const u8, + output: []u8, + ) error{Base64Invalid}![]const u8 { + assert(output.len >= self.maxLen(input)); + if (input.len == 0) return output[0..0]; + + var rem = input; + var written: usize = 0; + + // Complete a carried partial group first. + if (self.carry_len > 0) { + const take = @min(4 - @as(usize, self.carry_len), rem.len); + for (rem[0..take], self.carry_len..) |c, pos| { + if (!validPartialChar(pos, c)) return error.Base64Invalid; + } + @memcpy(self.carry[self.carry_len..][0..take], rem[0..take]); + self.carry_len += @intCast(take); + rem = rem[take..]; + if (self.carry_len < 4) return output[0..0]; + self.carry_len = 0; + const decoded, const padded = try group(self.carry, output); + written += decoded; + if (padded) { + if (rem.len > 0) return error.Base64Invalid; + return output[0..written]; + } + } + + // Bulk-decode all complete groups with the strict single-shot + // decode, which also enforces that padding only appears as a + // terminal suffix. Terminal padding must then end the feed. + const bulk = rem[0 .. rem.len - (rem.len % 4)]; + written += (try decodeStrict(bulk, output[written..], .required)).len; + if (bulk.len > 0 and bulk[bulk.len - 1] == '=') { + if (bulk.len != rem.len) return error.Base64Invalid; + return output[0..written]; + } + + // Carry the trailing partial group, validated eagerly so + // garbage is reported on the feed that contains it. + const tail = rem[bulk.len..]; + for (tail, 0..) |c, pos| { + if (!validPartialChar(pos, c)) return error.Base64Invalid; + } + @memcpy(self.carry[0..tail.len], tail); + self.carry_len = @intCast(tail.len); + return output[0..written]; + } + + /// Decode one complete four character group, the only place + /// padding is legal: the last two characters may be '=' ('=' in + /// the second-to-last position requires it in the last). Returns + /// the decoded length and whether the group was padded. + fn group( + g: [4]u8, + output: []u8, + ) error{Base64Invalid}!struct { usize, bool } { + const padded = g[3] == '='; + const decoded = try decodeStrict(&g, output, .required); + return .{ decoded.len, padded }; + } + + /// Whether c is valid at the given position of a partial group + /// still waiting for its remaining characters: the first two + /// positions must be alphabet characters and only the last two + /// may open the padding suffix. + fn validPartialChar(pos: usize, c: u8) bool { + return base64_scalar.isAlphabetChar(c) or (pos >= 2 and c == '='); + } + + /// Finish the stream: the concatenation of the fed chunks must + /// have formed complete groups, so a carried partial group is an + /// error (the stream was not correctly padded). The decoder is + /// ready for a fresh stream afterwards either way. + pub fn finish(self: *Streaming) error{Base64Invalid}!void { + defer self.* = .{}; + if (self.carry_len != 0) return error.Base64Invalid; + } +}; + // base64.cpp extern "c" fn ghostty_simd_base64_max_length( input: [*]const u8, @@ -93,3 +253,190 @@ test "base64 decode" { const str = try decode(input, output); try testing.expectEqualStrings("hello world", str); } + +test "base64 strict decode valid" { + const testing = std.testing; + var output: [128]u8 = undefined; + + const cases = [_]struct { input: []const u8, expect: []const u8 }{ + .{ .input = "", .expect = "" }, + .{ .input = "aGVsbG8gd29ybGQ=", .expect = "hello world" }, + .{ .input = "bGlnaHQgdw==", .expect = "light w" }, + .{ .input = "Zm9vYmFy", .expect = "foobar" }, + // Every alphabet character in one input. + .{ + .input = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/", + .expect = "\x00\x10\x83\x10\x51\x87\x20\x92\x8b\x30\xd3\x8f\x41\x14\x93\x51\x55\x97\x61\x96\x9b\x71\xd7\x9f\x82\x18\xa3\x92\x59\xa7\xa2\x9a\xab\xb2\xdb\xaf\xc3\x1c\xb3\xd3\x5d\xb7\xe3\x9e\xbb\xf3\xdf\xbf", + }, + }; + for (cases) |case| { + try testing.expectEqualStrings( + case.expect, + try decodeStrict(case.input, &output, .required), + ); + try testing.expectEqualStrings( + case.expect, + try decodeStrict(case.input, &output, .optional), + ); + } +} + +test "base64 strict decode invalid" { + const testing = std.testing; + var output: [128]u8 = undefined; + + // The invalid inputs from kitty's own strict decoding tests plus + // some extra padding-placement cases. All of these are invalid for + // both padding requirements. + const cases = [_][]const u8{ + "bGlnaHQgdw=", // missing one padding byte + "bGln!!Qgdw==", // invalid characters + "bGlnaHQgdw==\n", // trailing whitespace + "\nbGlnaHQgdw==", // leading whitespace + "bGlnaHQg dw==", // interior whitespace + "!!!!", + "=", + "==", + "A===", + "AB=C", // padding must be the suffix + "Zm9v YmFy", + }; + for (cases) |case| { + try testing.expectError( + error.Base64Invalid, + decodeStrict(case, &output, .required), + ); + try testing.expectError( + error.Base64Invalid, + decodeStrict(case, &output, .optional), + ); + } + + // Unpadded input is only tolerated when padding is optional. A + // single leftover character can never be decoded. + try testing.expectError( + error.Base64Invalid, + decodeStrict("bGlnaHQgdw", &output, .required), + ); + try testing.expectEqualStrings( + "light w", + try decodeStrict("bGlnaHQgdw", &output, .optional), + ); + try testing.expectError( + error.Base64Invalid, + decodeStrict("bGl", &output, .required), + ); + try testing.expectEqualStrings( + "li", + try decodeStrict("bGl", &output, .optional), + ); + try testing.expectError( + error.Base64Invalid, + decodeStrict("bGlna", &output, .optional), + ); +} + +test "base64 streaming decode chunk boundaries" { + const testing = std.testing; + const alloc = testing.allocator; + + // Decoding a stream split at every possible boundary, including + // one byte at a time, matches the single-shot decode. + const input = "c29tZSBsb25nZXIgZGF0YSB3aXRoIHBhZGRpbmc+Pz8="; + const expect = "some longer data with padding>??"; + for (0..input.len + 1) |split| { + var s: Streaming = .{}; + var result: std.ArrayListUnmanaged(u8) = .empty; + defer result.deinit(alloc); + var output: [64]u8 = undefined; + try result.appendSlice(alloc, try s.feed(input[0..split], &output)); + try result.appendSlice(alloc, try s.feed(input[split..], &output)); + try s.finish(); + try testing.expectEqualStrings(expect, result.items); + } + { + var s: Streaming = .{}; + var result: std.ArrayListUnmanaged(u8) = .empty; + defer result.deinit(alloc); + var output: [4]u8 = undefined; + for (0..input.len) |i| { + try result.appendSlice(alloc, try s.feed(input[i..][0..1], &output)); + } + try s.finish(); + try testing.expectEqualStrings(expect, result.items); + } +} + +test "base64 streaming decode invalid" { + const testing = std.testing; + var output: [64]u8 = undefined; + + // Invalid characters are rejected wherever they appear. + { + var s: Streaming = .{}; + try testing.expectError(error.Base64Invalid, s.feed("!!!!", &output)); + } + { + var s: Streaming = .{}; + try testing.expectError(error.Base64Invalid, s.feed("SGVs!!!bG8=", &output)); + } + { + var s: Streaming = .{}; + try testing.expectError( + error.Base64Invalid, + s.feed("\nc29tZSBkYXRh", &output), + ); + } + + // Data after terminal padding within one feed is rejected, even + // when the padded group only completes in that feed. + { + var s: Streaming = .{}; + try testing.expectError(error.Base64Invalid, s.feed("Z29vZA==SGVsbG8=", &output)); + } + { + var s: Streaming = .{}; + _ = try s.feed("Z29vZA=", &output); + try testing.expectError(error.Base64Invalid, s.feed("=SGVs", &output)); + } + + // A feed that ends exactly at terminal padding resets the stream: + // the next feed starts fresh, so clients that pad every chunk + // independently keep working (matching the kitty implementation, + // which resets its streaming decoder on EOF). + { + var s: Streaming = .{}; + try testing.expectEqualStrings("good", try s.feed("Z29vZA==", &output)); + try testing.expectEqualStrings("Hello", try s.feed("SGVsbG8=", &output)); + try s.finish(); + } + { + // Padding split across feeds resets too. + var s: Streaming = .{}; + try testing.expectEqualStrings("goo", try s.feed("Z29vZA=", &output)); + try testing.expectEqualStrings("d", try s.feed("=", &output)); + try testing.expectEqualStrings("more", try s.feed("bW9yZQ==", &output)); + try s.finish(); + } + + // Misplaced padding within a group. + { + var s: Streaming = .{}; + try testing.expectError(error.Base64Invalid, s.feed("YQ=X", &output)); + } + { + var s: Streaming = .{}; + try testing.expectError(error.Base64Invalid, s.feed("=AAA", &output)); + } + + // A stream that ends in a partial group is missing its padding. + // The failed finish resets the decoder for the next stream. + { + var s: Streaming = .{}; + try testing.expectEqualStrings("Hel", try s.feed("SGVsbG8", &output)); + try testing.expectError(error.Base64Invalid, s.finish()); + try testing.expectEqualStrings("Hel", try s.feed("SGVsbG8", &output)); + try testing.expectEqualStrings("lo", try s.feed("=", &output)); + try s.finish(); + } +} diff --git a/src/simd/base64_scalar.zig b/src/simd/base64_scalar.zig index 08886f187..69b3f29c7 100644 --- a/src/simd/base64_scalar.zig +++ b/src/simd/base64_scalar.zig @@ -6,6 +6,12 @@ pub const scalar_decoder: Base64Decoder = .init( null, ); +/// Whether c is one of the 64 standard base64 alphabet characters +/// (padding is not part of the alphabet). +pub fn isAlphabetChar(c: u8) bool { + return scalar_decoder.char_to_index[c] != Base64Decoder.invalid_char; +} + /// Copied from Zig 0.14.1 stdlib and commented out the invalid padding /// scenarios, because Kitty Graphics requires a decoder that doesn't care /// about invalid padding scenarios. diff --git a/src/terminal/kitty/clipboard.zig b/src/terminal/kitty/clipboard.zig index 24d002968..32cf8b90e 100644 --- a/src/terminal/kitty/clipboard.zig +++ b/src/terminal/kitty/clipboard.zig @@ -8,28 +8,35 @@ //! disagree in places. Notable reference behaviors we reproduce: //! //! * Malformed metadata (any record without '=', including an empty -//! metadata section), an unknown or missing `type`, and invalid -//! base64 in `mime`, `name`, or `pw` all silently drop the request -//! with no response. +//! metadata section) and an unknown or missing `type` silently drop +//! the request with no response. +//! * All base64 (payloads and the `mime`, `name`, and `pw` metadata +//! values) is strict RFC 4648 per the spec's "Encoding of payloads" +//! section: characters outside the standard alphabet (including +//! whitespace) and incorrect padding are rejected, never silently +//! skipped. An invalid value on `wdata` or `walias` aborts an +//! in-flight write with EINVAL; an invalid `read` is dropped with +//! no response since reads have no error status. //! * Decoded metadata and MIME-list payloads must be valid UTF-8. An //! invalid value on `wdata` or `walias`, or a `walias` without a //! target MIME type, aborts an in-flight write with EINVAL. +//! * The `wdata` payloads for one MIME type form a single base64 +//! stream split at arbitrary packet boundaries; only the +//! concatenation must be correctly padded. Like kitty (which +//! resets its streaming decoder on EOF), a packet ending exactly +//! at terminal padding restarts the stream, so independently +//! encoded chunks also work. //! * `mime`, `name`, and `pw` metadata values are base64-encoded UTF-8; //! everything else is verbatim. Unknown keys are ignored. //! * `id` is sanitized by stripping characters outside [a-zA-Z0-9-_+.] //! and truncating to 512 bytes, then echoed verbatim in every //! response packet (omitted when empty). -//! * A write data chunk with invalid base64 is dropped and the -//! transaction continues; it is not a protocol error. //! * 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. //! * Responses never send a payload section for an empty payload, //! except the targets ('.') listing DATA packet which is always sent. //! -//! I plan to open an upstream issue asking for clarification on these -//! once I implement this. -//! //! Specification: https://sw.kovidgoyal.net/kitty/clipboard/ const oscpkg = @import("../osc.zig"); diff --git a/src/terminal/kitty/clipboard_command.zig b/src/terminal/kitty/clipboard_command.zig index f5a6b1753..67bc26f06 100644 --- a/src/terminal/kitty/clipboard_command.zig +++ b/src/terminal/kitty/clipboard_command.zig @@ -95,10 +95,10 @@ pub const Metadata = struct { max_mime_len, ) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, - // Base64 acceptance is intentionally left unchanged while the - // protocol's exact requirements are being specified. - error.InvalidBase64 => return null, - error.Overflow, error.InvalidUtf8 => return error.InvalidValue, + error.Overflow, + error.InvalidBase64, + error.InvalidUtf8, + => return error.InvalidValue, }; result.pw = decodeValue( alloc, @@ -106,11 +106,10 @@ pub const Metadata = struct { max_pw_len, ) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, - error.InvalidBase64 => return null, + error.InvalidBase64, error.InvalidUtf8 => return error.InvalidValue, // An over-long password behaves as if none was given: it can // never match a stored grant. error.Overflow => "", - error.InvalidUtf8 => return error.InvalidValue, }; result.name = decodeValue( alloc, @@ -118,8 +117,10 @@ pub const Metadata = struct { max_name_len, ) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, - error.InvalidBase64 => return null, - error.Overflow, error.InvalidUtf8 => return error.InvalidValue, + error.Overflow, + error.InvalidBase64, + error.InvalidUtf8, + => return error.InvalidValue, }; return result; } @@ -193,7 +194,8 @@ pub const Metadata = struct { /// Base64-decode a metadata value. Per the spec these values "are /// UTF-8 strings that are base64 encoded", so the decoded result - /// must be valid UTF-8. + /// must be valid UTF-8, and the encoding is strict RFC 4648 with + /// required padding per the spec's "Encoding of payloads" section. fn decodeValue(alloc: Allocator, value: []const u8, max_len: usize) error{ OutOfMemory, Overflow, @@ -208,9 +210,10 @@ pub const Metadata = struct { // Decode const buf = try alloc.alloc(u8, simd.base64.maxLen(value)); errdefer alloc.free(buf); - const decoded = simd.base64.decode( + const decoded = simd.base64.decodeStrict( value, buf, + .required, ) catch return error.InvalidBase64; // Must be valid UTF-8 @@ -227,17 +230,20 @@ pub const Payload = struct { buf: []u8, data: []const u8, - /// Decode a base64 payload into freshly allocated memory. An - /// invalid payload means the sequence is dropped. + /// Decode a base64 payload into freshly allocated memory. The + /// encoding is strict RFC 4648 with required padding per the + /// spec's "Encoding of payloads" section; how an invalid payload + /// is reported (or not) depends on the packet type. pub fn init( alloc: Allocator, payload: []const u8, ) error{ OutOfMemory, Invalid }!Payload { const buf = try alloc.alloc(u8, simd.base64.maxLen(payload)); errdefer alloc.free(buf); - const data = simd.base64.decode( + const data = simd.base64.decodeStrict( payload, buf, + .required, ) catch return error.Invalid; return .{ .buf = buf, .data = data }; } @@ -363,11 +369,35 @@ test "metadata: mime decoded" { try testing.expectEqualStrings("text/plain", meta.mime); } -test "metadata: invalid mime base64 dropped" { +test "metadata: invalid mime base64 reported" { const testing = std.testing; var arena: std.heap.ArenaAllocator = .init(testing.allocator); defer arena.deinit(); - try testing.expect((try Metadata.parse(arena.allocator(), "type=wdata:mime=!!!")) == null); + + // Invalid base64 in a metadata value aborts an in-flight write + // per the spec, so like invalid UTF-8 it is reported rather than + // silently dropping the packet. Unpadded or whitespace-laced + // values are invalid too: metadata values use the same strict + // encoding as payloads. + const cases = [_][]const u8{ + "type=wdata:mime=!!!", + // "text/plain" without its padding. + "type=wdata:mime=dGV4dC9wbGFpbg", + // "text/plain" with its final byte replaced by '!'. + "type=wdata:mime=dGV4dC9wbGFpbg=!", + // A newline inside otherwise valid base64. + "type=wdata:mime=dGV4dC9w\nbGFpbg==", + }; + for (cases) |case| { + try testing.expectError( + error.InvalidValue, + Metadata.parse(arena.allocator(), case), + ); + try testing.expectEqual( + Operation.wdata, + Metadata.operation(case).?, + ); + } } test "metadata: invalid mime utf8 reported" { @@ -447,10 +477,20 @@ test "payload: mime iterator" { test "payload: invalid base64" { const testing = std.testing; - try testing.expectError( - error.Invalid, - Payload.init(testing.allocator, "!!!"), - ); + const cases = [_][]const u8{ + "!!!", + // "text/plain" without its padding: payloads use the strict + // encoding, so missing padding is rejected. + "dGV4dC9wbGFpbg", + // A newline inside otherwise valid base64. + "dGV4dC9w\nbGFpbg==", + }; + for (cases) |case| { + try testing.expectError( + error.Invalid, + Payload.init(testing.allocator, case), + ); + } } test "payload: decoded text must be valid utf8" { diff --git a/src/terminal/kitty/clipboard_write.zig b/src/terminal/kitty/clipboard_write.zig index ad3ec445e..5951dcbe7 100644 --- a/src/terminal/kitty/clipboard_write.zig +++ b/src/terminal/kitty/clipboard_write.zig @@ -5,6 +5,7 @@ const std = @import("std"); const assert = @import("../../quirks.zig").inlineAssert; const Allocator = std.mem.Allocator; +const simd = @import("../../simd/main.zig"); const clipboard = @import("../clipboard.zig"); const clipboard_command = @import("clipboard_command.zig"); @@ -50,6 +51,13 @@ pub const WriteState = struct { /// Index into entries currently receiving data. current: ?usize = null, + /// Decodes the concatenated payload stream of the entry currently + /// receiving data. Per the spec's "Encoding of payloads" section, + /// individual wdata packet payloads split one base64 stream at + /// arbitrary boundaries; only the concatenation per MIME type must + /// be valid, correctly padded base64. + decoder: simd.base64.Streaming = .{}, + pub const Options = struct { /// Maximum total decoded bytes accumulated by the transaction. max_size: usize = max_write_size, @@ -100,15 +108,16 @@ 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 the transaction exceeds max_size. - /// The caller must fail the whole transaction with EFBIG and abort - /// it, as required by the protocol. + /// Returns error.TooLarge when the transaction exceeds max_size + /// and error.Invalid when the payload stream is not valid base64. + /// The caller must fail the whole transaction with EFBIG or EINVAL + /// respectively and abort it, as required by the protocol. pub fn data( self: *WriteState, alloc: Allocator, meta: *const Metadata, payload: []const u8, - ) error{ OutOfMemory, TooLarge }!void { + ) error{ OutOfMemory, TooLarge, Invalid }!void { assert(meta.op == .wdata); assert(meta.mime.len > 0); // Switch the receiving entry if this chunk is for a different @@ -120,7 +129,10 @@ pub const WriteState = struct { break :entry; } - // Finalize the previous region. + // Finalize the previous region. Its concatenated + // stream must have ended on a complete base64 group, + // otherwise the data was not correctly padded. + try self.finishCurrent(); entry.len = self.spool.items.len - entry.start; } @@ -151,31 +163,30 @@ pub const WriteState = struct { self.current = self.entries.items.len - 1; } - // Each packet's payload is independently base64-encoded: per - // the spec, "payload is base64 encoded data" and clients chunk - // the data before encoding. An invalid chunk is dropped and - // the transaction continues. - const decoded = Payload.init( - alloc, + // The payloads for one MIME region concatenate into a single + // strict base64 stream, decoded directly into the spool's + // unused capacity. Invalid data aborts the transaction; per + // the spec it must not be silently discarded "since that + // turns corrupted data into apparently valid data". + try self.spool.ensureUnusedCapacity(alloc, self.decoder.maxLen(payload)); + const decoded = self.decoder.feed( payload, - ) catch |err| switch (err) { - error.OutOfMemory => return error.OutOfMemory, - error.Invalid => { - log.warn("clipboard write chunk has invalid base64, ignoring chunk", .{}); - return; - }, - }; - defer decoded.deinit(alloc); - - // Empty slice, do nothing. - if (decoded.data.len == 0) return; + self.spool.unusedCapacitySlice(), + ) catch return error.Invalid; // 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) return error.TooLarge; - try self.spool.appendSlice(alloc, decoded.data); + if (decoded.len > remaining) return error.TooLarge; + self.spool.items.len += decoded.len; + } + + /// Finish the decode stream of the entry currently receiving + /// data. Returns error.Invalid when the stream ends mid-group, + /// i.e. the concatenated payload was not correctly padded. + fn finishCurrent(self: *WriteState) error{Invalid}!void { + self.decoder.finish() catch return error.Invalid; } /// Register aliases from a walias packet: meta.mime is the target @@ -250,13 +261,16 @@ pub const WriteState = struct { /// Commit the transaction (a wdata packet without a MIME type). /// The caller must use the result, call Committed.deinit, and then - /// deinit this state. + /// deinit this state. Returns error.Invalid when the last region's + /// concatenated payload was not correctly padded; the caller must + /// fail the transaction with EINVAL and abort it. pub fn commit( self: *WriteState, alloc: Allocator, - ) error{OutOfMemory}!Committed { + ) error{ OutOfMemory, Invalid }!Committed { // Finalize the region receiving data. if (self.current) |idx| { + try self.finishCurrent(); const entry = &self.entries.items[idx]; entry.len = self.spool.items.len - entry.start; self.current = null; @@ -389,7 +403,77 @@ test "write: reused mime overwrites" { try testing.expectEqualStrings("c", committed.contents[0].data); } -test "write: invalid base64 chunk is dropped, transaction continues" { +test "write: invalid base64 chunk aborts the transaction" { + const testing = std.testing; + const alloc = testing.allocator; + + // Invalid characters anywhere in the stream are error.Invalid, + // which the handlers turn into an EINVAL abort. These mirror + // kitty's own tests for the spec change. + const invalid_payloads = [_][]const u8{ + "!!!", + "SGVs!!!bG8=", + "\nZGF0YSB3aXRoIGEgbmV3bGluZQ==", + }; + for (invalid_payloads) |payload| { + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{}); + defer state.deinit(alloc); + try testing.expectError(error.Invalid, state.data( + alloc, + &.{ .op = .wdata, .mime = "text/plain" }, + payload, + )); + } + + // Also after an earlier valid chunk for the same MIME type. + { + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{}); + defer state.deinit(alloc); + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "Z29vZA=="); // "good" + try testing.expectError(error.Invalid, state.data( + alloc, + &.{ .op = .wdata, .mime = "text/plain" }, + "SGVs!!!bG8=", + )); + } +} + +test "write: chunks split one base64 stream at arbitrary boundaries" { + const testing = std.testing; + const alloc = testing.allocator; + + // The concatenation of all payloads per MIME type is the base64 + // stream; individual packets need not be a multiple of four bytes. + const encoded = "c29tZSBkYXRh"; // "some data" + const splits = [_][]const usize{ + &.{ 3, 7 }, + &.{ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 }, + }; + for (splits) |split| { + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{}); + defer state.deinit(alloc); + + var prev: usize = 0; + for (split) |end| { + try state.data( + alloc, + &.{ .op = .wdata, .mime = "text/plain" }, + encoded[prev..end], + ); + prev = end; + } + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, encoded[prev..]); + + const committed = try state.commit(alloc); + defer committed.deinit(alloc); + try testing.expectEqualStrings("some data", committed.contents[0].data); + } +} + +test "write: incorrectly padded stream aborts at commit" { const testing = std.testing; const alloc = testing.allocator; @@ -397,13 +481,62 @@ test "write: invalid base64 chunk is dropped, transaction continues" { var state: WriteState = try .init(alloc, &begin_meta, .{}); defer state.deinit(alloc); - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG8="); // "Hello" - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "!!!bad!!!"); - try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "V29ybGQ="); // "World" + // "Hello" without its final padding byte: every chunk decodes, + // but the stream ends mid-group so the commit reports it. + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG8"); + try testing.expectError(error.Invalid, state.commit(alloc)); +} + +test "write: incorrectly padded stream aborts at MIME switch" { + const testing = std.testing; + const alloc = testing.allocator; + + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{}); + defer state.deinit(alloc); + + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "SGVsbG8"); + try testing.expectError(error.Invalid, state.data( + alloc, + &.{ .op = .wdata, .mime = "text/html" }, + "PGI+aGk8L2I+", + )); +} + +test "write: independently padded chunks accumulate" { + const testing = std.testing; + const alloc = testing.allocator; + + // A packet payload ending exactly at terminal padding resets the + // stream, so clients that base64-encode every chunk independently + // keep working (matching kitty, which resets its streaming + // decoder on EOF). Padding followed by more data within a single + // packet stays invalid. + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{}); + defer state.deinit(alloc); + + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "Z29vZA=="); // "good" + try state.data(alloc, &.{ .op = .wdata, .mime = "text/plain" }, "bW9yZQ=="); // "more" const committed = try state.commit(alloc); defer committed.deinit(alloc); - try testing.expectEqualStrings("HelloWorld", committed.contents[0].data); + try testing.expectEqualStrings("goodmore", committed.contents[0].data); +} + +test "write: data after padding within one chunk aborts" { + const testing = std.testing; + const alloc = testing.allocator; + + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{}); + defer state.deinit(alloc); + + try testing.expectError(error.Invalid, state.data( + alloc, + &.{ .op = .wdata, .mime = "text/plain" }, + "Z29vZA==bW9yZQ==", + )); } test "write: aliases resolve at commit" { diff --git a/src/terminal/stream_terminal.zig b/src/terminal/stream_terminal.zig index 66a38f99b..61172423a 100644 --- a/src/terminal/stream_terminal.zig +++ b/src/terminal/stream_terminal.zig @@ -661,12 +661,20 @@ pub const Handler = struct { } // Decode the base64 payload with the SIMD decoder (the same one - // used for Kitty graphics payloads) rather than the scalar std - // implementation; clipboard payloads can be megabytes. + // used for Kitty clipboard payloads) rather than the scalar std + // implementation; clipboard payloads can be megabytes. The + // Kitty clipboard spec governs OSC 52 base64 handling too: a + // request with characters outside the base64 alphabet is + // discarded entirely (never partially decoded), while a + // missing-padding tail is tolerated since OSC 52 has no way + // to report errors to the client. const alloc = self.terminal.gpa(); const buf = try alloc.alloc(u8, simd.base64.maxLen(data)); defer alloc.free(buf); - const decoded = try simd.base64.decode(data, buf); + const decoded = simd.base64.decodeStrict(data, buf, .optional) catch { + log.warn("OSC 52 clipboard write is not valid base64, ignoring", .{}); + return; + }; const contents = [_]clipboard.Content{.{ .mime = "text/plain", @@ -1075,6 +1083,13 @@ pub const Handler = struct { .EFBIG, terminator, ), + + // An invalid base64 payload stream aborts the transaction. + error.Invalid => self.kittyClipboardFinish( + state, + .EINVAL, + terminator, + ), }; } @@ -1128,6 +1143,13 @@ pub const Handler = struct { self.kittyClipboardFinish(state, .EIO, terminator); return error.OutOfMemory; }, + + // The last MIME type's payload stream was not correctly + // padded, which aborts the transaction. + error.Invalid => { + self.kittyClipboardFinish(state, .EINVAL, terminator); + return; + }, }; defer committed.deinit(alloc); @@ -3398,6 +3420,9 @@ test "clipboard_write effect callback" { .{ .sequence = "\x1B]52;0;Y3V0\x1B\\", .location = .standard, .data = "cut" }, .{ .sequence = "\x1B]52;x;ZmFsbGJhY2s=\x1B\\", .location = .standard, .data = "fallback" }, .{ .sequence = "\x1B]52;c;YQBi\x1B\\", .location = .standard, .data = "a\x00b" }, + // Missing padding is tolerated for OSC 52 since it has no way + // to report errors to the client, matching kitty. + .{ .sequence = "\x1B]52;c;dW5wYWRkZWQ\x1B\\", .location = .standard, .data = "unpadded" }, }; for (cases, 1..) |case, expected_count| { @@ -3417,9 +3442,14 @@ test "clipboard_write effect callback" { try testing.expect(S.last_mime == null); try testing.expect(S.last_data == null); - // Reads and malformed base64 are ignored. + // Reads and malformed base64 are ignored. The whole request is + // discarded on invalid characters (including whitespace) rather + // than decoding around them, per the Kitty clipboard spec that + // governs OSC 52 base64 handling. s.nextSlice("\x1B]52;c;?\x1B\\"); s.nextSlice("\x1B]52;c;***\x1B\\"); + s.nextSlice("\x1B]52;c;SGVs!!!bG8=\x1B\\"); + s.nextSlice("\x1B]52;c;aGVs bG8=\x1B\\"); try testing.expectEqual(@as(usize, cases.len + 1), S.count); // OSC 1337 Copy shares the normalized clipboard write path. @@ -4370,7 +4400,7 @@ test "kitty clipboard oversized text write aborts with EFBIG" { ); } -test "kitty clipboard invalid wdata chunk is skipped" { +test "kitty clipboard invalid wdata chunk aborts with EINVAL" { var t: Terminal = try .init(testing.io, testing.allocator, .{ .cols = 80, .rows = 24 }); defer t.deinit(testing.allocator); @@ -4383,20 +4413,81 @@ test "kitty clipboard invalid wdata chunk is skipped" { var s: Stream = .init(.{ .allocator = testing.allocator, .handler = handler }); defer s.deinit(); - s.nextSlice("\x1B]5522;type=write\x1B\\"); + s.nextSlice("\x1B]5522;type=write:id=w\x1B\\"); s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;SGVsbG8=\x1B\\"); // "Hello" s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;!!!bad!!!\x1B\\"); + try testing.expectEqualStrings( + "\x1B]5522;type=write:status=EINVAL:id=w\x1B\\", + S.responseSlice(), + ); + try testing.expect(!s.handler.semantic_failure); + + // The transaction is gone: later data and the commit do nothing. s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;V29ybGQ=\x1B\\"); // "World" s.nextSlice("\x1B]5522;type=wdata\x1B\\"); + try testing.expectEqual(@as(usize, 0), S.write_count); + try testing.expectEqualStrings( + "\x1B]5522;type=write:status=EINVAL:id=w\x1B\\", + S.responseSlice(), + ); +} + +test "kitty clipboard wdata chunks split one base64 stream" { + var t: Terminal = try .init(testing.io, testing.allocator, .{ .cols = 80, .rows = 24 }); + defer t.deinit(testing.allocator); + + const S = KittyClipboardCapture; + S.reset(); + + var handler: Handler = .init(&t); + handler.effects.write_pty = &S.writePty; + handler.effects.clipboard_write = &S.clipboardWrite; + var s: Stream = .init(.{ .allocator = testing.allocator, .handler = handler }); + defer s.deinit(); + + // "some data" encoded as one stream, split at non-group + // boundaries across packets. + s.nextSlice("\x1B]5522;type=write\x1B\\"); + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;c29\x1B\\"); + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;tZSBk\x1B\\"); + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;YXRh\x1B\\"); + s.nextSlice("\x1B]5522;type=wdata\x1B\\"); try testing.expectEqual(@as(usize, 1), S.write_count); - try testing.expectEqualStrings("HelloWorld", S.dataAt(0)); + try testing.expectEqualStrings("some data", S.dataAt(0)); try testing.expectEqualStrings( "\x1B]5522;type=write:status=DONE\x1B\\", S.responseSlice(), ); } +test "kitty clipboard unpadded wdata stream aborts at commit" { + var t: Terminal = try .init(testing.io, testing.allocator, .{ .cols = 80, .rows = 24 }); + defer t.deinit(testing.allocator); + + const S = KittyClipboardCapture; + S.reset(); + + var handler: Handler = .init(&t); + handler.effects.write_pty = &S.writePty; + handler.effects.clipboard_write = &S.clipboardWrite; + var s: Stream = .init(.{ .allocator = testing.allocator, .handler = handler }); + defer s.deinit(); + + // "Hello" without its final padding byte: every packet decodes, + // but the stream ends mid-group so the commit reports EINVAL. + s.nextSlice("\x1B]5522;type=write:id=w\x1B\\"); + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;SGVsbG8\x1B\\"); + try testing.expectEqual(@as(usize, 0), S.responses_len); + s.nextSlice("\x1B]5522;type=wdata\x1B\\"); + try testing.expectEqual(@as(usize, 0), S.write_count); + try testing.expectEqualStrings( + "\x1B]5522;type=write:status=EINVAL:id=w\x1B\\", + S.responseSlice(), + ); + try testing.expect(!s.handler.semantic_failure); +} + test "kitty clipboard in-flight transaction is freed on deinit" { var t: Terminal = try .init(testing.io, testing.allocator, .{ .cols = 80, .rows = 24 }); defer t.deinit(testing.allocator); diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index a933d2d20..27e06dcb7 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -1244,6 +1244,13 @@ pub const StreamHandler = struct { .EFBIG, terminator, ), + + // An invalid base64 payload stream aborts the transaction. + error.Invalid => try self.kittyClipboardWriteFinish( + state, + .EINVAL, + terminator, + ), }; } @@ -1307,6 +1314,14 @@ pub const StreamHandler = struct { ); return error.OutOfMemory; }, + + // The last MIME type's payload stream was not correctly + // padded, which aborts the transaction. + error.Invalid => return try self.kittyClipboardWriteFinish( + state, + .EINVAL, + terminator, + ), }; // The transaction is complete; the surface owns the reply. @@ -1317,7 +1332,7 @@ pub const StreamHandler = struct { self: *StreamHandler, state: *terminal.kitty.clipboard.WriteState, terminator: terminal.osc.Terminator, - ) error{OutOfMemory}!void { + ) error{ OutOfMemory, Invalid }!void { const committed = try state.commit(self.alloc); defer committed.deinit(self.alloc);