Repository navigation
Binary PATH execution regression on Windows (since 3.0.15) #17
Description
Activity
I can confirm these issues on my machines. Can you create PRs for that?
@michael-o : Thanks for the bump ^^, it was out of my mind. I had a look on problem when I have opened the bug, but the root cause wasn't trivial (for me). I will try to found the time todo that.
Reacted by Michael OsipovProblem is due to Charles Duffy / @krosenvold improvment (b38a1b3 : Commandline shell injection problems).
IMO, re- add for Windows thecmd /cwhen executable is a cmd internal command or when the prefix path doesn't exist (=> on PATH) is mandatory ...Hummm ... after reflexion, if conditional processing or redirection is used, it is perhaps more sustainable to use
getShellCommandlinein all cases (for Windows).- added a commit that references this issue
on Jun 18, 2017 @michael-o (or a maintainer): Could you please update the milestone to 3.1.0 ? Version has been released today, the changelog should be relevant (this fix/change is major for Windows usage). Thank you.
Done.
@khmarbaise: Thanks !
Apparently this breaks Ctrl+C on Windows, which will kill the
cmdbut not the actual command.
See tbroyer/gwt-maven-plugin#110Apparently this breaks Ctrl+C on Windows, which will kill the cmd but not the actual command.
See tbroyer/gwt-maven-plugin#110@tbroyer : Do you have a simple use case to reproduce the problem ? Because using this simple
Main:import java.io.File; import org.codehaus.plexus.util.cli.Commandline; public class Main { public static void main(String[] args) throws Exception { Commandline cmd = new Commandline(); cmd.setWorkingDirectory(new File(".")); cmd.setExecutable("ping"); cmd.createArg().setValue("-n"); cmd.createArg().setValue("1000"); cmd.createArg().setValue("127.0.0.1"); Process process = cmd.execute(); System.out.println("Executed"); while (process.isAlive()) { System.out.println("Wait"); Thread.sleep(5000); } } }And execute it in a cmd (
java -cp C:/[maven-repo]/org/codehaus/plexus/plexus-utils/3.1.0/plexus-utils-3.1.0.jar;. Main), the result is the same with v3.10 or v3.0.16 whenCTRL+C: Thepingcommand is always ran and attached to cmd.Same behavior with native Java Runtime:
Runtime runtime = Runtime.getRuntime(); Process process = runtime.exec(new String[] { "ping", "-n", "1000", "127.0.0.1" });Is the problem not more linked to differences between a Linux Shell and Windows cmd ?
I agree with you that this behavior could be a problem, but is it not more to Java code to manage that, by adding a hook (for sample).
Process process = cmd.execute(); Runtime.getRuntime().addShutdownHook(new Thread() { public void run() { process.destroyForcibly(); } });Perhaps could be a "feature" of
Commandline(e.g: if Windows, add this shutdown hook by default) ... but in all case addressed in a new issue and validated by a maintainer.Do you have a simple use case to reproduce the problem ?
Best I can provide is:
> mvn archetype:generate ^ -DarchetypeGroupId=net.ltgt.gwt.archetypes ^ -DarchetypeArtifactId=modular-webapp ^ -DarchetypeVersion=2018.5.2 […] > mvn gwt:codeserver -pl *-client -am
I agree with you that this behavior could be a problem, but is it not more to Java code to manage that, by adding a hook (for sample).
There is a shutdown hook already:
plexus-utils/src/main/java/org/codehaus/plexus/util/cli/CommandLineUtils.java
Lines 136 to 154 in ac81ee2
@tbroyer : The root cause is the usage of
cmd.exe /X /Cfor Windows Shell. The problem could be reproduced with this simple snippet (in IDE or native command line) :final String[] cmdNative = new String[] { "ping", "-n", "1000", "127.0.0.1" }; // --> Will be killed final String[] cmdSlashC = new String[] { "cmd.exe", "/X", "/C", "ping", "-n", "1000", "127.0.0.1" }; // Will not be killed Runtime runtime = Runtime.getRuntime(); Process process = runtime.exec(cmdSlashC); Thread.sleep(5000); process.destroy();But I have not (now) found any solution to destroy/kill the sub-process started by
cmd.
Please open a new issue for this problem, this one is closed and not really same perimeter (although linked).I personally do not understand why we have to launch a subshell at all. Why can't we simply pass the command as-is? @khmarbaise
I personally do not understand why we have to launch a subshell at all. Why can't we simply pass the command as-is?
@michael-o : Without that, on Windows, any binaries on PATH or any embedded commands (like
echo) doesn't work 😢 (More details in beginning of this issue).I don't see the bug, if you want some builtin command of an executable, you have to run that executable. Executables work, cmd scripts work for me:
public static void main(String[] args) throws IOException { ProcessBuilder builder = new ProcessBuilder("svn", "--version"); builder.redirectOutput(Redirect.INHERIT); builder.start(); }output:
svn, version 1.9.7 (r1800392) compiled Aug 8 2017, 22:14:48 on x86-microsoft-windows Copyright (C) 2017 The Apache Software Foundation. This software consists of contributions made by many people; see the NOTICE file for more information. Subversion is open source software, see http://subversion.apache.org/ The following repository access (RA) modules are available: * ra_svn : Module for accessing a repository using the svn network protocol. - with Cyrus SASL authentication - handles 'svn' scheme * ra_local : Module for accessing a repository on local disk. - handles 'file' scheme * ra_serf : Module for accessing a repository via WebDAV protocol using serf. - using serf 1.3.9 (compiled with 1.3.9) - handles 'http' scheme - handles 'https' scheme The following authentication credential caches are available: * Wincrypt cache in C:\Users\mosipov\AppData\Roaming\Subversionanother one:
public static void main(String[] args) throws IOException { ProcessBuilder builder = new ProcessBuilder("mvn.cmd", "--version"); builder.redirectOutput(Redirect.INHERIT); builder.start(); }output:
Apache Maven 3.5.3 (3383c37e1f9e9b3bc3df5050c29c8aff9f295297; 2018-02-24T20:49:05+01:00) Maven home: D:\Entwicklung\Programme\apache-maven-3.5.3\bin\.. Java version: 1.8.0_172, vendor: Oracle Corporation Java home: C:\Program Files\Java\jdk1.8.0_172\jre Default locale: de_DE, platform encoding: Cp1252 OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"@michael-o : The problem is mainly with Windows shell interpretations ... if you open a shell on Linux or Windows (cmd), the commands
echo foobarormvn -versionwill work.These interpretation are not native on Windows when invoking:
ProcessBuilder builder = new ProcessBuilder("mvn", "--version"); builder.redirectOutput(Redirect.INHERIT); builder.start(); --> Will fail, no '.bat' or '.cmd'CommandLineUtilsis here to be cross-platform ... so if we should manage OS specific usage (extensions, ...) the interest is less.
(On another side I'm agree that the not subprocess killed whenCTRL+Cis used is a problem ... )echois a shell builtin. Don't rely on. The missing suffix is purely a Windows problem. I see no issue passingmvn.cmd. Yes, it is cumbersome, but Microsoft ist to blame.So this commit can be purely removed ... or we can to deal with these corners cases, with:
- When binary can't be found (ex:
mvn,IOExceptionwith message containsCreateProcess error=2), trying to find it on PATH withwherecommand and retry the command with complete binary path if found - For cmd builtins (hardcoded list), use
cmd /X /Cas prefix
I agree it is not a really beautiful solution, but I did not see other solution to be cross-platform as much as it can.
- When binary can't be found (ex:
I wouldn't handle both at all:
- This is purely a client issue to pass correct parameters for his env. It is not our task to search for.
- Again, this is a client issue. If the client wants
echolet him passcmd.exe,/X,/Con his own.
All we do is simply to abstract. As soon we leave Java with
Processwe cannot be platform-neutral.OK. So commit a49305fb52674cfec3a180cb5fae11baea1423e3 should be revert.
Yes, and in my opinion it needs a redesign. At the end, it is not our task to invoke
cmdorsh, it Java one's.
Hi,
Upgrading plexus-utils from 3.0.15 to last version in some application using Commandline occurs problems on Windows for executed binaries when on Windows PATH.
With v3.0.24, this snippet (not the best stream management, this is just a sample):
Fails with (tested with java 1.7.0_51, 1.8.0_31, 1.8.0_91):
Same with cmd embedded command:
NB: These snippets worked fine in v3.0.15 (or in pure cmd command-line).
Is there some upgrades to do for this code continuing to work ? (Environment items to fill, ...)
I have tried to use
cmd.addSystemEnvironment();(containing PATH), but no change.I'm not able to establish if it should be considered as a bug or not, because executing all plexus-utils unit tests (on Windows 10) for :
Some fails are similar to my PATH binary execution problem:
Some others seems more to be a forgot unit test expected results update about quotes management improvements (perhaps except the last/third):
I could work on PR (fix or UT update), but some opinion/feedback/advice would be useful.
Thanks in advance.
Best regards