wyj/neovim
1
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2024-09-17 20:58:20 -04:00
Code Issues Packages Projects Releases Wiki Activity

feat(startup): validate --listen address

Browse Source 
Problem:
`nvim --listen` does not error on EADDRINUSE. #30123

Solution:
Now that `$NVIM_LISTEN_ADDRESS` is deprecated and input *only* (instead
of the old, ambiguous situation where it was both an input *and* an
output), we can be fail fast instead of trying to "recover". This
reverts the "recovery" behavior of
704ba4151e, but that was basically
a workaround for the fragility of `$NVIM_LISTEN_ADDRESS`.
This commit is contained in:
Justin M. Keyes 2024-09-02 15:57:07 +02:00
parent 137f98cf64
commit 96128a5076
6 changed files with 101 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
runtime/doc/news.txt
View File

@ -53,12 +53,6 @@ EDITOR
documented and skips help buffers if run from a non-help buffer, otherwise
it moves to another help buffer.
VIM SCRIPT
• |v:msgpack_types| has the type "binary" removed. |msgpackparse()| no longer
treats BIN, STR and FIXSTR as separate types. Any of these is returned as a
string if possible, or a |blob| if the value contained embedded NUL:s.
EVENTS
• TODO
@ -98,6 +92,12 @@ TUI
• TODO
VIMSCRIPT
• |v:msgpack_types| has the type "binary" removed. |msgpackparse()| no longer
treats BIN, STR and FIXSTR as separate types. Any of these is returned as a
string if possible, or a |blob| if the value contained embedded NUL:s.
==============================================================================
NEW FEATURES *news-features*
@ -162,7 +162,8 @@ PLUGINS
STARTUP
• TODO
• Nvim will fail if the |--listen| or |$NVIM_LISTEN_ADDRESS| address is
invalid, instead of silently skipping an invalid address.
TERMINAL

8
src/nvim/api/ui.c
View File

@ -93,15 +93,15 @@ void remote_ui_free_all_mem(void)
}
#endif
/// Wait until ui has connected on stdio channel if only_stdio
/// is true, otherwise any channel.
/// Wait until UI has connected.
///
/// @param only_stdio UI is expected to connect on stdio.
void remote_ui_wait_for_attach(bool only_stdio)
{
if (only_stdio) {
Channel *channel = find_channel(CHAN_STDIO);
if (!channel) {
// this function should only be called in --embed mode, stdio channel
// can be assumed.
// `only_stdio` implies --embed mode, thus stdio channel can be assumed.
abort();
}

14
src/nvim/main.c
View File

@ -332,12 +332,6 @@ int main(int argc, char **argv)
#endif
bool use_builtin_ui = (has_term && !headless_mode && !embedded_mode && !silent_mode);
// don't bind the server yet, if we are using builtin ui.
// This will be done when nvim server has been forked from the ui process
if (!use_builtin_ui) {
server_init(params.listen_addr);
}
if (params.remote) {
remote_request(&params, params.remote, params.server_addr, argc, argv,
use_builtin_ui);
@ -355,11 +349,19 @@ int main(int argc, char **argv)
ui_client_channel_id = rv;
}
// NORETURN: Start builtin UI client.
if (ui_client_channel_id) {
time_finish();
ui_client_run(remote_ui); // NORETURN
}
assert(!ui_client_channel_id && !use_builtin_ui);
// Nvim server...
int listen_rv = server_init(params.listen_addr);
if (listen_rv != 0) {
mainerr("Failed to --listen", listen_rv < 0
? os_strerror(listen_rv) : (listen_rv == 1 ? "empty address" : NULL));
}
TIME_MSG("expanding arguments");

31
src/nvim/msgpack_rpc/server.c
View File

@ -28,27 +28,32 @@ static garray_T watchers = GA_EMPTY_INIT_VALUE;
#endif
/// Initializes the module
bool server_init(const char *listen_addr)
///
/// @returns 0: success, 1: validation error, 2: already listening, -errno: failed to bind/listen.
int server_init(const char *listen_addr)
{
bool must_free = false;
ga_init(&watchers, sizeof(SocketWatcher *), 1);
// $NVIM_LISTEN_ADDRESS (deprecated)
if (!listen_addr && os_env_exists(ENV_LISTEN)) {
if ((!listen_addr || listen_addr[0] == '\0') && os_env_exists(ENV_LISTEN)) {
listen_addr = os_getenv(ENV_LISTEN);
}
int rv = listen_addr ? server_start(listen_addr) : 1;
if (0 != rv) {
if (!listen_addr || listen_addr[0] == '\0') {
listen_addr = server_address_new(NULL);
if (!listen_addr) {
return false;
}
rv = server_start(listen_addr);
xfree((char *)listen_addr);
must_free = true;
}
if (!listen_addr) {
abort(); // Cannot happen.
}
int rv = server_start(listen_addr);
if (os_env_exists(ENV_LISTEN)) {
// Unset $NVIM_LISTEN_ADDRESS, it's a liability hereafter.
// Unset $NVIM_LISTEN_ADDRESS, it's a liability hereafter. It is "input only", it must not be
// leaked to child jobs or :terminal.
os_unsetenv(ENV_LISTEN);
}
@ -57,7 +62,11 @@ bool server_init(const char *listen_addr)
ELOG("test log message");
}
return rv == 0;
if (must_free) {
xfree((char *)listen_addr);
}
return rv;
}
/// Teardown a single server

25
test/functional/options/defaults_spec.lua
View File

@ -247,6 +247,7 @@ describe('startup defaults', function()
} })
eq('Xtest-logpath', eval('$NVIM_LOG_FILE'))
end)
it('defaults to stdpath("log")/log if empty', function()
eq(true, mkdir(xdgdir) and mkdir(xdgstatedir))
clear({
@ -257,6 +258,7 @@ describe('startup defaults', function()
})
eq(xdgstatedir .. '/log', string.gsub(eval('$NVIM_LOG_FILE'), '\\', '/'))
end)
it('defaults to stdpath("log")/log if invalid', function()
eq(true, mkdir(xdgdir) and mkdir(xdgstatedir))
clear({
@ -266,6 +268,8 @@ describe('startup defaults', function()
},
})
eq(xdgstatedir .. '/log', string.gsub(eval('$NVIM_LOG_FILE'), '\\', '/'))
-- Avoid "failed to open $NVIM_LOG_FILE" noise in test output.
expect_exit(command, 'qall!')
end)
end)
end)
@ -339,9 +343,11 @@ describe('XDG defaults', function()
local state_dir = is_os('win') and 'nvim-data' or 'nvim'
local root_path = is_os('win') and 'C:' or ''
describe('with too long XDG variables', function()
describe('with too long XDG vars', function()
before_each(function()
clear({
-- Ensure valid --listen address despite broken XDG vars (else Nvim won't start).
args = { '--listen', is_os('win') and '' or t.tmpname(false) },
args_rm = { 'runtimepath' },
env = {
NVIM_LOG_FILE = testlog,
@ -361,6 +367,9 @@ describe('XDG defaults', function()
it('are correctly set', function()
if not is_os('win') then
-- Broken XDG vars cause serverstart() to fail (except on Windows, where servernames are not
-- informed by $XDG_STATE_HOME).
t.matches('Failed to start server: no such file or directory', t.pcall_err(fn.serverstart))
assert_log('Failed to start server: no such file or directory: /X/X/X', testlog, 10)
end
@ -522,9 +531,11 @@ describe('XDG defaults', function()
end)
end)
describe('with XDG variables that can be expanded', function()
describe('with expandable XDG vars', function()
before_each(function()
clear({
-- Ensure valid --listen address despite broken XDG vars (else Nvim won't start).
args = { '--listen', is_os('win') and '' or t.tmpname(false) },
args_rm = { 'runtimepath' },
env = {
NVIM_LOG_FILE = testlog,
@ -544,6 +555,9 @@ describe('XDG defaults', function()
it('are not expanded', function()
if not is_os('win') then
-- Broken XDG vars cause serverstart() to fail (except on Windows, where servernames are not
-- informed by $XDG_STATE_HOME).
t.matches('Failed to start server: no such file or directory', t.pcall_err(fn.serverstart))
assert_log(
'Failed to start server: no such file or directory: %$XDG_RUNTIME_DIR%/',
testlog,
@ -895,7 +909,7 @@ describe('stdpath()', function()
assert_alive() -- Check for crash. #8393
end)
it('reacts to $NVIM_APPNAME', function()
it('supports $NVIM_APPNAME', function()
local appname = 'NVIM_APPNAME_TEST' .. ('_'):rep(106)
clear({ env = { NVIM_APPNAME = appname, NVIM_LOG_FILE = testlog } })
eq(appname, fn.fnamemodify(fn.stdpath('config'), ':t'))
@ -916,7 +930,7 @@ describe('stdpath()', function()
local function test_appname(testAppname, expected_exitcode)
local lua_code = string.format(
[[
local child = vim.fn.jobstart({ vim.v.progpath, '--clean', '--headless', '+qall!' }, { env = { NVIM_APPNAME = %q } })
local child = vim.fn.jobstart({ vim.v.progpath, '--clean', '--headless', '--listen', 'x', '+qall!' }, { env = { NVIM_APPNAME = %q } })
return vim.fn.jobwait({ child }, %d)[1]
]],
alter_slashes(testAppname),
@ -935,9 +949,6 @@ describe('stdpath()', function()
-- Valid appnames:
test_appname('a/b', 0)
test_appname('a/b\\c', 0)
if not is_os('win') then
assert_log('Failed to start server: no such file or directory:', testlog)
end
end)
describe('returns a String', function()

67
test/functional/vimscript/server_spec.lua
View File

@ -4,7 +4,6 @@ local n = require('test.functional.testnvim')()
local assert_log = t.assert_log
local eq, neq, eval = t.eq, t.neq, n.eval
local clear, fn, api = n.clear, n.fn, n.api
local ok = t.ok
local matches = t.matches
local pcall_err = t.pcall_err
local check_close = n.check_close
@ -49,15 +48,6 @@ describe('server', function()
eq('', eval('$NVIM_LISTEN_ADDRESS'))
end)
it('sets new v:servername if $NVIM_LISTEN_ADDRESS is invalid', function()
clear({ env = { NVIM_LISTEN_ADDRESS = '.' } })
-- Cleared on startup.
eq('', eval('$NVIM_LISTEN_ADDRESS'))
local servers = fn.serverlist()
eq(1, #servers)
ok(string.len(servers[1]) > 4) -- "~/.local/state/nvim…/…" or "\\.\pipe\…"
end)
it('sets v:servername at startup or if all servers were stopped', function()
clear()
local initial_server = api.nvim_get_vvar('servername')
@ -89,20 +79,26 @@ describe('server', function()
end)
it('serverstop() returns false for invalid input', function()
clear { env = {
NVIM_LOG_FILE = testlog,
NVIM_LISTEN_ADDRESS = '.',
} }
clear {
args_rm = { '--listen' },
env = {
NVIM_LOG_FILE = testlog,
NVIM_LISTEN_ADDRESS = '',
},
}
eq(0, eval("serverstop('')"))
eq(0, eval("serverstop('bogus-socket-name')"))
assert_log('Not listening on bogus%-socket%-name', testlog, 10)
end)
it('parses endpoints', function()
clear { env = {
NVIM_LOG_FILE = testlog,
NVIM_LISTEN_ADDRESS = '.',
} }
clear {
args_rm = { '--listen' },
env = {
NVIM_LOG_FILE = testlog,
NVIM_LISTEN_ADDRESS = '',
},
}
clear_serverlist()
eq({}, fn.serverlist())
@ -178,18 +174,41 @@ end)
describe('startup --listen', function()
it('validates', function()
clear()
local cmd = { unpack(n.nvim_argv) }
table.insert(cmd, '--listen')
matches('nvim.*: Argument missing after: "%-%-listen"', fn.system(cmd))
cmd = { unpack(n.nvim_argv) }
table.insert(cmd, '--listen2')
matches('nvim.*: Garbage after option argument: "%-%-listen2"', fn.system(cmd))
-- Tests args with and without "--headless".
local function _test(args, expected)
-- XXX: clear v:shell_error, sigh...
fn.system({ n.nvim_prog, '-es', '+qall!' })
assert(0 == eval('v:shell_error'))
local cmd = vim.list_extend({ unpack(n.nvim_argv) }, vim.list_extend({ '--headless' }, args))
local output = fn.system(cmd)
assert(0 ~= eval('v:shell_error'))
-- TODO(justinmk): output not properly captured on Windows?
if is_os('win') then
return
end
matches(expected, output)
cmd = vim.list_extend({ unpack(n.nvim_argv) }, args)
matches(expected, fn.system(cmd))
end
_test({ '--listen' }, 'nvim.*: Argument missing after: "%-%-listen"')
_test({ '--listen2' }, 'nvim.*: Garbage after option argument: "%-%-listen2"')
_test({ '--listen', n.eval('v:servername') }, 'nvim.*: Failed to %-%-listen: ".* already .*"')
_test({ '--listen', '/' }, 'nvim.*: Failed to %-%-listen: ".*"')
_test(
{ '--listen', 'https://example.com' },
('nvim.*: Failed to %%-%%-listen: "%s"'):format(
(is_os('mac') or is_os('win')) and 'unknown node or service'
or 'service not available for socket type'
)
)
end)
it('sets v:servername, overrides $NVIM_LISTEN_ADDRESS', function()
local addr = (is_os('win') and [[\\.\pipe\Xtest-listen-pipe]] or './Xtest-listen-pipe')
clear({ env = { NVIM_LISTEN_ADDRESS = './Xtest-env-pipe' }, args = { '--listen', addr } })
eq('', eval('$NVIM_LISTEN_ADDRESS')) -- Cleared on startup.
eq(addr, api.nvim_get_vvar('servername'))
-- Address without slashes is a "name" which is appended to a generated path. #8519