Windows: uv_os_getenv() with empty (but defined) variable

[justinmk]

• Version: 1.30.0
• Platform: Windows 7, building with MSVC 2017 (appveyor)

If an env var is defined as empty:

SetEnvironmentVariable( "foo", "" );

The libuv Windows impl of uv_os_getenv returns -4094 (UV_UNKNOWN).

According to this post,

When GetEnvironmentVariable return 0, it may be that the variable doesn't exist, or that is exists with an empty content. In the later case, GetLastError returns 0.

But libuv checks GetEnvironmentVariableW() == 0, then returns uv_translate_sys_error(0).

libuv/src/win/util.c

Lines 1484 to 1505 in 8ed9112

	len = GetEnvironmentVariableW(name_w, var, MAX_ENV_VAR_LENGTH);
	uv__free(name_w);
	assert(len < MAX_ENV_VAR_LENGTH); /* len does not include the null */
	if (len == 0) {
	r = GetLastError();
	if (r == ERROR_ENVVAR_NOT_FOUND)
	return UV_ENOENT;
	return uv_translate_sys_error(r);
	}
	/* Check how much space we need */
	bufsize = WideCharToMultiByte(CP_UTF8, 0, var, -1, NULL, 0, NULL, NULL);
	if (bufsize == 0) {
	return uv_translate_sys_error(GetLastError());
	} else if (bufsize > *size) {
	*size = bufsize;
	return UV_ENOBUFS;
	}

It should instead return 0 with buffer size of 0.

[cjihrig]

So, I wrote cjihrig@3377726, which I expected to be a straightforward fix.

But, there seems to be a second issue. While trying to write a test for this issue, I set an environment variable to the empty string. Windows seems to delete the environment variable instead. There's a good chance I'm doing something wrong, and I don't have a Windows setup to test on, but that's where I'm at currently.

EDIT: It looks like Ruby avoids GetEnvironmentVariable() for this reason: ruby/ruby@2982c52

[saghul]

@cjihrig What Ruby does looks sensible IMHO. I do have a Windows laptop up and running if you want me to test stuff.

[erw7]

@cjihrig cjihrig@3377726 and the following patch seems to have fixed the neovim issue. I think GetEnvironmentVariable() doesn't set the last error code to 0 on success.

Ref. https://docs.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror

diff --git a/src/win/util.c b/src/win/util.c
index 54cf4f42..2fef4c5c 100644
--- a/src/win/util.c
+++ b/src/win/util.c
@@ -1412,13 +1412,13 @@ int uv_os_getenv(const char* name, char* buffer, size_t* size) {
   if (r != 0)
     return r;

+  SetLastError(ERROR_SUCCESS);
   len = GetEnvironmentVariableW(name_w, var, MAX_ENV_VAR_LENGTH);
+  r = GetLastError();
   uv__free(name_w);
   assert(len < MAX_ENV_VAR_LENGTH); /* len does not include the null */

   if (len == 0) {
-    r = GetLastError();
-
     if (r == ERROR_ENVVAR_NOT_FOUND)
       return UV_ENOENT;

I set an environment variable to the empty string. Windows seems to delete the environment variable instead.

I don't know how you set the environment variable to empty string, but the following code appears to create an empty environment variable.

SetEnvironmentVariableW(L"FOO", L"");
uv_os_setenv("BAR", ""); 

[cjihrig]

Thanks @erw7 - calling SetLastError() did the trick. Fix for this issue in #2419.