[vim/vim] fix(tag): tagfunc commands with special chars are truncated (PR #20790)
Mao-Yining
Problem: When tagfunc returns a tag with cmd containing special
characters like ';' or '|', they are incorrectly parsed as
command separators. This causes the actual command to be
truncated, breaking tag jumps that use such commands.
Solution: Wrap Ex commands from tagfunc with ':' and '|' prefix/suffix
in the internal tag format. Skip the leading ':' in
parse_match() so taglist() returns the original command.
fixes: #20788
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/20790
Commit Summary
- 2bca8cd fix(tag): tagfunc commands with special chars are truncated
File Changes
(2 files)
- M src/tag.c (4)
- M src/testdir/test_tagfunc.vim (19)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20790)Thanks, but I think this seems to break the following tagfunc:
" A tagfunc 'cmd' ending in '|' must execute verbatim on a tag jump. The " ':'/'|' wrapping adds an extra bar that :normal consumes, so the cursor " ends at column 1 instead of the intended column 4. func Test_tagfunc_cmd_jump_trailing_bar() call writefile(['line one', 'line two', 'abcdefghij'], 'Xtagcmd', 'D') func MyTagFunc(pat, flags, info) return [{'name': 'a', 'filename': 'Xtagcmd', 'cmd': 'normal 3G4|', 'kind': 'f'}] endfunc set tagfunc=MyTagFunc enew tag a call assert_equal([3, 4], [line('.'), col('.')]) bw! set tagfunc= delfunc MyTagFunc endfunc
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#20790)Thanks, but I think this seems to break the following tagfunc:
Thanks for that catch. I will fix it.
However, I don't know why ASAN is always failed. Could anybody please take a look?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20790)However, I don't know why ASAN is always failed. Could anybody please take a look?
This seems to be caused when has_extra is false, then *p++ = '|'; overwrites the NUL and passes over it.
I think you need this change on top of it:
diff --git a/src/tag.c b/src/tag.c index 0620c8dc6..d08b89086 100644 --- a/src/tag.c +++ b/src/tag.c @@ -1601,6 +1601,7 @@ find_tagfunc_tags( STRCPY(p, res_cmd); p += STRLEN(p); *p++ = '|'; + *p = NUL; if (has_extra) {
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#20790)*p = NUL;
Thanks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20790)Hmm? When it comes to ctags, only a line number (e.g. 53) or a search pattern (e.g. /foo/, ?bar?) gets set in cmd. I don't think it's designed to handle cmd values other than those.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#20790)Hmm? When it comes to ctags, only a line number (e.g.
53) or a search pattern (e.g./foo/,?bar?) gets set incmd. I don't think it's designed to handlecmdvalues other than those.
tagfunc's document said that 'cmd' can be any command. Maybe you want change the document? Anyway, the tagfunc was broken for just a missing '/' is absolutely a strange thing.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20790)It was actually mentioned in the documentation, though not explained in detail.
7. Using 'tagfunc' *tag-function*
(snip)...
*E987*
(snip)...
cmd Ex command used to locate the tag in the file. This
can be either an Ex search pattern or a line number.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20790)Note that if the dictionary does not contain the kind key, the value of cmd will not be changed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#20790)Anyway the current behaviour that make mistakes silently is not a good things. If you don’t like this patch, from my point of view, a check of tagfunc’s return value is also needed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20790)This patch regresses a supported case: a cmd that is a search pattern.
:help tag-function documents that cmd "can be either an Ex search pattern
or a line number", so a search-pattern cmd must keep working. With this
patch, a tag jump to such an entry no longer lands on the matched column.
Reproduce with a cmd of /bar/:
func Test_tagfunc_search_pattern_jump() call writefile(['line one', 'line two', 'foo bar baz'], 'Xtagsp', 'D') func MyTagFunc(pat, flags, info) return [{'name': 'a', 'filename': 'Xtagsp', 'cmd': '/bar/', 'kind': 'f'}]
endfunc set tagfunc=MyTagFunc enew tag a
" Before this patch the cursor lands on the matched column: [3, 5]. " After this patch it ends at the line head: [3, 1]. call assert_equal([3, 5], [line('.'), col('.')])
bw! set tagfunc= delfunc MyTagFunc endfunc
Measured on an otherwise identical build:
- before:
line=3 col=5(on thebofbar) - after:
line=3 col=1(line head)
Root cause: the unconditional : prefix and | suffix turn the internal tag
line into :/bar/|;"<TAB>f. In jumpto_tag(), find_extra() takes the
search-pattern branch on the leading /, consumes /bar/, then hits |
instead of ;" and returns FAIL. pbuf is therefore not trimmed, the "search
command with nothing following" check becomes false, and the jump falls through
to do_cmdline_cmd(":/bar/| ...") instead of do_search(). The Ex :/bar/
moves to the line but not the matched column. The search-specific handling is
lost too: the case-insensitive retry and the "^tagname\s*(" guess fallback.
The wrapping is applied to every cmd, including the two forms the
documentation supports, so a documented case breaks. That looks like a blocker
for this approach.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#20790)> This patch regresses a supported case: a `cmd` that is a search pattern.
________________________________
发件人: h_east ***@***.***>
发送时间: Monday, 20 July 2026 12:28:58
收件人: vim/vim ***@***.***>
抄送: Mao-Yining ***@***.***>; Author ***@***.***>
主题: Re: [vim/vim] fix(tag): tagfunc commands with special chars are truncated (PR #20790)
[https://avatars.githubusercontent.com/u/518808 left a comment (vim/vim#20790)<https://github.com/vim/vim/pull/20790#issuecomment-5018784989>
This patch regresses a supported case: a cmd that is a search pattern.
:help tag-function documents that cmd "can be either an Ex search pattern
or a line number", so a search-pattern cmd must keep working. With this
patch, a tag jump to such an entry no longer lands on the matched column.
Reproduce with a cmd of /bar/:
func Test_tagfunc_search_pattern_jump()
call writefile(['line one', 'line two', 'foo bar baz'], 'Xtagsp', 'D')
func MyTagFunc(pat, flags, info)
return [{'name': 'a', 'filename': 'Xtagsp', 'cmd': '/bar/', 'kind': 'f'}]
endfunc
set tagfunc=MyTagFunc
enew
tag a
" Before this patch the cursor lands on the matched column: [3, 5].
" After this patch it ends at the line head: [3, 1].
call assert_equal([3, 5], [line('.'), col('.')])
bw!
set tagfunc=
delfunc MyTagFunc
endfunc
Measured on an otherwise identical build:
* after: line=3 col=1 (line head)
Root cause: the unconditional : prefix and | suffix turn the internal tag
line into :/bar/|;"<TAB>f. In jumpto_tag(), find_extra() takes the
search-pattern branch on the leading /, consumes /bar/, then hits |
instead of ;" and returns FAIL. pbuf is therefore not trimmed, the "search
command with nothing following" check becomes false, and the jump falls through
to do_cmdline_cmd(":/bar/| ...") instead of do_search(). The Ex :/bar/
moves to the line but not the matched column. The search-specific handling is
lost too: the case-insensitive retry and the "^tagname\s*(" guess fallback.
The wrapping is applied to every cmd, including the two forms the
documentation supports, so a documented case breaks. That looks like a blocker
for this approach.
Reply to this email directly, view it on GitHub<https://github.com/vim/vim/pull/20790#issuecomment-5018784989>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AYJDXIVP6R3JC2G7G2DMQMD5FWNYVAVCNFSNUABEKJSXA33TNF2G64TZHM2DAOJZG42DQMR3JFZXG5LFHM2DSMRSGA2TOMBYHCQXMAQ>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AYJDXIW76WQMQL5HBP5QUUL5FWNYVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBRHA3TQNBZHA42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/AYJDXIQ26Y2AL6NQV3LHNOD5FWNYVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBRHA3TQNBZHA42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20790)@mao-yining
Following up on the search-pattern regression I flagged above: here is a
version that keeps the generic-command support you are after, without that
regression. The one change in approach is that a terminator is added only to a
generic command, never to a search pattern or a line number.
Design
cmd is classified into three kinds, the way a tags file address is read:
- line number, or a
/pat//?pat?search pattern: an address,
stored as-is with no added terminator. Search patterns therefore stay on
thedo_search()path and keep landing on the matched column, so there is no
column regression. - any other Ex command: generic, stored with a
|;"terminator, exactly as
tags-file-formatalready requires ("When {tagaddress} is not a line number
or search pattern, then {term} must be|;""). It runs through
do_cmdline_cmd()undersecure+ sandbox, the same as a tags file command. - a value that cannot go into a tag line (an unterminated pattern, trailing
junk, or a<Tab>/newline outside a pattern): rejected withE987.
Because a generic command never starts with /, ? or a digit,
jumpto_tag() never mistakes it for a search, so the : prefix is not needed.
parse_match() already drops a trailing | (command_end = p - 1), so
taglist() round-trips with no change there; jumpto_tag() only needs a
one-line "drop the trailing bar" fix so the command is not executed with an
empty separator.
The |;" terminator is also what keeps the kind field intact: without it,
find_extra() cannot find the end of a generic command, so cmd swallows the
;"<Tab>kind part and kind comes back empty (the original #20781 symptom).
Security
A generic command runs under secure = 1 + sandbox, identical to a tags file
address, so delete() / writefile() / system() / :! are all blocked.
Verified: a tagfunc returning cmd: "call delete('X')" stops with E12 and
the file survives.
Tests
test_tagfunc covers the round-trip (including the kind field, which a
generic command dropped before, #20781), the jump with no regression (search /
line number / generic all land on the same column), and the sandbox block.
make test_tagfunc and make test_tagjump pass.
Feel free to take the patch below as-is or adapt it.
Patch (terminate only generic commands, 3 files)diff --git a/runtime/doc/tagsrch.txt b/runtime/doc/tagsrch.txt index e381d5fc0..76aec93aa 100644 --- a/runtime/doc/tagsrch.txt +++ b/runtime/doc/tagsrch.txt @@ -936,7 +936,11 @@ include the following entries and each value must be a string: either relative to the current directory or a full path. cmd Ex command used to locate the tag in the file. This - can be either an Ex search pattern or a line number. + can be a line number, a search pattern or any other + Ex command, like in a tags file (see + |tags-file-format|). A search pattern must start and + end with "/" or "?". An invalid value results in + |E987|. Note that the format is similar to that of |taglist()|, which makes it possible to use its output to generate the result. The following fields are optional: diff --git a/src/tag.c b/src/tag.c index 96afc699b..98b98897f 100644 --- a/src/tag.c +++ b/src/tag.c @@ -1425,6 +1425,38 @@ prepare_pats(pat_T *pats, int has_re) } #ifdef FEAT_EVAL +// Classification of a "cmd" value returned by a tagfunc. +typedef enum { + TAGCMD_INVALID, // cannot be stored in a tag line + TAGCMD_ADDRESS, // line number or /pat/ or ?pat? (no "|" needed) + TAGCMD_GENERIC // any other Ex command: needs a "|" terminator +} tagcmd_T; + +/* + * Classify "cmd" from a tagfunc result (see tagcmd_T), like a tags file + * address, to decide whether a "|" terminator must be appended before storing + * it in a tag line. + */ + static tagcmd_T +tagfunc_cmd_kind(char_u *cmd) +{ + if (VIM_ISDIGIT(*cmd)) + return *skipdigits(cmd) == NUL ? TAGCMD_ADDRESS : TAGCMD_INVALID; + if (*cmd == '/' || *cmd == '?') + { + // A Tab inside the pattern is fine (Universal Ctags emits one for a + // tab-indented line); only trailing content after the closing + // delimiter, which would break the tag line fields, is rejected. + char_u *end = skip_regexp(cmd + 1, *cmd, FALSE); + + return (*end == *cmd && end[1] == NUL) + ? TAGCMD_ADDRESS : TAGCMD_INVALID; + } + if (vim_strpbrk(cmd, (char_u *)"\t\r\n") != NULL) + return TAGCMD_INVALID; + return TAGCMD_GENERIC; +} + /* * Call the user-defined function to generate a list of tags used by * find_tags(). @@ -1570,6 +1602,15 @@ find_tagfunc_tags( break; } + tagcmd_T cmdkind = tagfunc_cmd_kind(res_cmd); + if (cmdkind == TAGCMD_INVALID) + { + emsg(_(e_invalid_return_value_from_tagfunc)); + break; + } + if (cmdkind == TAGCMD_GENERIC) + len += 3; // need space for "|;\"" + if (name_only) mfp = vim_strsave(res_name); else @@ -1599,7 +1640,10 @@ find_tagfunc_tags( STRCPY(p, res_cmd); p += STRLEN(p); - if (has_extra) + if (cmdkind == TAGCMD_GENERIC) + *p++ = '|'; // terminate the command + + if (cmdkind == TAGCMD_GENERIC || has_extra) { STRCPY(p, ";\""); p += STRLEN(p); @@ -3781,7 +3825,12 @@ jumpto_tag( str++; if (find_extra(&str) == OK) { - pbuf_end = str; + // Drop a trailing "|" that terminates a generic Ex command, so it + // is not executed as an empty command separator. + if (str > pbuf && str[-1] == '|') + pbuf_end = str - 1; + else + pbuf_end = str; *pbuf_end = NUL; } } diff --git a/src/testdir/test_tagfunc.vim b/src/testdir/test_tagfunc.vim index 4ce9d2164..4427839be 100644 --- a/src/testdir/test_tagfunc.vim +++ b/src/testdir/test_tagfunc.vim @@ -456,4 +456,84 @@ func Test_tagfunc_deletes_lines() set tagfunc= endfunc +" The 'cmd' field can be a line number, a search pattern or any other Ex +" command, like in a tags file. A value that would break the tag line (an +" unterminated pattern, trailing junk, or a <Tab> outside a pattern) is +" rejected with E987 instead of silently corrupting the result. +func Test_tagfunc_invalid_cmd() + func InvalidCmd(pat, flags, info) + return [{'name': 'a', 'filename': 'Xtest', 'cmd': a:pat, 'kind': 'f'}] + endfunc + set tagfunc=InvalidCmd + + " unterminated search pattern + call assert_fails("call taglist('/foo')", 'E987:') + " trailing content after the closing delimiter of a pattern + call assert_fails('call taglist("/foo/\tbar")', 'E987:') + " a line number with trailing junk + call assert_fails("call taglist('3G5')", 'E987:') + " a <Tab> in a generic command would break the tag line fields + call assert_fails('call taglist("call\tcursor(3, 5)")', 'E987:') + + " valid forms are accepted and returned unchanged: line number, delimited + " patterns, and a Tab inside a pattern (Universal Ctags emits one like this + " for a tab-indented line) + call assert_equal('42', taglist('42')[0].cmd) + call assert_equal('/foo/', taglist('/foo/')[0].cmd) + call assert_equal('?foo?', taglist('?foo?')[0].cmd) + call assert_equal("/^\tint foo$/", taglist("/^\tint foo$/")[0].cmd) + + " a generic command round-trips with its kind field intact: before the fix + " it swallowed the ';"<Tab>kind' part, so kind came back empty (#20781) + let t = taglist('call cursor(3, 5)')[0] + call assert_equal('call cursor(3, 5)', t.cmd) + call assert_equal('f', t.kind) + + set tagfunc& + delfunc InvalidCmd +endfunc + +" A tagfunc 'cmd' is executed on a tag jump. A search pattern must reach the +" matched column (no regression from the do_search() path), and a line number +" or a generic Ex command must position the cursor as written. +func Test_tagfunc_cmd_jump() + call writefile(['line one', 'line two', 'foo bar baz'], 'Xtagcmd', 'D') + func JumpCmd(pat, flags, info) + return [{'name': 'a', 'filename': 'Xtagcmd', 'cmd': s:cmd, 'kind': 'f'}] + endfunc + set tagfunc=JumpCmd + + " '5|' / cursor col 5 / the 'b' of 'bar' all land on [3, 5] + for cmd in ['/bar/', 'call cursor(3, 5)', 'normal! 3G5|'] + let s:cmd = cmd + enew + tag a + call assert_equal([3, 5], [line('.'), col('.')], 'cmd: ' .. cmd) + bw! + endfor + + set tagfunc& + delfunc JumpCmd +endfunc + +" A generic Ex command from a tagfunc runs under 'secure' and the sandbox, just +" like one from a tags file, so it cannot delete a file or run a shell. +func Test_tagfunc_cmd_secure() + call writefile(['one', 'two', 'three'], 'Xtagcmd', 'D') + call writefile(['keep me'], 'Xvictim', 'D') + func EvilTagFunc(pat, flags, info) + return [{'name': 'a', 'filename': 'Xtagcmd', + \ 'cmd': "call delete('Xvictim')", 'kind': 'f'}] + endfunc + set tagfunc=EvilTagFunc + + enew + call assert_fails('tag a', 'E12:') + call assert_true(filereadable('Xvictim')) + + bw! + set tagfunc& + delfunc EvilTagFunc +endfunc + " vim: shiftwidth=2 sts=2 expandtab
This patch was written with AI assistance (Claude). If you take it, please
keep the disclosure by adding the following trailer to the commit, per
CONTRIBUTING ("Using AI"):
Co-Authored-By: Claude Opus 4.8 (1M context) <nor...@anthropic.com>
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#20790)@h-east I've been busy with other things lately. If you have time, please open a separate PR with your solution. I am sorry for that.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20790)I've done it, so I'm going to close this issue.
This is my first patch to Vim core
The first enemy happened to be a bit too "hacky." Please give it another try.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.