https://bugs.winehq.org/show_bug.cgi?id=42508
Bug ID: 42508 Summary: start.exe does not detect its title argument when it should (breaking .e.g URL opening in League of Legends) Product: Wine Version: unspecified Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: programs Assignee: wine-bugs@winehq.org Reporter: hydratech@gmail.com Distribution: ---
Created attachment 57397 --> https://bugs.winehq.org/attachment.cgi?id=57397 Trace showing League of Legends Client attempting to open a URL.
When start.exe is invoked, its first argument in quotes that is not an argument of the application to be executed should be interpreted as the console title. Wine's start.exe fails to do so.
-- How to reproduce -- Running the following line in Wine's cmd should open a URL in your browser: Z:>start "" https://winehq.org/
the actual result is an application not found error. On Windows the same line opens the website as expected.
-- Why this is a bug -- I know this was marked as fixed in bug 10913 but the reasoning was incorrect.
The argument for stating the bug was fixed was, that when "escaping your quotes" in cmd to the title was parsed. This is however not consistent with with Windows behaviour.
As explained at the end of that bug report, if you escape your quotes in Wine's cmd start.exe parses the argument as the console title, i.e.: Z:>start "hello" cmd
I will now clarify why this is incorrect by showing Windows' behaviour.
The following command lines are not equivalent on Windows:
C:>start hello.exe
vs.
C:>start "hello.exe"
The latter will open a new cmd window titled hello.exe whereas the former attempts to execute an application named hello.exe. In other words cmd's start built-in parses its own command-line checking wether quotes where used. In fact escaping the quotes using back-slashes is invalid as it tries start a process named \hello.exe. I have tested this on both Windows 7 and Windows XP.
In other words, this is invalid: C:>start "hello.exe"
To illustrate how unintuitive this Windows behaviour is: C:>start "C:\Program Files\Application\hello.exe"
Few would guess this actually opens a console titled as such, instead of running an application. That's why I can understand the maintainers would think the bug was fixed.
Wine will however correctly emulate unescaped quote-senstive behaviour with the echo built-in: Z:>echo hoi hoi
Z:>echo "hoi" "hoi"
We can see echo being "aware" of the quotes and correctly displaying them; behaviour identical to Windows' echo built-in.
Looking at the wine source, start.exe relies on its "argv" parameter instead of using GetCommandLine() and thereby accessing the unmodified command-line. As a consequence the difference between a quoted argument and a non-quoted argument is lost as CreateProcess strips the quotes off the parameters when passing them to execve().
To quote the source of Wine's cmd.exe (wcmdmain.c): /* Can't use argc/argv as it will have stripped quotes from parameters * meaning cmd.exe /C echo "quoted string" is impossible */
It should now be evident that start.exe should either become a wrapper around cmd.exe's built-in start function, invoking cmd and let cmd itself implement start.exe's actual behaviour, or implement GetCommandLine() based argument parsing similar to cmd.exe's within start.exe, leading to duplicated command-line parsing code. Both are not ideal solutions.
-- Why does this bug matter? --
There are actually Windows applications relying on this behaviour to open URLs in the user's browser. As an example, League of Legends executes the following shell command to open a URL: cmd.exe /c start "" "<url here>"
In other words, League instructs start to launch an untitled shell window with a URL as application, effectively causing the shell to pass it to the browser. However, wine's start.exe interprets this as League trying to execute an empty string as executable name, with a URL as argument. I have attached the relevant output running League with WINEDEBUG=cmd,start,process,exec set.
I will speculate that more applications will run into this exact same issue. League's client is based on CEF and it might very well be CEF which implements this style of external URL opening, furthermore there is documentation recommending the use of empty quotes as console title argument when invoking start to prevent it from accidentally parsing follow up arguments in quotes as the console title, think "C:\Program Files\etc...", establishing this as a pattern that might well have been adopted by other applications. See: https://ss64.com/nt/start.html
I will also say that, given some instruction, I will be glad to help fixing this bug and write any necessary code.
There is actually another issue with current start.exe's console title parsing which I will submit as a separate issue. That issue is however, significantly easier to fix. Finally this also means wine's current handling of command-line arguments might need fixing as: $ wine start '""' "http://winehq.og" will break in a fixed version of 'start.exe', while it should work. For this I will file yet another issue, although the necessity of this fix is debatable.
https://bugs.winehq.org/show_bug.cgi?id=42508
michael bishop cleverca22@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cleverca22@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=42508
David K. hevanen@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hevanen@googlemail.com
https://bugs.winehq.org/show_bug.cgi?id=42508
Bernhard Übelacker bernhardu@mailbox.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |bernhardu@mailbox.org
--- Comment #1 from Bernhard Übelacker bernhardu@mailbox.org --- Came across this report because the launcher for "syngo fastView" triggers this problem too.
It looks like the quotes get removed in kernel32/process.c, function build_argv.
As explained at the end of that bug report, if you escape your quotes in Wine's cmd start.exe parses the argument as the console title, i.e.: Z:>start "hello" cmd
This does not yet work if it contains a space. But adding another pair unescaped quotation marks around would work again: c:>start ""a b"" wineconsole
It should now be evident that start.exe should either become a wrapper around cmd.exe's built-in start function, invoking cmd and let cmd itself implement start.exe's actual behaviour, or implement GetCommandLine() based argument parsing similar to cmd.exe's within start.exe, leading to duplicated command-line parsing code. Both are not ideal solutions.
Therefore, just for completeness, there is probably a third way: In cmd/builtins.c WCMD_start encode simple quotes to escaped quotes with surrounding quotes.
But I don't know which way would be the best.
https://bugs.winehq.org/show_bug.cgi?id=42508
--- Comment #2 from Bas Weelinck hydratech@gmail.com --- Created attachment 57562 --> https://bugs.winehq.org/attachment.cgi?id=57562 Preliminary patch which escapes the quotes in the title argument to start, but does not add quotes surrounding those.
https://bugs.winehq.org/show_bug.cgi?id=42508
--- Comment #3 from Bas Weelinck hydratech@gmail.com --- Created attachment 57563 --> https://bugs.winehq.org/attachment.cgi?id=57563 output log showing cmd quote handling parse behaviour for arguments surrounded by quotes but containing escaped quotes.
I spoke to madewokherd in #winehackers yesterday and he suggested the same. As such I have appended a patch for your testing pleasure. It works for quoted arguments containing no spaces, but has the problems you described when the argument contains spaces. So we definitely want to add surrounding quotes.
I did run into another issue though, it seems WCMD_parameter_with_delims does not handle escaped quotes. I am not sure if it is a bug or not, but it does seem odd to me. It can cause WCMD_parameter to return an argument that is not actually surrounded by quotes, but does start with one. The quote block is terminated by the escaped quote, whereafter the next delimiter terminates the entire argument.
I've attached output of a wine running with the patch and the debug channels start,cmd,exec,process where you can observe this behaviour.
https://bugs.winehq.org/show_bug.cgi?id=42508
Bas Weelinck hydratech@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #57562|0 |1 is obsolete| |
--- Comment #4 from Bas Weelinck hydratech@gmail.com --- Created attachment 57622 --> https://bugs.winehq.org/attachment.cgi?id=57622 Updated patch which completely implements start's title detection and parsing, but does not handle switch chaining.
I have created a new patch which should completely fix this problem. It employs the exact same parsing behaviour as start does on Windows when it comes to processing the console title. This also means that spaces in the title are now no longer a problem.
start does have the behaviour of treating slashes as argument separators, thus:
start "hello"/i
is valid and will treat /i as a switch and not part of the console title.
This chained switches behaviour is however not implemented by both cmd and start.exe as of yet. My point of this patch was however to get the start built-in to correctly detect the console title and handle it as such, which it now should.
I'll soon take the necessary steps to get the patch submitted to wine.
https://bugs.winehq.org/show_bug.cgi?id=42508
Gijs Vermeulen acescopezz@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |acescopezz@gmail.com
--- Comment #5 from Gijs Vermeulen acescopezz@gmail.com --- This should have been fixed with: https://source.winehq.org/git/wine.git/commit/9baceabb88e2aeb90a32bb5f692310...
https://bugs.winehq.org/show_bug.cgi?id=42508
Gijs Vermeulen gijsvrm@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hydratech@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=42508
winetest@luukku.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |winetest@luukku.com
--- Comment #6 from winetest@luukku.com --- (In reply to Bas Weelinck from comment #4)
Created attachment 57622 [details]
I'll soon take the necessary steps to get the patch submitted to wine.
Is this bug now fixed? I tried to patch wine with the patch but it rejects it and comment above indicates a commit message.
https://bugs.winehq.org/show_bug.cgi?id=42508
Bas Weelinck hydratech@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED
--- Comment #7 from Bas Weelinck hydratech@gmail.com --- (In reply to winetest from comment #6)
(In reply to Bas Weelinck from comment #4)
Created attachment 57622 [details]
I'll soon take the necessary steps to get the patch submitted to wine.
Is this bug now fixed? I tried to patch wine with the patch but it rejects it and comment above indicates a commit message.
Yes, I've just checked out wine-2.13 and the patches have indeed been included. marking as fixed.
https://bugs.winehq.org/show_bug.cgi?id=42508
fjfrackiewicz@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fjfrackiewicz@gmail.com
--- Comment #8 from fjfrackiewicz@gmail.com --- (In reply to Bas Weelinck from comment #7)
(In reply to winetest from comment #6)
(In reply to Bas Weelinck from comment #4)
Created attachment 57622 [details]
I'll soon take the necessary steps to get the patch submitted to wine.
Is this bug now fixed? I tried to patch wine with the patch but it rejects it and comment above indicates a commit message.
Yes, I've just checked out wine-2.13 and the patches have indeed been included. marking as fixed.
For future reference, please include the SHA1 number of the commit that fixed the issue in the "Fixed by SHA1" field :)
So, according to Gils, the SHA1 that fixes this issue is 9baceabb88e2aeb90a32bb5f6923107904ed58ea
https://bugs.winehq.org/show_bug.cgi?id=42508
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #9 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 2.14.
https://bugs.winehq.org/show_bug.cgi?id=42508
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |9baceabb88e2aeb90a32bb5f692 | |3107904ed58ea CC| |focht@gmx.net Version|unspecified |2.2