fix(messages): disallow user-defined integer message-id #38359

Problem:
`nvim_echo(…, {id=…})` accepts user-defined id as a string or integer.
Generated ids are always higher than last highest msg-id used. Thus
plugins may accidentally advance the integer id "address space", which,
at minimum, could lead to confusion when troubleshooting, or in the
worst case, could overflow or "exhaust" the id address space.

There's no use-case for it, and it could be the mildly confusing, so we
should just disallow it.

Solution:
Disallow *integer* user-defined message-id.
Only allow *string* user-defined message-id.

src/nvim/api/vim.c

@@ -865,6 +865,12 @@ Union(Integer, String) nvim_echo(ArrayOf(Tuple(String, *HLGroupID)) chunks, Bool
     goto error;
   });
 
+  // Message-id may be user-defined only if String, not Integer.
+  VALIDATE(opts->id.type != kObjectTypeInteger || msg_id_exists(opts->id.data.integer),
+           "Invalid 'id': %" PRId64, opts->id.data.integer, {
+    goto error;
+  });
+
   MessageData msg_data = { .title = opts->title, .status = opts->status,
                            .percent = opts->percent, .data = opts->data };
 

src/nvim/lua/executor.c

@@ -1013,7 +1013,7 @@ static void nlua_print_event(void **argv)
   HlMessageChunk chunk = { { .data = argv[0], .size = (size_t)(intptr_t)argv[1] - 1 }, 0 };
   kv_push(msg, chunk);
   bool needs_clear = false;
-  msg_multihl(INTEGER_OBJ(0), msg, "lua_print", true, false, NULL, &needs_clear);
+  msg_multihl(NIL, msg, "lua_print", true, false, NULL, &needs_clear);
 }
 
 /// Print as a Vim message

src/nvim/message.c

@@ -169,6 +169,12 @@ static int msg_grid_pos_at_flush = 0;
 
 static int64_t msg_id_next = 1;           ///< message id to be allocated to next message
 
+/// Returns true if the given integer message-id was previously generated.
+bool msg_id_exists(int64_t id)
+{
+  return id > 0 && id < msg_id_next;
+}
+
 static void ui_ext_msg_set_pos(int row, bool scrolled)
 {
   char buf[MAX_SCHAR_SIZE];
@@ -356,12 +362,13 @@ static HlMessage format_progress_message(HlMessage hl_msg, MessageData *msg_data
 MsgID msg_multihl(MsgID id, HlMessage hl_msg, const char *kind, bool history, bool err,
                   MessageData *msg_data, bool *needs_msg_clear)
 {
-  // provide a new id if not given
+  // - Nil: Generate a new Integer id.
+  // - Integer: Existing id.
+  // - String: User-defined id (new or existing).
   if (id.type == kObjectTypeNil) {
     id = INTEGER_OBJ(msg_id_next++);
-  } else if (id.type == kObjectTypeInteger) {
-    id = id.data.integer > 0 ? id : INTEGER_OBJ(msg_id_next++);
-    msg_id_next = MAX(msg_id_next, id.data.integer + 1);
+  } else if (id.type == kObjectTypeInteger && !msg_id_exists(id.data.integer)) {
+    abort();
   }
 
   // don't display progress message in cmd when target doesn't have cmd
@@ -1341,7 +1348,7 @@ void ex_messages(exarg_T *eap)
     if (redirecting() || !ui_has(kUIMessages)) {
       msg_silent += ui_has(kUIMessages);
       bool needs_clear = false;
-      msg_multihl(INTEGER_OBJ(0), p->msg, p->kind, false, false, NULL, &needs_clear);
+      msg_multihl(NIL, p->msg, p->kind, false, false, NULL, &needs_clear);
       msg_silent -= ui_has(kUIMessages);
     }
   }

test/functional/api/vim_spec.lua

@@ -3821,6 +3821,11 @@ describe('API', function()
         pcall_err(api.nvim_echo, { { '', '', '' } }, 1, {})
       )
       eq("Invalid 'hl_group': 'text highlight'", pcall_err(api.nvim_echo, { { '', false } }, 1, {}))
+      eq("Invalid 'id': 4", pcall_err(api.nvim_echo, { { 'foo' } }, false, { id = 4 }))
+      eq("Invalid 'id': 0", pcall_err(api.nvim_echo, { { 'foo' } }, false, { id = 0 }))
+      eq("Invalid 'id': -1", pcall_err(api.nvim_echo, { { 'foo' } }, false, { id = -1 }))
+      -- String ids are always allowed (user-defined).
+      eq('my.msg.id', api.nvim_echo({ { 'foo' } }, false, { id = 'my.msg.id' }))
     end)
 
     it('should clear cmdline message before echo', function()
@@ -3913,8 +3918,8 @@ describe('API', function()
 
     it('increments message ID', function()
       eq(1, api.nvim_echo({ { 'foo' } }, false, {}))
-      eq(4, api.nvim_echo({ { 'foo' } }, false, { id = 4 }))
-      eq(5, api.nvim_echo({ { 'foo' } }, false, {}))
+      eq(1, api.nvim_echo({ { 'foo' } }, false, { id = 1 })) -- User may pass existing id.
+      eq(2, api.nvim_echo({ { 'foo' } }, false, {}))
     end)
 
     it('no use-after-free for custom kind with :messages #38289', function()

test/functional/ui/messages2_spec.lua

@@ -678,9 +678,9 @@ describe('messages2', function()
 
   it('replace by message ID', function()
     exec_lua(function()
-      vim.api.nvim_echo({ { 'foo' } }, true, { id = 1 })
-      vim.api.nvim_echo({ { 'bar\nbaz' } }, true, { id = 2 })
-      vim.api.nvim_echo({ { 'foo' } }, true, { id = 3 })
+      assert(1 == vim.api.nvim_echo({ { 'foo' } }, true, {}))
+      assert(2 == vim.api.nvim_echo({ { 'bar\nbaz' } }, true, {}))
+      assert(3 == vim.api.nvim_echo({ { 'foo' } }, true, {}))
       vim.keymap.set('n', 'Q', function()
         vim.api.nvim_echo({ { 'Syntax', 23 }, { '\n  - ', 0 }, { 'cCommentL', 439 } }, false, {})
       end)

test/functional/ui/messages_spec.lua

@@ -3674,9 +3674,9 @@ describe('progress-message', function()
     local id5 = api.nvim_echo(
       { { 'test-message 30' } },
       true,
-      { id = 10, kind = 'progress', title = 'TestSuit', percent = 30, status = 'running' }
+      { kind = 'progress', title = 'TestSuit', percent = 30, status = 'running' }
     )
-    eq(10, id5)
+    eq(5, id5)
 
     -- updating progress message does not create new msg-id
     local id5_update = api.nvim_echo(
@@ -3691,7 +3691,7 @@ describe('progress-message', function()
       true,
       { kind = 'progress', title = 'TestSuit', percent = 30, status = 'running' }
     )
-    eq(11, id6)
+    eq(6, id6)
 
     local id7 = api.nvim_echo(
       { { 'supports str-id' } },
@@ -3707,7 +3707,7 @@ describe('progress-message', function()
       true,
       { kind = 'progress', title = 'TestSuit', percent = 30, status = 'running' }
     )
-    eq(13, id8)
+    eq(8, id8)
   end)
 
   it('accepts caller-defined id (string)', function()