https://bugs.winehq.org/show_bug.cgi?id=45320
Bug ID: 45320 Summary: cmd creates an environment variable with an empty name Product: Wine Version: 3.9 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: cmd Assignee: wine-bugs@winehq.org Reporter: dima@gmail.com Distribution: ---
CMD creates an environment variable with an empty name when it starts up. That doesn't match the behavior on Windows (at least, Win10) and breaks programs like Python.
Apparently, CMD is trying to save the current working directory; the code was added in https://source.winehq.org/git/wine.git/commit/d0db751e0cb7a0526c3aecca489189..., but without an explanation.
To reproduce, run this:
#include <stdio.h>
int main(int argc, char **argv, char **envp) { for (char **env = envp; *env != 0; env++) { printf("%s\n", *env); } return 0; }
The last line of the output will be similar to this:
=Z:=Z:\home\dima
Python gets an exception if it tries to unset that variable:
import os os.environ.clear()
Traceback (most recent call last): File "<stdin>", line 1, in <module> File "C:\ProgramData\Miniconda3\lib_collections_abc.py", line 820, in clear self.popitem() File "C:\ProgramData\Miniconda3\lib_collections_abc.py", line 813, in popitem del self[key] File "C:\ProgramData\Miniconda3\lib\os.py", line 680, in __delitem__ self.unsetenv(encodedkey) File "C:\ProgramData\Miniconda3\lib\os.py", line 718, in <lambda> _unsetenv = lambda key: _putenv(key, "") OSError: [Errno 0] Error
This is arguably a Python bug, and it happens on UNIX, too: https://bugs.python.org/issue20658 . However, cmd should probably match the Windows behavior and not trigger the Python bug unnecessarily.
https://bugs.winehq.org/show_bug.cgi?id=45320
Fabian Maurer dark.shadow4@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Keywords| |source, testcase Status|UNCONFIRMED |NEW CC| |dark.shadow4@web.de
--- Comment #1 from Fabian Maurer dark.shadow4@web.de --- Confirming.
Is there even a point in having an environment variable without name?
https://bugs.winehq.org/show_bug.cgi?id=45320
Jason Edmeades us@edmeades.me.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |us@edmeades.me.uk
--- Comment #2 from Jason Edmeades us@edmeades.me.uk --- Its not created without an empty name, they have a name starting with an equals sign. Please see the comments from when the patch was originally requested
https://www.winehq.org/pipermail/wine-patches/2007-March/036789.html
On windows, try "echo %=c:%" for example...
I suspect these are masked from the application depending on how they query the environment somehow
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #3 from Dima Ryazanov dima@gmail.com --- Jason: wow, you're correct.
I ran Example 3 from https://msdn.microsoft.com/en-us/library/windows/desktop/ms682009(v=vs.85).a... ; it uses GetEnvironmentStrings to print out environment variables.
On Wine, it produces exactly the same output as my testcase. On Windows, however, I see three extra variables:
=::=::\ =C:=C:\Users\dima\Documents =ExitCode=00000000
So looks like they're just hidden from the POSIX APIs?
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #4 from Jason Edmeades us@edmeades.me.uk --- My gut feeling is that CreateProcess itself should be setting the =C: type environment variable, but nothing has ever cared that it doesnt so its not worth implementing. In cmd we needed a way to remember the current directory for any other drive letter you visited, so using the same mechanism made sense, hence setting it explicitly at cmd start.
Since Python doesnt like it, I guess you need to look at how Python retrieves and enumerates its copy of the environment to see which API potentially masks these out (assuming this works on windows). Alternatively is the env block passed into a launched process excluding these somehow?
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #5 from Dima Ryazanov dima@gmail.com --- Created attachment 61642 --> https://bugs.winehq.org/attachment.cgi?id=61642 env.c
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #6 from Dima Ryazanov dima@gmail.com --- Created attachment 61643 --> https://bugs.winehq.org/attachment.cgi?id=61643 test.c
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #7 from Dima Ryazanov dima@gmail.com --- Looks like Python is using _wenviron: https://github.com/python/cpython/blob/master/Modules/posixmodule.c#L1395
I've attached two test files: env.c: prints out the contents of _wenviron test.c: runs env.exe with a custom environment: X=x, Y=y, and =%FOO=bar.
On Wine, I see this:
X=x =%FOO=bar Y=y ProgramW6432=C:\Program Files ProgramFiles=C:\Program Files CommonProgramW6432=C:\Program Files\Common Files CommonProgramFiles=C:\Program Files\Common Files
On Windows 10, I only see:
X=x Y=y
If instead of _wenviron I use GetEnvironmentStrings, then the Wine output stays the same, but on Windows 10, I see:
X=x =%FOO=bar Y=y
So yeah, the =% variables just get filtered out.
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #8 from Dima Ryazanov dima@gmail.com --- Oops, don't know where I got the "=%" from. Anything that starts with "=" gets filtered out.
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #9 from Jason Edmeades us@edmeades.me.uk --- Created attachment 62293 --> https://bugs.winehq.org/attachment.cgi?id=62293 Try various environment checks
I've been doing some playing. As far as I can see, win32 API (Get/SetEnvironmentVariable, GetEnvironmentStrings) show the =C: style variables, but SetCurrentDirectory does not modify them. However, environ and getenv does not show these (they must get filtered out by the c runtime). In addition, they do not get set when you launch an exe from explorer, only by the command shell.
I conclude that cmd.exe alone sets them, and the C runtime must filter them out when an environment block is built and passed to a program, or getenv is called (dont know if getenv looks in environ or passes through to GetEnvironmentVariable). With this being the case, I dont think the bug is in wine's cmd shell, but probably the C runtime.
https://bugs.winehq.org/show_bug.cgi?id=45320
Jason Edmeades us@edmeades.me.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #62293|0 |1 is obsolete| |
--- Comment #10 from Jason Edmeades us@edmeades.me.uk --- Created attachment 62294 --> https://bugs.winehq.org/attachment.cgi?id=62294 Program in the form of individual pass/fail tests
I reworked my program and added more experiments, plus converted it more to a pass/fail set of operations. It highlights wine gets it wrong when the exe is built with /MD (In my case, imports from MSVCR110.dll) and passes when the exe is build with /MT (I believe this builds in the calls).
Under wine: Script started on 2018-09-14 23:25:41+0100 wine cmd /c testmd OK - Prove not in getenv: Getenv (null) ERROR - Found in Environ: =Z:=Z:\jason\test\45320 OK - Prove found via GetEnvironmentVariable Z:\jason\test\45320 (19, 0) ERROR - Found in Envp: =Z:=Z:\jason\test\45320 OK - Prove found in GetEnvironmentStrings: =Z:=Z:\jason\test\45320 Prove current directory and =Z:: are independant: Before, curdir: Z:\jason\test\45320 Change env var with SetEnvironmentVariable (19, 0) Check env var change works with GetEnvironmentVariable2 Z:\ (3, 0) OK - Has dir changed (it shouldnt) - Curdir: Z:\jason\test\45320 Prove changing directory does not change =Z: Change into a subdir Has the variable changed? Should be what was set higher, ie Z:\ Current directory: Z:\jason\test\45320\subdir OK - GetEnvironmentVariable3 Z:\ (3, 0) Prove putenv fails to set one: OK - putenv failed OK - Prove not in environment now Getenv empty OK - Not in win32 environ either (0, 203) Press any key to end
wine cmd /c testmt OK - Prove not in getenv: Getenv (null) OK - Prove not found in Environ OK - Prove found via GetEnvironmentVariable Z:\jason\test\45320 (19, 0) OK - Prove not found in envp OK - Prove found in GetEnvironmentStrings: =Z:=Z:\jason\test\45320 Prove current directory and =Z:: are independant: Before, curdir: Z:\jason\test\45320 Change env var with SetEnvironmentVariable (19, 0) Check env var change works with GetEnvironmentVariable2 Z:\ (3, 0) OK - Has dir changed (it shouldnt) - Curdir: Z:\jason\test\45320 Prove changing directory does not change =Z: Change into a subdir Has the variable changed? Should be what was set higher, ie Z:\ Current directory: Z:\jason\test\45320\subdir OK - GetEnvironmentVariable3 Z:\ (3, 0) Prove putenv fails to set one: OK - putenv failed OK - Prove not in environment now Getenv empty OK - Not in win32 environ either (0, 203) Press any key to end
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #11 from Jason Edmeades us@edmeades.me.uk --- Created attachment 62295 --> https://bugs.winehq.org/attachment.cgi?id=62295 Patch
This resolves the problems shown in my test case - are you able to confirm it sorts it out for you before I submit please
https://bugs.winehq.org/show_bug.cgi?id=45320
Jason Edmeades us@edmeades.me.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch
https://bugs.winehq.org/show_bug.cgi?id=45320
Jason Edmeades us@edmeades.me.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|wine-bugs@winehq.org |us@edmeades.me.uk
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #12 from Dima Ryazanov dima@gmail.com --- Awesome, thanks, I'll test it now.
I've looked at the code - and I think you need to do the check when calculating "count" and "len", too:
for (ptr = environ_strings; *ptr; ptr += strlen(ptr) + 1) { count++; len += strlen(ptr) + 1; }
Wasting a few bytes is probably not a big deal - but the doc string is pretty explicit about the layout of the result:
* blk is an array of pointers to environment strings, ending with a NULL * and after that the actual copy of the environment strings, ending in a \0
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #13 from Jason Edmeades us@edmeades.me.uk --- I started with trying that, then I deliberately didn't sort the count and length when allocating the blocks, because I was concerned about the memcpy where it takes the GetEnvironmentStrings (which includes the =C:) and copies it into the allocated memory (len would be too short, and I didnt want the performance hit of copying smaller chunks).
I think looking again, you would be correct that I can fix count, so effectively you have an accurate size for the char * pointers, just leaving the actual data copied in would still contain the =C: with no pointers to it. As you say, it'll only save a few bytes, but its probably cleaner.
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #14 from Jason Edmeades us@edmeades.me.uk --- Where did you see the docs on the layout?
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #15 from Dima Ryazanov dima@gmail.com --- The doc is above msvcrt_SnapshotOfEnvironmentA, line 65.
I didn't even think about the memcpy... I guess technically, copying all of environ_strings would be incorrect, too, but it does seem like a very minor problem.
I'll try this on Windows, and see if environ/wenviron actually follow that format exactly.
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #16 from Dima Ryazanov dima@gmail.com --- Well, FWIW, in Windows 10, the contents of _environ does follow that format.
Here's my test code:
#include <stdio.h> #include <stdlib.h> #include <string.h>
int main() { char **e;
getenv(""); for (e = _environ; *e != NULL; e++) { printf("%p - %p\n", *e, *e + strlen(*e)); } printf("%p\n", e); return 0; }
I ran it with minimal environment, using the test.c I attached earlier.
Windows 10:
0000000000BE14A0 - 0000000000BE14A3 0000000000BE14A4 - 0000000000BE14AD 0000000000BE14AE - 0000000000BE14B1 0000000000BE1498
Wine (with the patch):
0000000000011908 - 000000000001190B 0000000000011916 - 000000000001191F 0000000000011920 - 0000000000011923 0000000000011924 - 0000000000011941 0000000000011942 - 000000000001195F 0000000000011960 - 0000000000011990 0000000000011991 - 00000000000119C1 00000000000118F8
You can see that in Windows 10, the data starts after 8 bytes (i.e., right after the NULL pointer), and there are no gaps between the env strings.
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #17 from Dima Ryazanov dima@gmail.com --- That said, the patch does fix "os.environ.clear()" in Python (though I had to work around bug 42474 to test it).
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #18 from Jason Edmeades us@edmeades.me.uk --- Glad it sorts out the problem, and thanks for the windows testing.
My gut feeling is go with what we have (so char** array doesn't point to them but they are actually in the space allocated for the environ but not referenceable unless you walk the environ explicitly) until we find an app that will care. Anyone walking the char** array wont see them, and whether there are gaps or not seems an implementation detail not exposed to the user through normal API means.
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #19 from Dima Ryazanov dima@gmail.com --- Yeah, makes sense.
https://bugs.winehq.org/show_bug.cgi?id=45320
--- Comment #20 from Jason Edmeades us@edmeades.me.uk --- https://www.winehq.org/pipermail/wine-devel/2018-September/132654.html
https://bugs.winehq.org/show_bug.cgi?id=45320
Jason Edmeades us@edmeades.me.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |0dcfc97fcbea15037d15dd10218 | |70de25b8382df Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #21 from Jason Edmeades us@edmeades.me.uk --- Committed
https://bugs.winehq.org/show_bug.cgi?id=45320
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #22 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 3.17.
https://bugs.winehq.org/show_bug.cgi?id=45320
Michael Stefaniuc mstefani@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |3.0.x
https://bugs.winehq.org/show_bug.cgi?id=45320
Michael Stefaniuc mstefani@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|3.0.x |---
--- Comment #23 from Michael Stefaniuc mstefani@winehq.org --- Removing the 3.0.x milestone from bug fixes included in 3.0.5.