diff --git a/src/terminal/apc.zig b/src/terminal/apc.zig index 6a6b8cc36..26c59729a 100644 --- a/src/terminal/apc.zig +++ b/src/terminal/apc.zig @@ -122,6 +122,26 @@ test "garbage Kitty command" { try testing.expect(h.end() == null); } +test "Kitty command with overflow u32" { + const testing = std.testing; + const alloc = testing.allocator; + + var h: Handler = .{}; + h.start(); + for ("Ga=p,i=10000000000") |c| h.feed(alloc, c); + try testing.expect(h.end() == null); +} + +test "Kitty command with overflow i32" { + const testing = std.testing; + const alloc = testing.allocator; + + var h: Handler = .{}; + h.start(); + for ("Ga=p,i=1,z=-9999999999") |c| h.feed(alloc, c); + try testing.expect(h.end() == null); +} + test "valid Kitty command" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/kitty/graphics_command.zig b/src/terminal/kitty/graphics_command.zig index 0ef054293..d199711d3 100644 --- a/src/terminal/kitty/graphics_command.zig +++ b/src/terminal/kitty/graphics_command.zig @@ -23,10 +23,10 @@ pub const Parser = struct { /// This is the list of KV pairs that we're building up. kv: KV = .{}, - /// This is used as a buffer to store the key/value of a KV pair. - /// The value of a KV pair is at most a 32-bit integer which at most - /// is 10 characters (4294967295). - kv_temp: [10]u8 = undefined, + /// This is used as a buffer to store the key/value of a KV pair. The value + /// of a KV pair is at most a 32-bit integer which at most is 10 characters + /// (4294967295), plus one character for the sign bit on signed ints. + kv_temp: [11]u8 = undefined, kv_temp_len: u4 = 0, kv_current: u8 = 0, // Current kv key @@ -237,16 +237,14 @@ pub const Parser = struct { } } - // Only "z" is currently signed. This is a bit of a kloodge; if more - // fields become signed we can rethink this but for now we parse - // "z" as i32 then bitcast it to u32 then bitcast it back later. - if (self.kv_current == 'z') { - const v = try std.fmt.parseInt(i32, self.kv_temp[0..self.kv_temp_len], 10); - try self.kv.put(alloc, self.kv_current, @bitCast(v)); - } else { - const v = try std.fmt.parseInt(u32, self.kv_temp[0..self.kv_temp_len], 10); - try self.kv.put(alloc, self.kv_current, v); - } + // Handle integer fields, parsing signed fields accordingly. We still + // store the fields as u32 as they can be bitcast back later during + // building of the higher-level command tree. + const v: u32 = switch (self.kv_current) { + 'z', 'H', 'V' => @bitCast(try std.fmt.parseInt(i32, self.kv_temp[0..self.kv_temp_len], 10)), + else => try std.fmt.parseInt(u32, self.kv_temp[0..self.kv_temp_len], 10), + }; + try self.kv.put(alloc, self.kv_current, v); // Clear our temp buffer self.kv_temp_len = 0; @@ -505,8 +503,8 @@ pub const Display = struct { virtual_placement: bool = false, // U parent_id: u32 = 0, // P parent_placement_id: u32 = 0, // Q - horizontal_offset: u32 = 0, // H - vertical_offset: u32 = 0, // V + horizontal_offset: i32 = 0, // H + vertical_offset: i32 = 0, // V z: i32 = 0, // z pub const CursorMovement = enum { @@ -591,11 +589,13 @@ pub const Display = struct { } if (kv.get('H')) |v| { - result.horizontal_offset = v; + // We can bitcast here because of how we parse it earlier. + result.horizontal_offset = @bitCast(v); } if (kv.get('V')) |v| { - result.vertical_offset = v; + // We can bitcast here because of how we parse it earlier. + result.vertical_offset = @bitCast(v); } return result; @@ -1069,6 +1069,95 @@ test "ignore very long values" { try testing.expectEqual(@as(u32, 0), v.height); } +test "ensure very large negative values don't get skipped" { + const testing = std.testing; + const alloc = testing.allocator; + var p = Parser.init(alloc); + defer p.deinit(); + + const input = "a=p,i=1,z=-2000000000"; + for (input) |c| try p.feed(c); + const command = try p.complete(); + defer command.deinit(alloc); + + try testing.expect(command.control == .display); + const v = command.control.display; + try testing.expectEqual(1, v.image_id); + try testing.expectEqual(-2000000000, v.z); +} + +test "ensure proper overflow error for u32" { + const testing = std.testing; + const alloc = testing.allocator; + var p = Parser.init(alloc); + defer p.deinit(); + + const input = "a=p,i=10000000000"; + for (input) |c| try p.feed(c); + try testing.expectError(error.Overflow, p.complete()); +} + +test "ensure proper overflow error for i32" { + const testing = std.testing; + const alloc = testing.allocator; + var p = Parser.init(alloc); + defer p.deinit(); + + const input = "a=p,i=1,z=-9999999999"; + for (input) |c| try p.feed(c); + try testing.expectError(error.Overflow, p.complete()); +} + +test "all i32 values" { + const testing = std.testing; + const alloc = testing.allocator; + + { + // 'z' (usually z-axis values) + var p = Parser.init(alloc); + defer p.deinit(); + const input = "a=p,i=1,z=-1"; + for (input) |c| try p.feed(c); + const command = try p.complete(); + defer command.deinit(alloc); + + try testing.expect(command.control == .display); + const v = command.control.display; + try testing.expectEqual(1, v.image_id); + try testing.expectEqual(-1, v.z); + } + + { + // 'H' (relative placement, horizontal offset) + var p = Parser.init(alloc); + defer p.deinit(); + const input = "a=p,i=1,H=-1"; + for (input) |c| try p.feed(c); + const command = try p.complete(); + defer command.deinit(alloc); + + try testing.expect(command.control == .display); + const v = command.control.display; + try testing.expectEqual(1, v.image_id); + try testing.expectEqual(-1, v.horizontal_offset); + } + + { + // 'V' (relative placement, vertical offset) + var p = Parser.init(alloc); + defer p.deinit(); + const input = "a=p,i=1,V=-1"; + for (input) |c| try p.feed(c); + const command = try p.complete(); + defer command.deinit(alloc); + + try testing.expect(command.control == .display); + const v = command.control.display; + try testing.expectEqual(1, v.image_id); + try testing.expectEqual(-1, v.vertical_offset); + } +} + test "response: encode nothing without ID or image number" { const testing = std.testing; var buf: [1024]u8 = undefined; diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index c43bbbb9f..42f12ea07 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -495,3 +495,37 @@ test "kittygfx default format is rgba" { const img = storage.imageById(1).?; try testing.expectEqual(command.Transmission.Format.rgba, img.format); } + +test "kittygfx test valid u32 (expect invalid image ID)" { + const testing = std.testing; + const alloc = testing.allocator; + + var t = try Terminal.init(alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + + const cmd = try command.Parser.parseString( + alloc, + "a=p,i=4294967295", + ); + defer cmd.deinit(alloc); + const resp = execute(alloc, &t, &cmd).?; + try testing.expect(!resp.ok()); + try testing.expectEqual(resp.message, "ENOENT: image not found"); +} + +test "kittygfx test valid i32 (expect invalid image ID)" { + const testing = std.testing; + const alloc = testing.allocator; + + var t = try Terminal.init(alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + + const cmd = try command.Parser.parseString( + alloc, + "a=p,i=1,z=-2147483648", + ); + defer cmd.deinit(alloc); + const resp = execute(alloc, &t, &cmd).?; + try testing.expect(!resp.ok()); + try testing.expectEqual(resp.message, "ENOENT: image not found"); +}