Binary PATH execution regression on Windows (since 3.0.15)

[axel3rd]

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):

String bin = "mvn";
String arg = "-version";

Commandline cmd = new Commandline();
cmd.setExecutable( bin );
cmd.createArg().setValue( arg );
Process p = cmd.execute();
p.waitFor();
System.out.println( "Out: " + IOUtil.toString( p.getInputStream() ) );
System.out.println( "Err: " + IOUtil.toString( p.getErrorStream() ) );

Fails with (tested with java 1.7.0_51, 1.8.0_31, 1.8.0_91):

Exception in thread "main" org.codehaus.plexus.util.cli.CommandLineException: Error while executing process.
	at org.codehaus.plexus.util.cli.Commandline.execute(Commandline.java:675)
	at Main.main(Main.java:21)
Caused by: java.io.IOException: Cannot run program "mvn": CreateProcess error=2, Le fichier spécifié est introuvable
	at java.lang.ProcessBuilder.start(ProcessBuilder.java:1041)
	at java.lang.Runtime.exec(Runtime.java:617)
	at org.codehaus.plexus.util.cli.Commandline.execute(Commandline.java:655)
	... 1 more
Caused by: java.io.IOException: CreateProcess error=2, Le fichier spécifié est introuvable
	at java.lang.ProcessImpl.create(Native Method)
	at java.lang.ProcessImpl.<init>(ProcessImpl.java:385)
	at java.lang.ProcessImpl.start(ProcessImpl.java:136)
	at java.lang.ProcessBuilder.start(ProcessBuilder.java:1022)
	... 3 more

Same with cmd embedded command:

String bin = "echo";
String arg = "foo";

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 :

• v3.0.15, all works (with UT Fix : Quote Java binary when contain space (on Windows) #16).
• v3.0.25-SNAPSHOT: some fails, mainly on CommandlineTest :

Some fails are similar to my PATH binary execution problem:

testExecute(org.codehaus.plexus.util.cli.CommandlineTest)
junit.framework.AssertionFailedError: Error while executing process.

testDollarSignInArgumentPath(org.codehaus.plexus.util.cli.CommandlineTest)
java.lang.Exception: Unable to execute command: Error while executing process.

Some others seems more to be a forgot unit test expected results update about quotes management improvements (perhaps except the last/third):

testGetShellCommandLineBash(org.codehaus.plexus.util.cli.CommandlineTest)
junit.framework.ComparisonFailure: expected:<[\bin\echo] 'hello world'> but was:<['\bin\echo'] 'hello world'>

testGetShellCommandLineNonWindows(org.codehaus.plexus.util.cli.CommandlineTest)
junit.framework.ComparisonFailure: expected:<[\usr\bin a b]> but was:<['\usr\bin' 'a' 'b']>

testGetShellCommandLineBash_WithSingleQuotedArg(org.codehaus.plexus.util.cli.CommandlineTest)
junit.framework.ComparisonFailure: expected:<[\bin\echo 'hello world]'> but was:<['\bin\echo' ''"'"'hello world'"'"']'>

---

I could work on PR (fix or UT update), but some opinion/feedback/advice would be useful.

Thanks in advance.
Best regards

[michael-o]

I can confirm these issues on my machines. Can you create PRs for that?

[axel3rd]

@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.

[axel3rd]

Problem is due to Charles Duffy / @krosenvold improvment (b38a1b3 : Commandline shell injection problems).
IMO, re- add for Windows the cmd /c when executable is a cmd internal command or when the prefix path doesn't exist (=> on PATH) is mandatory ...

[axel3rd]

Hummm ... after reflexion, if conditional processing or redirection is used, it is perhaps more sustainable to use getShellCommandline in all cases (for Windows).

[axel3rd]

@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.

[khmarbaise]

Done.

[axel3rd]

@khmarbaise: Thanks !

[tbroyer]

Apparently this breaks Ctrl+C on Windows, which will kill the cmd but not the actual command.
See tbroyer/gwt-maven-plugin#110

[axel3rd]

Apparently 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 when CTRL+C : The ping command 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.

[tbroyer]

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

	final Process p = cl.execute();
	final Thread processHook = new Thread()
	{
	{
	this.setName( "CommandLineUtils process shutdown hook" );
	this.setContextClassLoader( null );
	}
	@Override
	public void run()
	{
	p.destroy();
	}
	};
	ShutdownHookUtils.addShutDownHook( processHook );

[axel3rd]

@tbroyer : The root cause is the usage of cmd.exe /X /C for 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).