diff options
author | Justin M. Keyes <justinkz@gmail.com> | 2024-09-08 12:48:32 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-09-08 12:48:32 -0700 |
commit | 8a2aec99748229ad9d1e12c1cbc0768d063e8eed (patch) | |
tree | deed001296b252ea886130fea4844a8a7a5d6a7f | |
parent | 3a881132460430d23f2fdc87822c87d47f721cfc (diff) | |
download | rneovim-8a2aec99748229ad9d1e12c1cbc0768d063e8eed.tar.gz rneovim-8a2aec99748229ad9d1e12c1cbc0768d063e8eed.tar.bz2 rneovim-8a2aec99748229ad9d1e12c1cbc0768d063e8eed.zip |
fix(startup): server fails if $NVIM_APPNAME is relative dir #30310
Problem:
If $NVIM_APPNAME is a relative dir path, Nvim fails to start its
primary/default server, and `v:servername` is empty.
Root cause is d34c64e342dfba9248d1055e702d02620a1b31a8, but this wasn't
noticed until 96128a5076b7 started reporting the error more loudly.
Solution:
- `server_address_new`: replace slashes "/" in the appname before using
it as a servername.
- `vim_mktempdir`: always prefer the system-wide top-level "nvim.user/"
directory. That isn't intended to be specific to NVIM_APPNAME; rather,
each *subdirectory* ("nvim.user/xxx") is owned by each Nvim instance.
Nvim "apps" can be identified by the server socket(s) stored in those
per-Nvim subdirs.
fix #30256
-rw-r--r-- | src/nvim/eval/funcs.c | 2 | ||||
-rw-r--r-- | src/nvim/fileio.c | 13 | ||||
-rw-r--r-- | src/nvim/msgpack_rpc/server.c | 7 | ||||
-rw-r--r-- | src/nvim/os/stdpaths.c | 18 | ||||
-rw-r--r-- | src/nvim/runtime.c | 6 | ||||
-rw-r--r-- | test/functional/core/fileio_spec.lua | 15 | ||||
-rw-r--r-- | test/functional/core/server_spec.lua (renamed from test/functional/vimscript/server_spec.lua) | 4 | ||||
-rw-r--r-- | test/functional/options/defaults_spec.lua | 21 | ||||
-rw-r--r-- | test/functional/testnvim.lua | 3 | ||||
-rw-r--r-- | test/testutil.lua | 2 |
10 files changed, 52 insertions, 39 deletions
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index de1d784577..aed6fdae14 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -7641,7 +7641,7 @@ static void get_xdg_var_list(const XDGVarType xdg, typval_T *rettv) return; } const void *iter = NULL; - const char *appname = get_appname(); + const char *appname = get_appname(false); do { size_t dir_len; const char *dir; diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c index f6a25a27a6..31a1c0e293 100644 --- a/src/nvim/fileio.c +++ b/src/nvim/fileio.c @@ -3264,18 +3264,12 @@ static void vim_mktempdir(void) char tmp[TEMP_FILE_PATH_MAXLEN]; char path[TEMP_FILE_PATH_MAXLEN]; char user[40] = { 0 }; - char appname[40] = { 0 }; os_get_username(user, sizeof(user)); // Usernames may contain slashes! #19240 memchrsub(user, '/', '_', sizeof(user)); memchrsub(user, '\\', '_', sizeof(user)); - // Appname may be a relative path, replace slashes to make it name-like. - xstrlcpy(appname, get_appname(), sizeof(appname)); - memchrsub(appname, '/', '%', sizeof(appname)); - memchrsub(appname, '\\', '%', sizeof(appname)); - // Make sure the umask doesn't remove the executable bit. // "repl" has been reported to use "0177". mode_t umask_save = umask(0077); @@ -3283,14 +3277,15 @@ static void vim_mktempdir(void) // Expand environment variables, leave room for "/tmp/nvim.<user>/XXXXXX/999999999". expand_env((char *)temp_dirs[i], tmp, TEMP_FILE_PATH_MAXLEN - 64); if (!os_isdir(tmp)) { + if (strequal("$TMPDIR", temp_dirs[i])) { + WLOG("$TMPDIR tempdir not a directory (or does not exist): %s", tmp); + } continue; } // "/tmp/" exists, now try to create "/tmp/nvim.<user>/". add_pathsep(tmp); - - xstrlcat(tmp, appname, sizeof(tmp)); - xstrlcat(tmp, ".", sizeof(tmp)); + xstrlcat(tmp, "nvim.", sizeof(tmp)); xstrlcat(tmp, user, sizeof(tmp)); os_mkdir(tmp, 0700); // Always create, to avoid a race. bool owned = os_file_owned(tmp); diff --git a/src/nvim/msgpack_rpc/server.c b/src/nvim/msgpack_rpc/server.c index 9cfe46454d..dc2b85154f 100644 --- a/src/nvim/msgpack_rpc/server.c +++ b/src/nvim/msgpack_rpc/server.c @@ -121,14 +121,15 @@ char *server_address_new(const char *name) { static uint32_t count = 0; char fmt[ADDRESS_MAX_SIZE]; - const char *appname = get_appname(); #ifdef MSWIN + (void)get_appname(true); int r = snprintf(fmt, sizeof(fmt), "\\\\.\\pipe\\%s.%" PRIu64 ".%" PRIu32, - name ? name : appname, os_get_pid(), count++); + name ? name : NameBuff, os_get_pid(), count++); #else char *dir = stdpaths_get_xdg_var(kXDGRuntimeDir); + (void)get_appname(true); int r = snprintf(fmt, sizeof(fmt), "%s/%s.%" PRIu64 ".%" PRIu32, - dir, name ? name : appname, os_get_pid(), count++); + dir, name ? name : NameBuff, os_get_pid(), count++); xfree(dir); #endif if ((size_t)r >= sizeof(fmt)) { diff --git a/src/nvim/os/stdpaths.c b/src/nvim/os/stdpaths.c index e4435bcaa8..e9a74d197f 100644 --- a/src/nvim/os/stdpaths.c +++ b/src/nvim/os/stdpaths.c @@ -63,22 +63,32 @@ static const char *const xdg_defaults[] = { #endif }; -/// Get the value of $NVIM_APPNAME or "nvim" if not set. +/// Gets the value of $NVIM_APPNAME, or "nvim" if not set. +/// +/// @param namelike Write "name-like" value (no path separators) in `NameBuff`. /// /// @return $NVIM_APPNAME value -const char *get_appname(void) +const char *get_appname(bool namelike) { const char *env_val = os_getenv("NVIM_APPNAME"); if (env_val == NULL || *env_val == NUL) { env_val = "nvim"; } + + if (namelike) { + // Appname may be a relative path, replace slashes to make it name-like. + xstrlcpy(NameBuff, env_val, sizeof(NameBuff)); + memchrsub(NameBuff, '/', '-', sizeof(NameBuff)); + memchrsub(NameBuff, '\\', '-', sizeof(NameBuff)); + } + return env_val; } /// Ensure that APPNAME is valid. Must be a name or relative path. bool appname_is_valid(void) { - const char *appname = get_appname(); + const char *appname = get_appname(false); if (path_is_absolute(appname) // TODO(justinmk): on Windows, path_is_absolute says "/" is NOT absolute. Should it? || strequal(appname, "/") @@ -193,7 +203,7 @@ char *get_xdg_home(const XDGVarType idx) FUNC_ATTR_WARN_UNUSED_RESULT { char *dir = stdpaths_get_xdg_var(idx); - const char *appname = get_appname(); + const char *appname = get_appname(false); size_t appname_len = strlen(appname); assert(appname_len < (IOSIZE - sizeof("-data"))); diff --git a/src/nvim/runtime.c b/src/nvim/runtime.c index 3aa558f305..030bda4fa5 100644 --- a/src/nvim/runtime.c +++ b/src/nvim/runtime.c @@ -1559,7 +1559,7 @@ static inline char *add_env_sep_dirs(char *dest, const char *const val, const ch return dest; } const void *iter = NULL; - const char *appname = get_appname(); + const char *appname = get_appname(false); const size_t appname_len = strlen(appname); do { size_t dir_len; @@ -1626,7 +1626,7 @@ static inline char *add_dir(char *dest, const char *const dir, const size_t dir_ if (!after_pathsep(dest - 1, dest)) { *dest++ = PATHSEP; } - const char *appname = get_appname(); + const char *appname = get_appname(false); size_t appname_len = strlen(appname); assert(appname_len < (IOSIZE - sizeof("-data"))); xmemcpyz(IObuff, appname, appname_len); @@ -1697,7 +1697,7 @@ char *runtimepath_default(bool clean_arg) size_t config_len = 0; size_t vimruntime_len = 0; size_t libdir_len = 0; - const char *appname = get_appname(); + const char *appname = get_appname(false); size_t appname_len = strlen(appname); if (data_home != NULL) { data_len = strlen(data_home); diff --git a/test/functional/core/fileio_spec.lua b/test/functional/core/fileio_spec.lua index 073041fced..d33710a63d 100644 --- a/test/functional/core/fileio_spec.lua +++ b/test/functional/core/fileio_spec.lua @@ -321,11 +321,11 @@ end) describe('tmpdir', function() local tmproot_pat = [=[.*[/\\]nvim%.[^/\\]+]=] local testlog = 'Xtest_tmpdir_log' - local os_tmpdir + local os_tmpdir ---@type string before_each(function() -- Fake /tmp dir so that we can mess it up. - os_tmpdir = vim.uv.fs_mkdtemp(vim.fs.dirname(t.tmpname(false)) .. '/nvim_XXXXXXXXXX') + os_tmpdir = assert(vim.uv.fs_mkdtemp(vim.fs.dirname(t.tmpname(false)) .. '/nvim_XXXXXXXXXX')) end) after_each(function() @@ -414,15 +414,4 @@ describe('tmpdir', function() rm_tmpdir() eq('E5431: tempdir disappeared (3 times)', api.nvim_get_vvar('errmsg')) end) - - it('$NVIM_APPNAME relative path', function() - clear({ - env = { - NVIM_APPNAME = 'a/b', - NVIM_LOG_FILE = testlog, - TMPDIR = os_tmpdir, - }, - }) - matches([=[.*[/\\]a%%b%.[^/\\]+]=], fn.tempname()) - end) end) diff --git a/test/functional/vimscript/server_spec.lua b/test/functional/core/server_spec.lua index 85a179b3d5..0ec11169e9 100644 --- a/test/functional/vimscript/server_spec.lua +++ b/test/functional/core/server_spec.lua @@ -154,7 +154,7 @@ describe('server', function() clear_serverlist() -- Address without slashes is a "name" which is appended to a generated path. #8519 - matches([[.*[/\\]xtest1%.2%.3%.4[^/\\]*]], fn.serverstart('xtest1.2.3.4')) + matches([[[/\\]xtest1%.2%.3%.4[^/\\]*]], fn.serverstart('xtest1.2.3.4')) clear_serverlist() eq('Vim:Failed to start server: invalid argument', pcall_err(fn.serverstart, '127.0.0.1:65536')) -- invalid port @@ -273,6 +273,6 @@ describe('startup --listen', function() -- Address without slashes is a "name" which is appended to a generated path. #8519 clear({ args = { '--listen', 'test-name' } }) - matches([[.*[/\\]test%-name[^/\\]*]], api.nvim_get_vvar('servername')) + matches([[[/\\]test%-name[^/\\]*]], api.nvim_get_vvar('servername')) end) end) diff --git a/test/functional/options/defaults_spec.lua b/test/functional/options/defaults_spec.lua index ca4a6eaca7..ad94ef1206 100644 --- a/test/functional/options/defaults_spec.lua +++ b/test/functional/options/defaults_spec.lua @@ -915,7 +915,7 @@ describe('stdpath()', function() assert_alive() -- Check for crash. #8393 end) - it('supports $NVIM_APPNAME', function() + it('$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')) @@ -957,6 +957,25 @@ describe('stdpath()', function() test_appname('a/b\\c', 0) end) + it('$NVIM_APPNAME relative path', function() + local tmpdir = t.tmpname(false) + t.mkdir(tmpdir) + + clear({ + args_rm = { '--listen' }, + env = { + NVIM_APPNAME = 'relative/appname', + NVIM_LOG_FILE = testlog, + TMPDIR = tmpdir, + }, + }) + + t.matches(vim.pesc(tmpdir), fn.tempname():gsub('\\', '/')) + t.assert_nolog('tempdir', testlog, 100) + t.assert_nolog('TMPDIR', testlog, 100) + t.matches([=[[/\\]relative%-appname.[^/\\]+]=], api.nvim_get_vvar('servername')) + end) + describe('returns a String', function() describe('with "config"', function() it('knows XDG_CONFIG_HOME', function() diff --git a/test/functional/testnvim.lua b/test/functional/testnvim.lua index 66ce6daacb..d74e8055ef 100644 --- a/test/functional/testnvim.lua +++ b/test/functional/testnvim.lua @@ -14,8 +14,7 @@ local is_os = t.is_os local ok = t.ok local sleep = uv.sleep ---- This module uses functions from the context of the test session, i.e. in the context of the ---- nvim being tests. +--- Functions executing in the current nvim session/process being tested. local M = {} local runtime_set = 'set runtimepath^=./build/lib/nvim/' diff --git a/test/testutil.lua b/test/testutil.lua index abfc10e806..19e297c503 100644 --- a/test/testutil.lua +++ b/test/testutil.lua @@ -16,7 +16,7 @@ local function shell_quote(str) return str end ---- This module uses functions from the context of the test runner. +--- Functions executing in the context of the test runner (not the current nvim test session). --- @class test.testutil local M = { paths = Paths, |