From 1b2b715389c5a34f323001f34e4b78b86c9cff8d Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Wed, 18 Mar 2026 21:07:17 -0400 Subject: [PATCH] fix(messages): disallow user-defined integer message-id #38359 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 6 ++++++ src/nvim/lua/executor.c | 2 +- src/nvim/message.c | 17 ++++++++++++----- test/functional/api/vim_spec.lua | 9 +++++++-- test/functional/ui/messages2_spec.lua | 6 +++--- test/functional/ui/messages_spec.lua | 8 ++++---- 6 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index e1ba6caa78..cc35e9afde 100644 --- a/src/nvim/api/vim.c +++ b/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 }; diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c index 3338928739..b14fa990e6 100644 --- a/src/nvim/lua/executor.c +++ b/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 diff --git a/src/nvim/message.c b/src/nvim/message.c index 7b9e3dcb00..3342c53fc5 100644 --- a/src/nvim/message.c +++ b/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); } } diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua index 1321a0eaf7..f9227d9cea 100644 --- a/test/functional/api/vim_spec.lua +++ b/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() diff --git a/test/functional/ui/messages2_spec.lua b/test/functional/ui/messages2_spec.lua index 48f5e1714d..7ce8d92cb2 100644 --- a/test/functional/ui/messages2_spec.lua +++ b/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) diff --git a/test/functional/ui/messages_spec.lua b/test/functional/ui/messages_spec.lua index 13bf17e6c9..7ccd05a078 100644 --- a/test/functional/ui/messages_spec.lua +++ b/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()