fix(rpc): "grid_line" event parsing crashes (#25581)

refactor: use a more idiomatic loop to iterate over the cells

There are two cases in which the following assertion would fail:
```c
assert(g->icell < g->ncells);
```

1. If `g->ncells = 0`. Update this to be legal.
2. If an EOF is reached while parsing `wrap`. In this case, the unpacker
   attempts to resume from `cells`, which is a bug. Create a new state
   for parsing `wrap`.

Reference: https://neovim.io/doc/user/ui.html#ui-event-grid_line

src/nvim/msgpack_rpc/unpacker.c

@@ -291,13 +291,13 @@ error:
 // objects. For the moment "redraw/grid_line" uses a hand-rolled decoder,
 // to avoid a blizzard of small objects for each screen cell.
 //
-// <0>[2, "redraw", <10>[{11}["method", <12>[args], <12>[args], ...], <11>[...], ...]]
+// <0>[2, "redraw", <10>[<11>["method", <12>[args], <12>[args], ...], <11>[...], ...]]
 //
 // Where [args] gets unpacked as an Array. Note: first {11} is not saved as a state.
 //
 // When method is "grid_line", we furthermore decode a cell at a time like:
 //
-// <0>[2, "redraw", <10>[{11}["grid_line", <14>[g, r, c, [<15>[cell], <15>[cell], ...]], ...], <11>[...], ...]]
+// <0>[2, "redraw", <10>[<11>["grid_line", <14>[g, r, c, [<15>[cell], <15>[cell], ...], <16>wrap]], <11>[...], ...]]
 //
 // where [cell] is [char, repeat, attr], where 'repeat' and 'attr' is optional
 
@@ -322,7 +322,7 @@ bool unpacker_advance(Unpacker *p)
       return false;
     }
 
-    if (p->state == 15) {
+    if (p->state == 16) {
       // grid_line event already unpacked
       goto done;
     } else {
@@ -357,10 +357,10 @@ done:
     p->state = 0;
     return true;
   case 13:
-  case 15:
+  case 16:
     p->ncalls--;
     if (p->ncalls > 0) {
-      p->state = (p->state == 15) ? 14 : 12;
+      p->state = (p->state == 16) ? 14 : 12;
     } else if (p->nevents > 0) {
       p->state = 11;
     } else {
@@ -393,7 +393,6 @@ bool unpacker_parse_redraw(Unpacker *p)
     return false; \
   }
 
-redo:
   switch (p->state) {
   case 10:
     NEXT_TYPE(tok, MPACK_TOKEN_ARRAY);
@@ -461,6 +460,7 @@ redo:
     FALLTHROUGH;
 
   case 15:
+    for (; g->icell != g->ncells; g->icell++) {
       assert(g->icell < g->ncells);
 
       NEXT_TYPE(tok, MPACK_TOKEN_ARRAY);
@@ -506,18 +506,18 @@ redo:
         }
       }
 
-    g->icell++;
+      p->read_ptr = data;
-    if (g->icell == g->ncells) {
+      p->read_size = size;
+    }
+    p->state = 16;
+    FALLTHROUGH;
+
+  case 16:
     NEXT_TYPE(tok, MPACK_TOKEN_BOOLEAN);
     g->wrap = mpack_unpack_boolean(tok);
     p->read_ptr = data;
     p->read_size = size;
     return true;
-    }
-
-    p->read_ptr = data;
-    p->read_size = size;
-    goto redo;
 
   case 12:
     return true;

test/helpers.lua

@@ -15,6 +15,7 @@ local function shell_quote(str)
   end
 end
 
+--- @class test.helpers
 local module = {
   REMOVE_THIS = {},
 }

test/unit/helpers.lua

@@ -861,6 +861,7 @@ local function is_asan()
   end
 end
 
+--- @class test.unit.helpers.module
 local module = {
   cimport = cimport,
   cppimport = cppimport,
@@ -890,7 +891,9 @@ local module = {
   debug_log = debug_log,
   is_asan = is_asan,
 }
+--- @class test.unit.helpers: test.unit.helpers.module, test.helpers
 module = global_helpers.tbl_extend('error', module, global_helpers)
+
 return function()
   return module
 end

test/unit/msgpack_spec.lua

@@ -0,0 +1,75 @@
+local helpers = require('test.unit.helpers')(after_each)
+local cimport = helpers.cimport
+local itp = helpers.gen_itp(it)
+local lib = cimport('./src/nvim/msgpack_rpc/unpacker.h', './src/nvim/memory.h')
+local ffi = helpers.ffi
+local eq = helpers.eq
+local to_cstr = helpers.to_cstr
+
+--- @class Unpacker
+--- @field read_ptr ffi.cdata*
+--- @field read_size number
+
+--- @alias Unpacker* table<number, Unpacker>
+--- @return Unpacker* unpacker `unpacker[0]` to dereference
+local function make_unpacker()
+  return ffi.gc(ffi.cast('Unpacker*', lib.xcalloc(1, ffi.sizeof('Unpacker'))), function(unpacker)
+    lib.unpacker_teardown(unpacker, nil, nil)
+    lib.xfree(unpacker)
+  end)
+end
+
+--- @param unpacker Unpacker*
+--- @param data string
+--- @param size number? *default: data:len()*
+local function unpacker_goto(unpacker, data, size)
+  unpacker[0].read_ptr = to_cstr(data)
+  unpacker[0].read_size = size or data:len()
+end
+
+--- @param unpacker Unpacker*
+--- @return boolean
+local function unpacker_advance(unpacker)
+  return lib.unpacker_advance(unpacker)
+end
+
+describe('msgpack', function()
+  describe('unpacker', function()
+    itp('does not crash when paused between `cells` and `wrap` params of `grid_line` #25184', function()
+      -- [kMessageTypeNotification, "redraw", [
+      --   ["grid_line",
+      --     [2, 0, 0, [[" " , 0, 77]], false]
+      --   ]
+      -- ]]
+      local payload =
+        '\x93\x02\xa6\x72\x65\x64\x72\x61\x77\x91\x92\xa9\x67\x72\x69\x64\x5f\x6c\x69\x6e\x65\x95\x02\x00\x00\x91\x93\xa1\x20\x00\x4d\xc2'
+
+      local unpacker = make_unpacker()
+      lib.unpacker_init(unpacker)
+
+      unpacker_goto(unpacker, payload, payload:len() - 1)
+      local finished = unpacker_advance(unpacker)
+      eq(finished, false)
+
+      unpacker[0].read_size = unpacker[0].read_size + 1
+      finished = unpacker_advance(unpacker)
+      eq(finished, true)
+    end)
+
+    itp('does not crash when parsing grid_line event with 0 `cells` #25184', function()
+      local unpacker = make_unpacker()
+      lib.unpacker_init(unpacker)
+
+      unpacker_goto(unpacker,
+        -- [kMessageTypeNotification, "redraw", [
+        --   ["grid_line",
+        --     [2, 0, 0, [], false]
+        --   ]
+        -- ]]
+        '\x93\x02\xa6\x72\x65\x64\x72\x61\x77\x91\x92\xa9\x67\x72\x69\x64\x5f\x6c\x69\x6e\x65\x95\x02\x00\x00\x90\xc2'
+      )
+      local finished = unpacker_advance(unpacker)
+      eq(finished, true)
+    end)
+  end)
+end)