terminal/kitty: increase value buffer, make 'H' and 'V' i32

This commit increases the value buffer to 11 characters - this is
technically what it should be to allow for large negative integers
(i.e., 2.4 billion plus the sign character).

Additionally corrected the types for 'H' (horizontal offset) and 'V'
(vertical offset) - these are supposed to be i32 versus u32.

Added some extra tests to test all of this stuff as well.

I've also added a few more tests to help track these cases.

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;

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);
+        // 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;

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");
+}