mirrors/ghostty
1
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2026-08-24 16:11:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

terminal/kitty: reject conflicting image identifiers (#13889)

Browse Source 
Reject Kitty graphics commands that specify both an image ID and an
image number across every protocol action.

These commands previously produced no wire response for transmissions,
while put and delete actions could proceed using one identifier and
mutate terminal state.

Retain the original command identifiers, validate them before action
dispatch, and preserve them in the EINVAL response. Add regression
coverage for every action, response encoding, quiet suppression, and
pre-mutation rejection.


https://sw.kovidgoyal.net/kitty/graphics-protocol/#requesting-image-ids-from-the-terminal
This commit is contained in:
Mitchell Hashimoto
2026-08-18 10:24:42 -07:00
committed by GitHub
parent d4d72f3205 83145c0a37
commit 409e682c83
3 changed files with 417 additions and 178 deletions
Download Patch File Download Diff File Expand all files Collapse all files

454
src/terminal/kitty/graphics_command.zig
View File

@@ -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);

139
src/terminal/kitty/graphics_exec.zig
View File

@@ -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;

2
src/terminal/kitty/graphics_storage.zig
View File

@@ -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.