Problem: [security]: stack-buffer-overflow in option callback functions
Solution: pass size of errbuf down the call stack, use snprintf()
instead of sprintf()
We pass the error buffer down to the option callback functions, but in
some parts of the code, we simply use sprintf(buf) to write into the error
buffer, which can overflow.
So let's pass down the length of the error buffer and use sprintf(buf, size)
instead.
Reported by @henices, thanks!
b39b240c38
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: use-after-free in win-enter
Solution: validate window pointer before calling win_enter()
win_goto() may stop visual mode, if it is active. However, this may in
turn trigger the ModeChanged autocommand, which could potentially free
the wp pointer which was valid before now became stale and points to now
freed memory.
So before calling win_enter(), let's verify one more time, that the
wp pointer still points to a valid window structure.
Reported by @henices, thanks!
eec0c2b3a4
Co-authored-by: Christian Brabandt <cb@256bit.org>
This reverts commit fe30d8ccef.
The original commit intends to prevent heap-use-after-free with EXITFREE
caused by changedtick_di, which is no longer a problem.
Freeing buffers after freeing variables will cause heap-use-after-free
with EXITFREE when a partial is used as prompt callback.
Some escape sequences (in particular, OSC 52 paste responses) can be
very large, even unbounded in length. These can easily overflow
termkey's internal buffer. In order to process these long sequences,
dynamically grow termkey's internal buffer.
Problem: ml_get error when scrolling after delete
Solution: mark topline to be validated in main_loop
if it is larger than current buffers line
count
reset_lnums() is called after e.g. TextChanged autocommands and it may
accidentally cause curwin->w_topline to become invalid, e.g. if the
autocommand has deleted some lines.
So verify that curwin->w_topline points to a valid line and if not, mark
the window to have w_topline recalculated in main_loop() in
update_topline() after reset_lnums() returns.
fixes: vim/vim#13568fixes: vim/vim#13578c4ffeddfe5
The error doesn't happen in Nvim because Nvim triggers TextChanged after
calling update_topline().
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: Signcolumn width does not increase when ranged sign does not
start at sentinel line.
Solution: Handle paired range of added sign when checking signcols.
decor->text.str pointer must go. This removes it for conceal char,
in preparation for a larger PR which will also handle the sign case.
By actually allowing composing chars for a conceal chars, this
becomes a feature and not just a refactor, as a bonus.
When parsing with a range, languagetree looks up injections and adds
them if needed. This explicitly invalidates parser, making `is_valid`
report `false` both when including and excluding children.
This is an attempt to describe desired behaviour of `is_valid` in tests,
with what ended up being a single line change to satisfy them.
Problem: File info disappears immediately when 'cmdheight' has just
decreased due to switching tabpage and 'shortmess' doesn't
contain 'o' or 'O'.
Solution: Make sure msg_row isn't smaller than cmdline_row.
fixes: vim/vim#13560closes: vim/vim#1356140ed6711bd
Problem: Unable to identify legacy signs when fetching extmarks with
`nvim_buf_get_extmarks()`.
Solution: Add "sign_name" to the extmark detail array.
Add some misc. changes as follow-up to #25724
Remove the monolithic Decoration struct. Before this change, each extmark
could either represent just a hl_id + priority value as a inline
decoration, or it would take a pointer to this monolitic 112 byte struct
which has to be allocated.
This change separates the decorations into two pieces: DecorSignHighlight
for signs, highlights and simple set-flag decorations (like spell,
ui-watched), and DecorVirtText for virtual text and lines.
The main separation here is whether they are expected to allocate more
memory. Currently this is not really true as sign text has to be an
allocated string, but the plan is to get rid of this eventually (it can
just be an array of two schar_T:s). Further refactors are expected to
improve the representation of each decoration kind individually. The
goal of this particular PR is to get things started by cutting the
Gordian knot which was the monolithic struct Decoration.
Now, each extmark can either contain chained indicies/pointers to
these kinds of objects, or it can fit a subset of DecorSignHighlight
inline.
The point of this change is not only to make decorations smaller in
memory. In fact, the main motivation is to later allow them to grow
_larger_, but on a dynamic, on demand fashion. As a simple example, it
would be possible to augment highlights to take a list of multiple
`hl_group`:s, which then would trivially map to a chain of multiple
DecorSignHighlight entries.
One small feature improvement included with this refactor itself, is
that the restriction that extmarks cannot be removed inside a decoration
provider has been lifted. These are instead safely lifetime extended
on a "to free" list until the current iteration of screen drawing is done.
NB: flags is a mess. but DecorLevel is useless, this slightly less so
Quick fix as follow up to https://github.com/neovim/neovim/pull/26108
kqueue only reports events on a watched folder itself, not for files
created or deleted within. So the approach the PR took doesn't work on FreeBSD.
We'll either need to bring back polling for it, combine watching with manual
file tracking, or disable LSP file watching on FreeBSD
Should help with https://github.com/neovim/neovim/issues/23291
On linux `new_fs_event` doesn't support recursive watching, but we can
still use it to watch folders.
The downside of this approach is that we may end up sending some false
`Deleted` events. For example, if you save a file named `foo` there will
be a intermediate `foo~` due to the save mechanism of neovim.
The events we get from vim.uv in that case are:
- rename: foo~
- rename: foo~
- rename: foo
- rename: foo
- change: foo
- change: foo
The mechanism in this PR uses a debounce to reduce this to:
- deleted: foo~
- changed: foo
`foo~` will be the false positive.
I suspect that for the LSP case this is good enough. If not, we may need
to follow up on this and keep a table in memory that tracks available
files.
Problem: The legacy signlist data structures and associated functions are
redundant since the introduction of extmark signs.
Solution: Store signs defined through the legacy commands in a hashmap, placed
signs in the extmark tree. Replace signlist associated functions.
Usage of the legacy sign commands should yield no change in behavior with the
exception of:
- "orphaned signs" are now always removed when the line it is placed on is
deleted. This used to depend on the value of 'signcolumn'.
- It is no longer possible to place multiple signs with the same identifier
in a single group on multiple lines. This will now move the sign instead.
Moreover, both signs placed through the legacy sign commands and through
|nvim_buf_set_extmark()|:
- Will show up in both |sign-place| and |nvim_buf_get_extmarks()|.
- Are displayed by increasing sign identifier, left to right.
Extmark signs used to be ordered decreasingly as opposed to legacy signs.
Problem: buffer text with composing chars are converted from UTF-8
to an array of up to seven UTF-32 values and then converted back
to UTF-8 strings.
Solution: Convert buffer text directly to UTF-8 based schar_T values.
The limit of the text size is now in schar_T bytes, which is currently
31+1 but easily could be raised as it no longer multiplies the size
of the entire screen grid when not used, the full size is only required
for temporary scratch buffers.
Also does some general cleanup to win_line text handling, which was
unnecessarily complicated due to multibyte rendering being an "opt-in"
feature long ago. Nowadays, a char is just a char, regardless if it consists
of one ASCII byte or multiple bytes.
Problem: buffer-overflow in trunc_string()
Solution: Add NULL at end of buffer
Currently trunc_string() assumes that when the string is too long,
buf[e-1] will always be writeable. But that assumption may not always be
true. The condition currently looks like this
else if (e + 3 < buflen)
[...]
else
{
// can't fit in the "...", just truncate it
buf[e - 1] = NUL;
}
but this means, we may run into the last else clause with e still being
larger than buflen. So a buffer overflow occurs.
So instead of using `buf[e - 1]`, let's just always
truncate at `buf[buflen - 1]` which should always be writable.
3bd7fa12e1
vim-patch:9.0.2004: Missing test file
Problem: Missing test file
Solution: git-add the file to the repo
closes: vim/vim#13305d4afbdd071
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: [security]: FPE in adjust_plines_for_skipcol
Solution: don't divide by zero, return zero
Prevent a floating point exception when calculating w_skipcol (which can
happen with a small window when the number option is set and cpo+=n).
Add a test to verify
cb0b99f067
Co-authored-by: Christian Brabandt <cb@256bit.org>
The 'termsync' option enables a mode (provided the underlying terminal
supports it) where all screen updates during a redraw cycle are buffered
and drawn together when the redraw is complete. This eliminates tearing
or flickering in cases where Nvim redraws slower than the terminal
redraws the screen.
Problem:
'nofsync' may lose data if the system has a hard shutdown. #9888
Solution:
Change default to 'fsync'. This may be revisited in the future when
'nofsync' can be made safer.
libtermkey does not interpret OSC sequences that end with a BEL (0x07)
instead of an ST (0x1b 0x5c) terminator. This causes these tests to fail
since the OSC response is now parsed via libtermkey. Change the tests to
use the ST terminator to appease libtermkey.
Problem: matchparen highlight not cleared in completion mode
Solution: Clear matchparen highlighting in completion mode
Remove hard-coded hack in insexpand.c to clear the :3match before
displaying the completion menu.
Add a test for matchparen highlighting. While at it, move all test tests
related to the matchparen plugin into a separate test file.
closes: vim/vim#13493closes: vim/vim#135249588666360
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: Various things no6 properly tested.
Solution: Add various test cases. (Yegappan Lakshmanan, closesvim/vim#10259)
885de449c0
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
We already have an extensive suite of static analysis tools we use,
which causes a fair bit of redundancy as we get duplicate warnings. PVS
is also prone to give false warnings which creates a lot of work to
identify and disable.
refactor!: `vim.lsp.inlay_hint()` -> `vim.lsp.inlay_hint.enable()`
Problem:
The LSP specification allows inlay hints to include tooltips, clickable
label parts, and code actions; but Neovim provides no API to query for
these.
Solution:
Add minimal viable extension point from which plugins can query for
inlay hints in a range, in order to build functionality on top of.
Possible Next Steps
---
- Add `virt_text_idx` field to `vim.fn.getmousepos()` return value, for
usage in mappings of `<LeftMouse>`, `<C-LeftMouse>`, etc
Problem: Not easy to filter the output of maplist().
Solution: Add mode_bits to the dictionary. (Ernie Rael, closesvim/vim#10356)
d8f5f76621
Co-authored-by: Ernie Rael <errael@raelity.com>
Problem: It is not easy to restore saved mappings.
Solution: Make mapset() accept a dict argument. (Ernie Rael, closesvim/vim#10295)
51d04d16f2
Co-authored-by: Ernie Rael <errael@raelity.com>
Problem: maparg() does not indicate the type of script where it was defined.
Solution: Add "scriptversion".
a9528b39a6
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem: No way to have extmarks automatically removed when the range it
is attached to is deleted.
Solution: Add new 'invalidate' property that will hide a mark when the
entirety of its range is deleted. When "undo_restore" is set
to false, delete the mark from the buffer instead.
It is a design goal of extmarks that they allow precise tracking
of changes across undo/redo, including restore the exact positions
after a do/undo or undo/redo cycle. However this behavior is not useful
for all usecases. Many plugins won't keep marks around for long after
text changes, but uses them more like a cache until some external source
(like LSP semantic highlights) has fully updated to changed text and
then will explicitly readjust/replace extmarks as needed.
Add a "undo_restore" flag which is true by default (matches existing
behavior) but can be set to false to opt-out of this behavior.
Delete dead u_extmark_set() code.
Previously, 'rightleftcmd' was implemented by having all code which
would affect msg_col or output screen cells be conditional on `cmdmsg_rl`.
This change removes all that and instead implements rightleft as a
mirroring post-processing step.
connection from any channel or stdio will unblock
remote_ui_wait_for_attach. Wait on stdio only if
only —embed specified, if both —embed and
—listen then wait on any channel.
While the interfaces for setting number and boolean options are now unified by #25394, there is still a separate `set_string_option` function that is used for setting a string option. This PR removes that function and merges it with set_option.
BREAKING CHANGE: `v:option_old` is now the old global value for all global-local options, instead of just string global-local options. Local value for a global-local number/boolean option is now unset when the option is set (e.g. using `:set` or `nvim_set_option_value`) without a scope, which means they now behave the same way as string options.
Ref: #25672
Problem: TextChangedI may not always trigger
Solution: trigger it in more cases: for insert/
append/change operations, and when
opening a new line,
fixes: vim/vim#13367closes: vim/vim#133754bca4897a1
Co-authored-by: Christian Brabandt <cb@256bit.org>
The prefix option of the diagnostic virtual text can be a function,
but previously it was only a function of diagnostic.
This function should also have additional parameters index and total,
more consistently and similarily as in the prefix function for
`vim.diagnostic.open_float()`.
These additional parameters will be useful when there are too many
number of diagnostics in a single line.
Fixes a regression from 5e5f5174e3
Until that commit we had a logic like this:
`local prefix = startbyte and line:sub(startbyte + 1) or line_to_cursor:sub(word_boundary)`
The commit changed the logic and no longer cut off the line at the cursor, resulting in a prefix that included trailing characters
Problem:
`win_get_bordertext_col` returns column < 1 for right or center
aligned text, if its length is more than window width.
Solution:
Return max(resulting_column, 1)
Fixes https://github.com/neovim/neovim/issues/25177
I initially wanted to split this into a refactor commit to make it more
testable, but it appears that already accidentally fixed the issue by
normalizing lnum/col to 0-indexing
`SREQ_*` values are now actual typedef'd enums. `get_option_value_strict()` has also been refactored and split into two functions, `get_option_attrs()` for getting the option attributes, and `get_option_value_strict()` for getting the actual value. Moreover, it now returns an `OptVal`. Other miscellaneous refactors have also been made.
Problem: Wrong color for half of wide character next to pum scrollbar.
Solution: Redraw the screen cell with the right color. (closesvim/vim#9874)
35d8c2010e
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem: [security] use-after-free with wildmenu
Solution: properly clean up the wildmenu when exiting
Fix wildchar/wildmenu/pum memory corruption with special wildchar's
Currently, using `wildchar=<Esc>` or `wildchar=<C-\>` can lead to a
memory corruption if using wildmenu+pum, or wrong states if only using
wildmenu. This is due to the code only using one single place inside the
cmdline process loop to perform wild menu clean up (by checking
`end_wildmenu`) but there are other odd situations where the loop could
have exited and we need a post-loop clean up just to be sure. If the
clean up was not done you would have a stale popup menu referring to
invalid memory, or if not using popup menu, incorrect status line (if
`laststatus=0`).
For example, if you hit `<Esc>` two times when it's wildchar, there's a
hard-coded behavior to exit command-line as a failsafe for user, and if
you hit `<C-\><C-\><C-N>` it will also exit command-line, but the clean
up code would not have hit because of specialized `<C-\>` handling.
Fix Ctrl-E / Ctrl-Y to not cancel/accept wildmenu if they are also
used for 'wildchar'/'wildcharm'. Currently they don't behave properly,
and also have potentially memory unsafe behavior as the logic is
currently not accounting for this situation and try to do both.
(Previous patch that addressed this: vim/vim#11677)
Also, correctly document Escape key behavior (double-hit it to escape)
in wildchar docs as it's previously undocumented.
In addition, block known invalid chars to be set in `wildchar` option,
such as Ctrl-C and `<CR>`. This is just to make it clear to the user
they shouldn't be set, and is not required for this bug fix.
closes: vim/vim#133618f4fb007e4
Co-authored-by: Yee Cheng Chin <ychin.git@gmail.com>
Problem: A few remaining cmdline completion issues with C-E/Y
Solution: Fix cmdline completion fuzzy/Ctrl-E/Ctrl-Y/options when not
used at the end
Fix cmdline completion fuzzy/Ctrl-E/Ctrl-Y/options when not used at the end
A few places in the cmdline completion code only works properly when the
user hits Tab (or 'wildchar') at the end of the cmdline, even though
it's supposed to work even in the middle of the line.
For fuzzy search, `:e ++ff`, and `:set hl=`, fix completion code to make
sure to use `xp_pattern_len` instead of assuming the entire `xp_pattern`
is the search pattern (since it contains texts after the cursor).
Fix Ctrl-E / Ctrl-Y to not jump to the end when canceling/accepting a
wildmenu completion. Also, make them work even when not using
`set wildoptions+=pum` as there is no drawback to doing so.
(Related issue where this was brought up: vim/vim#13331)
closes: vim/vim#13362209ec90b9b
Cherry-pick ex_getln.c changes from patch 9.0.2035.
Co-authored-by: Yee Cheng Chin <ychin.git@gmail.com>
BREAKING CHANGE: This breaks the OptionSet autocommand, as the `v:` values associated with it (`v:option_new`, `v:option_old`, `v:option_oldlocal` and `v:option_oldglobal`) are now the same type as the option, instead of all option values being converted to strings.
Problem: Cannot accurately get mouse clicking position when clicking on
a TAB or with virtual text.
Solution: Add a "coladd" field to getmousepos() result.
closes: vim/vim#13335f5a94d5165
Problem: `TextChangedI` can trigger on entering Insert mode if there
was previously a change not in Insert mode.
Solution: Make it trigger only when text is actually changed in Insert
mode.
closes: vim/vim#13265closes: vim/vim#13338d7ae263af8
Co-authored-by: Evgeni Chasnovski <evgeni.chasnovski@gmail.com>
Problem:
The next command after `silent !{cmd}` or `silent lua print('str')`
prints an empty line before printing a message, because these commands
set `msg_didout = true` despite not printing any messages.
Solution:
Set `msg_didout = true` only if `msg_silent == 0`
Problem: Peeking and flushing output slows down execution.
Solution: Do not update the mode message when global_busy is set. Do not
flush when only peeking for a character. (Ken Takata)
cb574f4154
PROBLEM:
Currently `:echoerr` prints multi-line strings in a single line
as `:echom` does (Note: `:echon` can print multi-line strings well).
This makes stacktrace printed via echoerr difficult to read.
Example code:
try
lua error("lua stacktrace")
catch
echoerr v:exception
endtry
Output:
Error detected while processing a.vim[5]..a.vim:
line 4:
Vim(lua):E5108: Error executing lua [string ":lua"]:1: lua stacktrace^@stack traceback:^@^I[C]: in function 'error'^@^I[string ":lua"]:1: in main chunk
SOLUTION:
Allow echoerr to print multiline messages (e.g., lua exceptions),
because this command is usually used to print stacktraces.
Output after the fix:
Error detected while processing a.vim[5]..a.vim:
line 4:
Vim(lua):E5108: Error executing lua [string ":lua"]:1: lua stacktrace
stack traceback:
[C]: in function 'error'
[string ":lua"]:1: in main chunk
Problem: Vim9: in script cannot set item in uninitialized list.
Solution: When a list is NULL allocate an empty one. (closesvim/vim#8461)
e65081d1b5
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Using fnameescape() for the path argument of findfile() and finddir() is
wrong, as fnameescape() is intended to be used for parts of Ex commands,
not function arguments.
"VimEnter foo" was accepted as a valid event name for "VimEnter".
Events delimited with commas, eg. "VimEnter,BufRead", were also
accepted, even though only the first event was actually parsed.
Co-authored-by: ii14 <ii14@users.noreply.github.com>
The 'arabicshape' feature of vim is a transformation of unicode text to
make arabic and some related scripts look better at display time. In
particular the content of a cell will be adjusted depending on the
(original) content of the cells just before and after it.
This is implemented by the arabic_shape() function in nvim. Before this
commit, shaping was invoked in four different contexts:
- when rendering buffer text in win_line()
- in line_putchar() for rendering virtual text
- as part of grid_line_puts, used by messages and statuslines and
similar
- as part of draw_cmdline() for drawing the cmdline
This replaces all these with a post-processing step in grid_put_linebuf(),
which has become the entry point for all text rendering after recent
refactors.
An aim of this is to make the handling of multibyte text yet simpler.
One of the main reasons multibyte chars needs to be "parsed" into
codepoint arrays of composing chars is so that these could be inspected
for the purpose of shaping. This can likely be vastly simplified in many
contexts where only the total length (in bytes) and width of composed
char is needed.
Problem:
On Windows, "gf" fails on a filepath that has a line:column suffix.
Example:
E447: Can't find file "src/app/core/services/identity/identity.service.ts:64:23"
Solution:
- Remove ":" from 'isfname' on Windows. Colon is not a valid filename
character (except for the drive-letter).
- Handle drive letters specially in file_name_in_line().
Fixes#25160
This finalizes the long running refactor from the old TUI-focused grid
implementation where text-drawing cursor was not separated from the
visible cursor.
Still, the pattern of setting cursor position together with updating a
line was convenient. Introduce grid_line_cursor_goto() to still allow
this but now being explicit about it.
Only having batched drawing functions makes code involving drawing
a bit longer. But it is better to be explicit, and this highlights
cases where multiple small redraws can be grouped together. This was the
case for most of the changed places (messages, lastline, and :intro)
Problem:
The swapfile "E325: ATTENTION" dialog is displayed when editing a file
already open in another (running) Nvim. Usually this behavior is
annoying and irrelevant:
- "Recover" and the other options ("Open readonly", "Quit", "Abort") are
almost never wanted.
- swapfiles are less relevant for "multi-Nvim" since 'autoread' is
enabled by default.
- Even less relevant if user enables 'autowrite'.
Solution:
Define a default SwapExists handler which does the following:
1. If the swapfile is owned by a running Nvim process, automatically
chooses "(E)dit anyway" (caveat: this creates a new, extra swapfile,
which is mostly harmless and ignored except by `:recover` or `nvim -r`.
2. Shows a 1-line "ignoring swapfile..." message.
3. Users can disable the default SwapExists handler via `autocmd! nvim_swapfile`.
msg_puts_display was more complex than necessary in nvim, as in
nvim, it no longer talks directly with a terminal.
In particular we don't need to scroll the grid before emiting the last
char. The TUI already takes care of things like that, for terminals
where it matters.
