[Bug 45320] New: cmd creates an environment variable with an empty name
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(a)winehq.org Reporter: dima(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Keywords| |source, testcase Status|UNCONFIRMED |NEW CC| |dark.shadow4(a)web.de --- Comment #1 from Fabian Maurer <dark.shadow4(a)web.de> --- Confirming. Is there even a point in having an environment variable without name? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Jason Edmeades <us(a)edmeades.me.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |us(a)edmeades.me.uk --- Comment #2 from Jason Edmeades <us(a)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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #3 from Dima Ryazanov <dima(a)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? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #4 from Jason Edmeades <us(a)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? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #5 from Dima Ryazanov <dima(a)gmail.com> --- Created attachment 61642 --> https://bugs.winehq.org/attachment.cgi?id=61642 env.c -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #6 from Dima Ryazanov <dima(a)gmail.com> --- Created attachment 61643 --> https://bugs.winehq.org/attachment.cgi?id=61643 test.c -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #7 from Dima Ryazanov <dima(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #8 from Dima Ryazanov <dima(a)gmail.com> --- Oops, don't know where I got the "=%" from. Anything that starts with "=" gets filtered out. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #9 from Jason Edmeades <us(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Jason Edmeades <us(a)edmeades.me.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62293|0 |1 is obsolete| | --- Comment #10 from Jason Edmeades <us(a)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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #11 from Jason Edmeades <us(a)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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Jason Edmeades <us(a)edmeades.me.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Jason Edmeades <us(a)edmeades.me.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|wine-bugs(a)winehq.org |us(a)edmeades.me.uk -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #12 from Dima Ryazanov <dima(a)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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #13 from Jason Edmeades <us(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #14 from Jason Edmeades <us(a)edmeades.me.uk> --- Where did you see the docs on the layout? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #15 from Dima Ryazanov <dima(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #16 from Dima Ryazanov <dima(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #17 from Dima Ryazanov <dima(a)gmail.com> --- That said, the patch does fix "os.environ.clear()" in Python (though I had to work around bug 42474 to test it). -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #18 from Jason Edmeades <us(a)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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #19 from Dima Ryazanov <dima(a)gmail.com> --- Yeah, makes sense. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 --- Comment #20 from Jason Edmeades <us(a)edmeades.me.uk> --- https://www.winehq.org/pipermail/wine-devel/2018-September/132654.html -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Jason Edmeades <us(a)edmeades.me.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |0dcfc97fcbea15037d15dd10218 | |70de25b8382df Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #21 from Jason Edmeades <us(a)edmeades.me.uk> --- Committed -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #22 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 3.17. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |3.0.x -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45320 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|3.0.x |--- --- Comment #23 from Michael Stefaniuc <mstefani(a)winehq.org> --- Removing the 3.0.x milestone from bug fixes included in 3.0.5. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org