diff --git a/src/terminal/kitty/graphics_command.zig b/src/terminal/kitty/graphics_command.zig index aa1bef5e9..c4657dcd9 100644 --- a/src/terminal/kitty/graphics_command.zig +++ b/src/terminal/kitty/graphics_command.zig @@ -406,6 +406,53 @@ pub const Command = struct { transmit_animation_frame: AnimationFrameLoading, control_animation: AnimationControl, compose_animation: AnimationFrameComposition, + + pub const Identifiers = struct { + image_id: u32 = 0, + image_number: u32 = 0, + placement_id: u32 = 0, + }; + + /// Returns the image and placement identifiers for any action. + pub fn identifiers(self: Control) Identifiers { + return switch (self) { + .query, .transmit => |t| .{ + .image_id = t.image_id, + .image_number = t.image_number, + .placement_id = t.placement_id, + }, + .transmit_and_display => |t| .{ + .image_id = t.transmission.image_id, + .image_number = t.transmission.image_number, + .placement_id = t.transmission.placement_id, + }, + .display => |d| .{ + .image_id = d.image_id, + .image_number = d.image_number, + .placement_id = d.placement_id, + }, + .delete => |d| .{ + .image_id = d.image_id, + .image_number = d.image_number, + .placement_id = d.placement_id, + }, + .transmit_animation_frame => |f| .{ + .image_id = f.image_id, + .image_number = f.image_number, + .placement_id = f.placement_id, + }, + .control_animation => |a| .{ + .image_id = a.image_id, + .image_number = a.image_number, + .placement_id = a.placement_id, + }, + .compose_animation => |c| .{ + .image_id = c.image_id, + .image_number = c.image_number, + .placement_id = c.placement_id, + }, + }; + } }; /// Take ownership over the data in this command. If the returned value @@ -697,6 +744,9 @@ pub const Display = struct { }; pub const AnimationFrameLoading = struct { + image_id: u32 = 0, // i + image_number: u32 = 0, // I + placement_id: u32 = 0, // p x: u32 = 0, // x y: u32 = 0, // y create_frame: u32 = 0, // c @@ -715,6 +765,18 @@ pub const AnimationFrameLoading = struct { fn parse(kv: KV) !AnimationFrameLoading { var result: AnimationFrameLoading = .{}; + if (kv.get('i')) |v| { + result.image_id = v; + } + + if (kv.get('I')) |v| { + result.image_number = v; + } + + if (kv.get('p')) |v| { + result.placement_id = v; + } + if (kv.get('x')) |v| { result.x = v; } @@ -752,6 +814,9 @@ pub const AnimationFrameLoading = struct { }; pub const AnimationFrameComposition = struct { + image_id: u32 = 0, // i + image_number: u32 = 0, // I + placement_id: u32 = 0, // p frame: u32 = 0, // c edit_frame: u32 = 0, // r x: u32 = 0, // x @@ -765,6 +830,18 @@ pub const AnimationFrameComposition = struct { fn parse(kv: KV) !AnimationFrameComposition { var result: AnimationFrameComposition = .{}; + if (kv.get('i')) |v| { + result.image_id = v; + } + + if (kv.get('I')) |v| { + result.image_number = v; + } + + if (kv.get('p')) |v| { + result.placement_id = v; + } + if (kv.get('c')) |v| { result.frame = v; } @@ -810,6 +887,9 @@ pub const AnimationFrameComposition = struct { }; pub const AnimationControl = struct { + image_id: u32 = 0, // i + image_number: u32 = 0, // I + placement_id: u32 = 0, // p action: AnimationAction = .invalid, // s frame: u32 = 0, // r gap_ms: u32 = 0, // z @@ -826,6 +906,18 @@ pub const AnimationControl = struct { fn parse(kv: KV) !AnimationControl { var result: AnimationControl = .{}; + if (kv.get('i')) |v| { + result.image_id = v; + } + + if (kv.get('I')) |v| { + result.image_number = v; + } + + if (kv.get('p')) |v| { + result.placement_id = v; + } + if (kv.get('s')) |v| { result.action = switch (v) { 0 => .invalid, @@ -856,180 +948,196 @@ pub const AnimationControl = struct { } }; -pub const Delete = union(enum) { - // a/A - all: bool, - - // i/I - id: struct { - delete: bool = false, // uppercase - image_id: u32 = 0, // i - placement_id: u32 = 0, // p - }, - - // n/N - newest: struct { - delete: bool = false, // uppercase - image_number: u32 = 0, // I - placement_id: u32 = 0, // p - }, - - // c/C, - intersect_cursor: bool, - - // f/F - animation_frames: bool, - - // p/P - intersect_cell: struct { - delete: bool = false, // uppercase - x: u32 = 0, // x - y: u32 = 0, // y - }, - - // q/Q - intersect_cell_z: struct { - delete: bool = false, // uppercase - x: u32 = 0, // x - y: u32 = 0, // y - z: i32 = 0, // z - }, - - // r/R - range: struct { - delete: bool = false, // uppercase - first: u32 = 0, // x - last: u32 = 0, // y - }, - - // x/X - column: struct { - delete: bool = false, // uppercase - x: u32 = 0, // x - }, - - // y/Y - row: struct { - delete: bool = false, // uppercase - y: u32 = 0, // y - }, - - // z/Z - z: struct { - delete: bool = false, // uppercase - z: i32 = 0, // z - }, +pub const Delete = struct { + image_id: u32 = 0, // i + image_number: u32 = 0, // I + placement_id: u32 = 0, // p + action: Action, fn parse(kv: KV) !Delete { - const what: u8 = what: { - const value = kv.get('d') orelse break :what 'a'; - const c = std.math.cast(u8, value) orelse return error.InvalidFormat; - break :what c; - }; - - return switch (what) { - 'a', 'A' => .{ .all = what == 'A' }, - - 'i', 'I' => blk: { - var result: Delete = .{ .id = .{ .delete = what == 'I' } }; - if (kv.get('i')) |v| { - result.id.image_id = v; - } - if (kv.get('p')) |v| { - result.id.placement_id = v; - } - - break :blk result; - }, - - 'n', 'N' => blk: { - var result: Delete = .{ .newest = .{ .delete = what == 'N' } }; - if (kv.get('I')) |v| { - result.newest.image_number = v; - } - if (kv.get('p')) |v| { - result.newest.placement_id = v; - } - - break :blk result; - }, - - 'c', 'C' => .{ .intersect_cursor = what == 'C' }, - - 'f', 'F' => .{ .animation_frames = what == 'F' }, - - 'p', 'P' => blk: { - var result: Delete = .{ .intersect_cell = .{ .delete = what == 'P' } }; - if (kv.get('x')) |v| { - result.intersect_cell.x = v; - } - if (kv.get('y')) |v| { - result.intersect_cell.y = v; - } - - break :blk result; - }, - - 'q', 'Q' => blk: { - var result: Delete = .{ .intersect_cell_z = .{ .delete = what == 'Q' } }; - if (kv.get('x')) |v| { - result.intersect_cell_z.x = v; - } - if (kv.get('y')) |v| { - result.intersect_cell_z.y = v; - } - if (kv.get('z')) |v| { - // We can bitcast here because of how we parse it earlier. - result.intersect_cell_z.z = @bitCast(v); - } - - break :blk result; - }, - - 'r', 'R' => blk: { - const x = kv.get('x') orelse return error.InvalidFormat; - const y = kv.get('y') orelse return error.InvalidFormat; - if (x > y) return error.InvalidFormat; - break :blk .{ - .range = .{ - .delete = what == 'R', - .first = x, - .last = y, - }, - }; - }, - - 'x', 'X' => blk: { - var result: Delete = .{ .column = .{ .delete = what == 'X' } }; - if (kv.get('x')) |v| { - result.column.x = v; - } - - break :blk result; - }, - - 'y', 'Y' => blk: { - var result: Delete = .{ .row = .{ .delete = what == 'Y' } }; - if (kv.get('y')) |v| { - result.row.y = v; - } - - break :blk result; - }, - - 'z', 'Z' => blk: { - var result: Delete = .{ .z = .{ .delete = what == 'Z' } }; - if (kv.get('z')) |v| { - // We can bitcast here because of how we parse it earlier. - result.z.z = @bitCast(v); - } - - break :blk result; - }, - - else => return error.InvalidFormat, + return .{ + .image_id = kv.get('i') orelse 0, + .image_number = kv.get('I') orelse 0, + .placement_id = kv.get('p') orelse 0, + .action = try .parse(kv), }; } + + pub const Action = union(enum) { + // a/A + all: bool, + + // i/I + id: struct { + delete: bool = false, // uppercase + image_id: u32 = 0, // i + placement_id: u32 = 0, // p + }, + + // n/N + newest: struct { + delete: bool = false, // uppercase + image_number: u32 = 0, // I + placement_id: u32 = 0, // p + }, + + // c/C, + intersect_cursor: bool, + + // f/F + animation_frames: bool, + + // p/P + intersect_cell: struct { + delete: bool = false, // uppercase + x: u32 = 0, // x + y: u32 = 0, // y + }, + + // q/Q + intersect_cell_z: struct { + delete: bool = false, // uppercase + x: u32 = 0, // x + y: u32 = 0, // y + z: i32 = 0, // z + }, + + // r/R + range: struct { + delete: bool = false, // uppercase + first: u32 = 0, // x + last: u32 = 0, // y + }, + + // x/X + column: struct { + delete: bool = false, // uppercase + x: u32 = 0, // x + }, + + // y/Y + row: struct { + delete: bool = false, // uppercase + y: u32 = 0, // y + }, + + // z/Z + z: struct { + delete: bool = false, // uppercase + z: i32 = 0, // z + }, + + fn parse(kv: KV) !Action { + const what: u8 = what: { + const value = kv.get('d') orelse break :what 'a'; + const c = std.math.cast(u8, value) orelse return error.InvalidFormat; + break :what c; + }; + + return switch (what) { + 'a', 'A' => .{ .all = what == 'A' }, + + 'i', 'I' => blk: { + var result: Action = .{ .id = .{ .delete = what == 'I' } }; + if (kv.get('i')) |v| { + result.id.image_id = v; + } + if (kv.get('p')) |v| { + result.id.placement_id = v; + } + + break :blk result; + }, + + 'n', 'N' => blk: { + var result: Action = .{ .newest = .{ .delete = what == 'N' } }; + if (kv.get('I')) |v| { + result.newest.image_number = v; + } + if (kv.get('p')) |v| { + result.newest.placement_id = v; + } + + break :blk result; + }, + + 'c', 'C' => .{ .intersect_cursor = what == 'C' }, + + 'f', 'F' => .{ .animation_frames = what == 'F' }, + + 'p', 'P' => blk: { + var result: Action = .{ .intersect_cell = .{ .delete = what == 'P' } }; + if (kv.get('x')) |v| { + result.intersect_cell.x = v; + } + if (kv.get('y')) |v| { + result.intersect_cell.y = v; + } + + break :blk result; + }, + + 'q', 'Q' => blk: { + var result: Action = .{ .intersect_cell_z = .{ .delete = what == 'Q' } }; + if (kv.get('x')) |v| { + result.intersect_cell_z.x = v; + } + if (kv.get('y')) |v| { + result.intersect_cell_z.y = v; + } + if (kv.get('z')) |v| { + // We can bitcast here because of how we parse it earlier. + result.intersect_cell_z.z = @bitCast(v); + } + + break :blk result; + }, + + 'r', 'R' => blk: { + const x = kv.get('x') orelse return error.InvalidFormat; + const y = kv.get('y') orelse return error.InvalidFormat; + if (x > y) return error.InvalidFormat; + break :blk .{ + .range = .{ + .delete = what == 'R', + .first = x, + .last = y, + }, + }; + }, + + 'x', 'X' => blk: { + var result: Action = .{ .column = .{ .delete = what == 'X' } }; + if (kv.get('x')) |v| { + result.column.x = v; + } + + break :blk result; + }, + + 'y', 'Y' => blk: { + var result: Action = .{ .row = .{ .delete = what == 'Y' } }; + if (kv.get('y')) |v| { + result.row.y = v; + } + + break :blk result; + }, + + 'z', 'Z' => blk: { + var result: Action = .{ .z = .{ .delete = what == 'Z' } }; + if (kv.get('z')) |v| { + // We can bitcast here because of how we parse it earlier. + result.z.z = @bitCast(v); + } + + break :blk result; + }, + + else => return error.InvalidFormat, + }; + } + }; }; pub const CompositionMode = enum { @@ -1210,7 +1318,7 @@ test "delete command" { defer command.deinit(alloc); try testing.expect(command.control == .delete); - const v = command.control.delete; + const v = command.control.delete.action; try testing.expect(v == .intersect_cell); const dv = v.intersect_cell; try testing.expect(!dv.delete); @@ -1428,7 +1536,7 @@ test "delete range command 1" { defer command.deinit(alloc); try testing.expect(command.control == .delete); - const v = command.control.delete; + const v = command.control.delete.action; try testing.expect(v == .range); const range = v.range; try testing.expect(!range.delete); @@ -1448,7 +1556,7 @@ test "delete range command 2" { defer command.deinit(alloc); try testing.expect(command.control == .delete); - const v = command.control.delete; + const v = command.control.delete.action; try testing.expect(v == .range); const range = v.range; try testing.expect(range.delete); diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index d05ef7abc..c0cd36549 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -44,6 +44,26 @@ pub fn execute( // this can change. var quiet = cmd.quiet; + // The protocol makes i and I mutually exclusive for every action, so this + // must happen before dispatch and before an action can mutate storage. + // https://sw.kovidgoyal.net/kitty/graphics-protocol/#requesting-image-ids-from-the-terminal + const identifiers = cmd.control.identifiers(); + if (identifiers.image_id > 0 and identifiers.image_number > 0) { + const resp: Response = .{ + .id = identifiers.image_id, + .image_number = identifiers.image_number, + .placement_id = identifiers.placement_id, + .message = "EINVAL: image ID and number are mutually exclusive", + }; + log.warn("erroneous kitty graphics response: {s}", .{resp.message}); + + return switch (quiet) { + .no => resp, + .ok => resp, + .failures => null, + }; + } + const resp_: ?Response = switch (cmd.control) { .query => query(io, alloc, terminal, cmd), .display => display(io, alloc, terminal, cmd), @@ -90,6 +110,7 @@ pub fn execute( return null; } + /// Execute a "query" command. /// /// This command is used to attempt to load an image and respond with @@ -144,9 +165,6 @@ fn transmit( .image_number = t.image_number, .placement_id = t.placement_id, }; - if (t.image_id > 0 and t.image_number > 0) { - return .{ .message = "EINVAL: image ID and number are mutually exclusive" }; - } const load = loadAndAddImage(io, alloc, terminal, cmd) catch |err| { encodeError(&result, err); @@ -328,7 +346,7 @@ fn delete( cmd: *const Command, ) Response { const storage = &terminal.screens.active.kitty_images; - storage.delete(io, alloc, terminal, cmd.control.delete); + storage.delete(io, alloc, terminal, cmd.control.delete.action); // Delete never responds on success return .{}; @@ -431,6 +449,119 @@ fn encodeError(r: *Response, err: EncodeableError) void { } } +test "kittygfx image id and number are mutually exclusive for every action" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var t = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + + const inputs = [_][]const u8{ + "a=q,f=24,s=1,v=1,i=1,I=2,p=3;AAAA", + "a=t,f=24,s=1,v=1,i=1,I=2,p=3;AAAA", + "a=T,f=24,s=1,v=1,i=1,I=2,p=3;AAAA", + "a=p,i=1,I=2,p=3", + "a=d,d=a,i=1,I=2,p=3", + "a=f,i=1,I=2,p=3;AAAA", + "a=a,i=1,I=2,p=3", + "a=c,i=1,I=2,p=3", + }; + + for (inputs) |input| { + const cmd = try command.Parser.parseString(alloc, input); + defer cmd.deinit(alloc); + + const resp = execute(io, alloc, &t, &cmd).?; + try testing.expect(!resp.ok()); + try testing.expectEqual(@as(u32, 1), resp.id); + try testing.expectEqual(@as(u32, 2), resp.image_number); + try testing.expectEqual(@as(u32, 3), resp.placement_id); + try testing.expectEqualStrings( + "EINVAL: image ID and number are mutually exclusive", + resp.message, + ); + + var buf: [128]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buf); + try resp.encode(&writer); + try testing.expectEqualStrings( + "\x1b_Gi=1,I=2,p=3;EINVAL: image ID and number are mutually exclusive\x1b\\", + writer.buffered(), + ); + } + + try testing.expectEqual(@as(usize, 0), t.screens.active.kitty_images.images.count()); + try testing.expectEqual(@as(usize, 0), t.screens.active.kitty_images.placements.count()); +} + +test "kittygfx conflicting identifiers are rejected before mutation" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var t = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + const storage = &t.screens.active.kitty_images; + + // Store an image so a buggy put would succeed and mutate placement state. + { + const cmd = try command.Parser.parseString( + alloc, + "a=t,f=24,s=1,v=1,i=1;/wAA", + ); + defer cmd.deinit(alloc); + try testing.expect(execute(io, alloc, &t, &cmd).?.ok()); + } + + // Directly constructed commands must receive the same validation as + // commands produced by the parser. + { + const cmd: Command = .{ .control = .{ .display = .{ + .image_id = 1, + .image_number = 2, + .placement_id = 7, + .cursor_movement = .none, + } } }; + const resp = execute(io, alloc, &t, &cmd).?; + try testing.expect(!resp.ok()); + try testing.expectEqual(@as(u32, 1), resp.id); + try testing.expectEqual(@as(u32, 2), resp.image_number); + try testing.expectEqual(@as(u32, 7), resp.placement_id); + try testing.expectEqual(@as(usize, 0), storage.placements.count()); + } + + // Add a real placement so a buggy delete would remove it. + { + const cmd = try command.Parser.parseString(alloc, "a=p,i=1,p=7,C=1"); + defer cmd.deinit(alloc); + try testing.expect(execute(io, alloc, &t, &cmd).?.ok()); + try testing.expectEqual(@as(usize, 1), storage.placements.count()); + } + + { + const cmd = try command.Parser.parseString( + alloc, + "a=d,d=i,i=1,I=2,p=7", + ); + defer cmd.deinit(alloc); + const resp = execute(io, alloc, &t, &cmd).?; + try testing.expect(!resp.ok()); + try testing.expectEqual(@as(usize, 1), storage.placements.count()); + } + + // q=2 suppresses the required error response but not the validation. + { + const cmd = try command.Parser.parseString( + alloc, + "a=d,d=i,i=1,I=2,p=7,q=2", + ); + defer cmd.deinit(alloc); + try testing.expect(execute(io, alloc, &t, &cmd) == null); + try testing.expectEqual(@as(usize, 1), storage.placements.count()); + } +} + test "kittygfx more chunks with q=1" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index fe97b1509..a473468e6 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -453,7 +453,7 @@ pub const ImageStorage = struct { io: std.Io, alloc: Allocator, t: *terminal.Terminal, - cmd: command.Delete, + cmd: command.Delete.Action, ) void { // Deletes only ever remove placements/images, so comparing counts // before and after tells us whether anything actually changed.