From dda30fdfbb9df5f0939c3379777d49c7bc53b137 Mon Sep 17 00:00:00 2001 From: luukvbaal Date: Wed, 6 May 2026 18:25:25 +0200 Subject: [PATCH] fix(messages): disallow source="nvim" progress msg #39315 Problem: Internal progress messages use the "nvim" source (since ff68fd6b), plugins shouldn't be allowed to set the progress message source to "nvim". The message ID used for internal progress messages is not identifiable as such. Solution: Disallow setting opts->source to "nvim" with nvim_echo(). Refactor msg_progress() and callees to bypass nvim_echo(). Prepend message id for internal progress messages with "nvim.". --- runtime/doc/api-ui-events.txt | 6 ++- src/nvim/api/vim.c | 21 ++++---- src/nvim/bufwrite.c | 5 +- src/nvim/fileio.c | 5 +- src/nvim/indent.c | 4 +- src/nvim/insexpand.c | 6 +-- src/nvim/message.c | 78 +++++++++++++--------------- test/functional/api/vim_spec.lua | 2 + test/functional/ui/messages_spec.lua | 18 +++++-- 9 files changed, 79 insertions(+), 66 deletions(-) diff --git a/runtime/doc/api-ui-events.txt b/runtime/doc/api-ui-events.txt index fdfd901f46..917af97951 100644 --- a/runtime/doc/api-ui-events.txt +++ b/runtime/doc/api-ui-events.txt @@ -856,7 +856,11 @@ must handle. "list_cmd" List output for various commands (|:ls|, |:set|, …) "lua_error" Error in |:lua| code "lua_print" |print()| from |:lua| code - "progress" Progress message emitted by |nvim_echo()| + "progress" Progress message emitted by |nvim_echo()|. + Internal progress messages use these ids: + - `nvim.bufwrite ""` + - `nvim.completion` + - `nvim.indent` "quickfix" Quickfix navigation message "rpc_error" Error response from |rpcrequest()| "search_cmd" Entered search command diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index 9ef98a0a22..8aab7e24e3 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -864,10 +864,10 @@ Union(Integer, String) nvim_echo(ArrayOf(Tuple(String, *HLGroupID)) chunks, Bool goto error; }); - VALIDATE_EXP((!is_progress || strequal(opts->status.data, "success") - || strequal(opts->status.data, "failed") - || strequal(opts->status.data, "running") - || strequal(opts->status.data, "cancel")), + VALIDATE_EXP(!is_progress || strequal(opts->status.data, "success") + || strequal(opts->status.data, "failed") + || strequal(opts->status.data, "running") + || strequal(opts->status.data, "cancel"), "status", "success|failed|running|cancel", opts->status.data, { goto error; }); @@ -878,13 +878,16 @@ Union(Integer, String) nvim_echo(ArrayOf(Tuple(String, *HLGroupID)) chunks, Bool goto error; }); - VALIDATE_R((!is_progress || opts->source.size != 0), "opts.source", { + VALIDATE_R(!is_progress || opts->source.size != 0, "opts.source", { + goto error; + }); + VALIDATE_S(!is_progress || !strequal(opts->source.data, "nvim"), "source", opts->source.data, { 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, { + VALIDATE_INT(opts->id.type != kObjectTypeInteger || msg_id_exists(opts->id.data.integer), + "id", opts->id.data.integer, { goto error; }); @@ -917,10 +920,6 @@ Union(Integer, String) nvim_echo(ArrayOf(Tuple(String, *HLGroupID)) chunks, Bool verbose_stop(); // flush now } - if (is_progress) { - do_autocmd_progress(id, hl_msg, &msg_data); - } - if (!needs_clear) { // history takes ownership of `hl_msg` return id; diff --git a/src/nvim/bufwrite.c b/src/nvim/bufwrite.c index 4a14a1ecb2..cf158bf545 100644 --- a/src/nvim/bufwrite.c +++ b/src/nvim/bufwrite.c @@ -1669,6 +1669,9 @@ restore_backup: #endif if (!filtering) { add_quoted_fname(IObuff, IOSIZE, buf, fname); + // Append the filename to the message ID. + char msg_id[IOSIZE + 14] = "nvim.bufwrite "; + strncat(msg_id + 14, IObuff, strlen(IObuff) - 1); bool insert_space = false; if (write_info.bw_conv_error) { xstrlcat(IObuff, _(" CONVERSION ERROR"), IOSIZE); @@ -1707,7 +1710,7 @@ restore_backup: xstrlcat(IObuff, shortmess(SHM_WRI) ? _(" [w]") : _(" written"), IOSIZE); } } - set_keep_msg(msg_progress(IObuff, "bufwrite", "success", 0, true, true), 0); + set_keep_msg(msg_progress(IObuff, msg_id, "success", 0, true, true), 0); } // When written everything correctly: reset 'modified'. Unless not diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c index 69a9ba0e2d..022509b933 100644 --- a/src/nvim/fileio.c +++ b/src/nvim/fileio.c @@ -132,7 +132,10 @@ void filemess(buf_T *buf, char *name, char *s) msg_scrolled_ign = true; // may truncate the message to avoid a hit-return prompt if (*s == NUL) { - msg_progress(IObuff, "bufwrite", "running", 0, false, true); + // Append the filename to the message ID. + char msg_id[IOSIZE + 14] = "nvim.bufwrite "; + strncat(msg_id + 14, IObuff, strlen(IObuff) - 1); + msg_progress(IObuff, msg_id, "running", 0, false, true); } else { msg_outtrans(msg_may_trunc(false, IObuff), 0, false); } diff --git a/src/nvim/indent.c b/src/nvim/indent.c index b10173135e..9c0a9d371c 100644 --- a/src/nvim/indent.c +++ b/src/nvim/indent.c @@ -1008,7 +1008,7 @@ void op_reindent(oparg_T *oap, Indenter how) // Restore cursor to avoid redrawing curwin in msg_show callback. linenr_T save_lnum = curwin->w_cursor.lnum; curwin->w_cursor.lnum = start_lnum; - msg_progress(IObuff, "indent", "running", 0, true, false); + msg_progress(IObuff, "nvim.indent", "running", 0, true, false); curwin->w_cursor.lnum = save_lnum; } @@ -1050,7 +1050,7 @@ void op_reindent(oparg_T *oap, Indenter how) i = oap->line_count - (i + 1); snprintf(IObuff, IOSIZE, NGETTEXT("%" PRId64 " line indented ", "%" PRId64 " lines indented ", i), (int64_t)i); - msg_progress(IObuff, "indent", "success", 0, true, false); + msg_progress(IObuff, "nvim.indent", "success", 0, true, false); } if ((cmdmod.cmod_flags & CMOD_LOCKMARKS) == 0) { // set '[ and '] marks diff --git a/src/nvim/insexpand.c b/src/nvim/insexpand.c index 62c1cfd185..4c7b9b4902 100644 --- a/src/nvim/insexpand.c +++ b/src/nvim/insexpand.c @@ -1945,7 +1945,7 @@ static void ins_compl_files(int count, char **files, bool thesaurus, int flags, FILE *fp = os_fopen(files[i], "r"); // open dictionary file if (flags != DICT_EXACT && !shortmess(SHM_COMPLETIONSCAN) && !compl_autocomplete) { vim_snprintf(IObuff, IOSIZE, _("Scanning dictionary: %s"), files[i]); - msg_progress(IObuff, "completion", "running", HLF_R, false, true); + msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true); } if (fp == NULL) { @@ -3834,7 +3834,7 @@ static int process_next_cpt_value(ins_compl_next_state_T *st, int *compl_type_ar : st->ins_buf->b_sfname == NULL ? st->ins_buf->b_fname : st->ins_buf->b_sfname); - msg_progress(IObuff, "completion", "running", HLF_R, false, true); + msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true); } } else if (*st->e_cpt == NUL) { status = INS_COMPL_CPT_END; @@ -3868,7 +3868,7 @@ static int process_next_cpt_value(ins_compl_next_state_T *st, int *compl_type_ar compl_type = CTRL_X_TAGS; if (!shortmess(SHM_COMPLETIONSCAN) && !compl_autocomplete) { vim_snprintf(IObuff, IOSIZE, "%s", _("Scanning tags.")); - msg_progress(IObuff, "completion", "running", HLF_R, false, true); + msg_progress(IObuff, "nvim.completion", "running", HLF_R, false, true); } } } diff --git a/src/nvim/message.c b/src/nvim/message.c index 5b5a6ba2f8..b40dbe2bd0 100644 --- a/src/nvim/message.c +++ b/src/nvim/message.c @@ -315,7 +315,7 @@ static int is_multihl = 0; /// /// @param hl_msg Message chunks /// @param msg_data Additional data for progress messages -static HlMessage format_progress_message(HlMessage hl_msg, MessageData *msg_data) +static bool format_progress_message(HlMessage *hl_msg, MessageData *msg_data) { HlMessage updated_msg = KV_INITIAL_VALUE; // progress messages are special. displayed as "title: percent% msg" @@ -333,28 +333,26 @@ static HlMessage format_progress_message(HlMessage hl_msg, MessageData *msg_data } else if (strequal(msg_data->status.data, "cancel")) { hl_id = syn_check_group("WarningMsg", STRLEN_LITERAL("WarningMsg")); } - kv_push(updated_msg, - ((HlMessageChunk){ .text = copy_string(msg_data->title, NULL), .hl_id = hl_id })); - kv_push(updated_msg, ((HlMessageChunk){ .text = cstr_to_string(": "), .hl_id = 0 })); + kv_push(updated_msg, ((HlMessageChunk){ copy_string(msg_data->title, NULL), hl_id })); + kv_push(updated_msg, ((HlMessageChunk){ cstr_to_string(": "), 0 })); } if (msg_data->percent >= 0) { char percent_buf[10]; vim_snprintf(percent_buf, sizeof(percent_buf), "%3ld%% ", (long)msg_data->percent); String percent = cstr_to_string(percent_buf); int hl_id = syn_check_group("WarningMsg", STRLEN_LITERAL("WarningMsg")); - kv_push(updated_msg, ((HlMessageChunk){ .text = percent, .hl_id = hl_id })); + kv_push(updated_msg, ((HlMessageChunk){ percent, hl_id })); } if (kv_size(updated_msg) != 0) { - for (uint32_t i = 0; i < kv_size(hl_msg); i++) { - kv_push(updated_msg, - ((HlMessageChunk){ .text = copy_string(kv_A(hl_msg, i).text, NULL), - .hl_id = kv_A(hl_msg, i).hl_id })); + for (uint32_t i = 0; i < kv_size(*hl_msg); i++) { + HlMessageChunk chunk = kv_A(*hl_msg, i); + kv_push(updated_msg, ((HlMessageChunk){ copy_string(chunk.text, NULL), chunk.hl_id })); } - return updated_msg; - } else { - return hl_msg; + *hl_msg = updated_msg; + return true; } + return false; } /// Print message chunks, each with their own highlight ID. @@ -377,33 +375,30 @@ MsgID msg_multihl(MsgID id, HlMessage hl_msg, const char *kind, bool history, bo abort(); } + bool hl_msg_updated = false; // don't display progress message in cmd when target doesn't have cmd - if (strequal(kind, "progress") && (progress_msg_target & PROGRESS_TARGET_CMD) == 0) { - *needs_msg_clear = true; - return id; + if (strequal(kind, "progress")) { + do_autocmd_progress(id, hl_msg, msg_data); + if ((progress_msg_target & PROGRESS_TARGET_CMD) == 0) { + *needs_msg_clear = true; + return id; + } + if (msg_data && format_progress_message(&hl_msg, msg_data)) { + *needs_msg_clear = true; + hl_msg_updated = true; + } } no_wait_return++; msg_start(); msg_clr_eos(); bool need_clear = false; - bool hl_msg_updated = false; if (kind != NULL) { msg_ext_set_kind(kind); } msg_ext_skip_flush = true; msg_ext_id = id; - // progress message are special displayed as "title: percent% msg" - if (strequal(kind, "progress") && msg_data) { - HlMessage formated_message = format_progress_message(hl_msg, msg_data); - if (formated_message.items != hl_msg.items) { - *needs_msg_clear = true; - hl_msg_updated = true; - hl_msg = formated_message; - } - } - for (uint32_t i = 0; i < kv_size(hl_msg); i++) { HlMessageChunk chunk = kv_A(hl_msg, i); is_multihl++; @@ -416,7 +411,7 @@ MsgID msg_multihl(MsgID id, HlMessage hl_msg, const char *kind, bool history, bo } if (history && kv_size(hl_msg)) { - msg_hist_add_multihl(hl_msg, false, msg_data); + msg_hist_add_multihl(hl_msg, false); } msg_ext_skip_flush = false; @@ -1101,25 +1096,22 @@ char *msg_may_trunc(bool force, char *s) char *msg_progress(char *s, char *id, char *status, int hl_id, bool hist, bool trunc) { - Error err = ERROR_INIT; - Dict(echo_opts) opts = { - .kind = cstr_as_string("progress"), - .source = cstr_as_string("nvim"), - .status = cstr_as_string(status), - .id = CSTR_AS_OBJ(id), - }; if (hist && (!trunc || ui_has(kUIMessages))) { msg_hist_add(s, -1, 0); } if (trunc) { s = msg_may_trunc(false, s); } - MAXSIZE_TEMP_ARRAY(chunk, 2); - MAXSIZE_TEMP_ARRAY(chunks, 1); - ADD_C(chunk, CSTR_AS_OBJ(s)); - ADD_C(chunk, INTEGER_OBJ(hl_id)); - ADD_C(chunks, ARRAY_OBJ(chunk)); - nvim_echo(chunks, false, &opts, &err); + bool clear = false; + MessageData data = { + .source = cstr_as_string("nvim"), + .status = cstr_as_string(status), + .percent = -1, + }; + HlMessage chunks = KV_INITIAL_VALUE; + kv_push(chunks, ((HlMessageChunk){ cstr_as_string(s), hl_id })); + msg_multihl(CSTR_AS_OBJ(id), chunks, "progress", false, false, &data, &clear); + kv_destroy(chunks); ui_flush(); return s; } @@ -1153,7 +1145,7 @@ static void msg_hist_add(const char *s, int len, int hl_id) HlMessage msg = KV_INITIAL_VALUE; kv_push(msg, ((HlMessageChunk){ text, hl_id })); - msg_hist_add_multihl(msg, false, NULL); + msg_hist_add_multihl(msg, false); } static bool do_clear_hist_temp = true; @@ -1190,7 +1182,7 @@ void do_autocmd_progress(MsgID msg_id, HlMessage msg, MessageData *msg_data) kv_destroy(messages); } -static void msg_hist_add_multihl(HlMessage msg, bool temp, MessageData *msg_data) +static void msg_hist_add_multihl(HlMessage msg, bool temp) { if (do_clear_hist_temp) { msg_hist_clear_temp(); @@ -3417,7 +3409,7 @@ void msg_ext_ui_flush(void) xfree(chunk); } xfree(tofree->items); - msg_hist_add_multihl(msg, true, NULL); + msg_hist_add_multihl(msg, true); } xfree(tofree); msg_ext_overwrite = false; diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua index 6ae2e5be47..0c3a8f9af2 100644 --- a/test/functional/api/vim_spec.lua +++ b/test/functional/api/vim_spec.lua @@ -3888,6 +3888,8 @@ describe('API', function() 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' })) + local opts = { kind = 'progress', source = 'nvim', status = 'success' } + eq("Invalid 'source': 'nvim'", pcall_err(api.nvim_echo, { { '' } }, 1, opts)) end) it('should clear cmdline message before echo', function() diff --git a/test/functional/ui/messages_spec.lua b/test/functional/ui/messages_spec.lua index 0307aa563b..597bb212e0 100644 --- a/test/functional/ui/messages_spec.lua +++ b/test/functional/ui/messages_spec.lua @@ -95,7 +95,12 @@ describe('ui/ext_messages', function() {1:~ }|*3 ]], messages = { - { content = { { writemsg } }, history = true, id = 'bufwrite', kind = 'progress' }, + { + content = { { writemsg } }, + history = true, + id = 'nvim.bufwrite "Xtest_functional_ui_messages_spec"', + kind = 'progress', + }, { content = { { 'W10: Warning: Changing a readonly file', 19, 'WarningMsg' } }, history = true, @@ -587,7 +592,12 @@ describe('ui/ext_messages', function() {1:~ }|*2 ]], messages = { - { content = { { '3 lines indented ' } }, history = true, id = 'indent', kind = 'progress' }, + { + content = { { '3 lines indented ' } }, + kind = 'progress', + id = 'nvim.indent', + history = true, + }, }, }) end) @@ -1398,7 +1408,7 @@ stack traceback: { content = { { string.format('"%s" [New] 0L, 0B written', fname) } }, kind = 'progress', - id = 'bufwrite', + id = 'nvim.bufwrite "Xtest_functional_ui_messages_spec"', history = true, }, }, @@ -1645,7 +1655,7 @@ stack traceback: { content = { { 'Scanning tags.', 6, 'Question' } }, kind = 'progress', - id = 'completion', + id = 'nvim.completion', }, }, showmode = {