When executing a command on Windows, add its directory path to the Pa…

…th environment variable and execute it using its name. We do that to avoid passing an executable path that contains spaces, which cause parsing issues in Windows CMD.

build-info-extractor-go/src/main/java/org/jfrog/build/extractor/go/GoDriver.java

@@ -1,8 +1,6 @@
 package org.jfrog.build.extractor.go;
 
-import com.google.common.collect.Maps;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.commons.lang3.SystemUtils;
 import org.jfrog.build.api.util.Log;
 import org.jfrog.build.extractor.executor.CommandExecutor;
 import org.jfrog.build.extractor.executor.CommandResults;
@@ -26,60 +24,18 @@ public class GoDriver implements Serializable {
     private static final String GO_GET_CMD = "get";
     private static final String GO_LIST_MODULE_CMD = "list -m";
     private static final String GO_VERSION_CMD = "version";
-    private static final String DEFAULT_EXECUTABLE_NAME = "go";
 
     private static final long serialVersionUID = 1L;
     private final CommandExecutor commandExecutor;
     private final File workingDirectory;
     private final Log logger;
 
     public GoDriver(String executablePath, Map<String, String> env, File workingDirectory, Log logger) {
-        this.commandExecutor = generateCommandExecutor(executablePath, env);
+        this.commandExecutor = new CommandExecutor(StringUtils.defaultIfEmpty(executablePath, "go"), env);
         this.workingDirectory = workingDirectory;
         this.logger = logger;
     }
 
-    /**
-     * Generate a new mutable copy of environment variables map with the Go executable directory path inserted to the beginning of the Path.
-     *
-     * @param executablePath Go executable path
-     * @param env            Environment variables map
-     * @return a new Environment variables map
-     */
-    static Map<String, String> generateWindowsEnv(String executablePath, Map<String, String> env) {
-        // If executablePath ends with "go" or "go.exe" - remove it from the directory path
-        executablePath = StringUtils.removeEnd(executablePath, ".exe");
-        executablePath = StringUtils.removeEnd(executablePath, DEFAULT_EXECUTABLE_NAME);
-
-        // Insert the Go executable directory path to the beginning of the Path environment variable
-        // Make sure to copy the environment variables map to avoid changing the original map or in case it is immutable
-        Map<String, String> newEnv = Maps.newHashMap(env);
-        String windowsPathEnvKey = "Path";
-        if (newEnv.containsKey(windowsPathEnvKey)) {
-            newEnv.put(windowsPathEnvKey, executablePath + File.pathSeparator + newEnv.get(windowsPathEnvKey));
-        } else {
-            newEnv.put(windowsPathEnvKey, executablePath);
-        }
-        return newEnv;
-    }
-
-    /**
-     * Create a CommandExecutor with the given executable path and environment variables.
-     *
-     * @param executablePath Go executable path
-     * @param env            Environment variables map
-     * @return CommandExecutor
-     */
-    private static CommandExecutor generateCommandExecutor(String executablePath, Map<String, String> env) {
-        if (!SystemUtils.IS_OS_WINDOWS || StringUtils.isBlank(executablePath) || StringUtils.equals(executablePath, DEFAULT_EXECUTABLE_NAME) || env == null) {
-            return new CommandExecutor(StringUtils.defaultIfEmpty(executablePath, DEFAULT_EXECUTABLE_NAME), env);
-        }
-        // Handling Windows case with a given executable path:
-        // A bug was identified for the Go executable in Windows where the executable path may be incorrectly parsed
-        // as two command arguments when the path contains spaces (e.g., "C:\Program Files\Go\bin\go.exe").
-        return new CommandExecutor(DEFAULT_EXECUTABLE_NAME, generateWindowsEnv(executablePath, env));
-    }
-
     public CommandResults runCmd(String args, boolean verbose) throws IOException {
         List<String> argsList = new ArrayList<>(Arrays.asList(args.split(" ")));
         return runCmd(argsList, verbose);

build-info-extractor-go/src/test/java/org/jfrog/build/extractor/go/GoDriverTest.java

@@ -2,11 +2,9 @@
 
 import com.google.common.collect.Sets;
 import org.apache.commons.io.FileUtils;
-import org.apache.commons.lang3.SystemUtils;
 import org.jfrog.build.api.util.NullLog;
 import org.jfrog.build.extractor.executor.CommandResults;
 import org.testng.Assert;
-import org.testng.SkipException;
 import org.testng.annotations.Test;
 
 import java.io.File;
@@ -15,7 +13,6 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Arrays;
-import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -117,20 +114,4 @@ public void testGoGet() throws IOException {
             FileUtils.deleteDirectory(projectDir);
         }
     }
-
-    @Test
-    public void testGoDriverWindowsInit() throws IOException {
-        if (!SystemUtils.IS_OS_WINDOWS) {
-            throw new SkipException("Skipping non-windows test");
-        }
-        File projectDir = Files.createTempDirectory("").toFile();
-        try {
-            Map<String, String> systemEnv = System.getenv();
-            String executablePath = "C:\\Program Files\\Go\\bin\\go";
-            Map<String, String> executorEnv = GoDriver.generateWindowsEnv(executablePath, systemEnv);
-            assertTrue(executorEnv.get("Path").startsWith("C:\\Program Files\\Go\\bin"));
-        } finally {
-            FileUtils.deleteDirectory(projectDir);
-        }
-    }
 }

build-info-extractor/src/main/java/org/jfrog/build/extractor/executor/CommandExecutor.java

@@ -1,5 +1,6 @@
 package org.jfrog.build.extractor.executor;
 
+import com.google.common.collect.Maps;
 import org.apache.commons.lang3.SystemUtils;
 import org.jfrog.build.api.util.Log;
 import org.jfrog.build.extractor.UrlUtils;
@@ -8,11 +9,12 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Serializable;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.*;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
 
 import static java.lang.String.format;
 import static java.lang.String.join;
@@ -26,7 +28,7 @@ public class CommandExecutor implements Serializable {
     private static final int READER_SHUTDOWN_TIMEOUT_SECONDS = 30;
     private static final int PROCESS_TERMINATION_TIMEOUT_SECONDS = 30;
     private static final int EXECUTION_TIMEOUT_MINUTES = 120;
-    private Map<String, String> env;
+    private final Map<String, String> env;
     private final String executablePath;
 
     /**
@@ -133,10 +135,9 @@ public CommandResults exeCommand(File execDir, List<String> args, List<String> c
      */
     public CommandResults exeCommand(File execDir, List<String> args, List<String> credentials, Log logger, long timeout, TimeUnit unit) throws InterruptedException, IOException {
         List<String> command = new ArrayList<>(args);
-        command.add(0, executablePath);
         ExecutorService service = Executors.newFixedThreadPool(2);
         try {
-            Process process = runProcess(execDir, command, credentials, env, logger);
+            Process process = runProcess(execDir, executablePath, command, credentials, env, logger);
             // The output stream is not necessary in non-interactive scenarios, therefore we can close it now.
             process.getOutputStream().close();
             try (InputStream inputStream = process.getInputStream(); InputStream errorStream = process.getErrorStream()) {
@@ -179,13 +180,21 @@ private CommandResults getCommandResults(boolean terminatedProperly, List<String
         return commandRes;
     }
 
-    private static Process runProcess(File execDir, List<String> args, List<String> credentials, Map<String, String> env, Log logger) throws IOException {
+    private static Process runProcess(File execDir, String executablePath, List<String> args, List<String> credentials, Map<String, String> env, Log logger) throws IOException {
         if (credentials != null) {
             args.addAll(credentials);
         }
         if (SystemUtils.IS_OS_WINDOWS) {
+            Path execPath = Paths.get(executablePath);
+            if (execPath.isAbsolute()) {
+                env = generateWindowsEnv(execPath, env);
+                args.add(0, execPath.getFileName().toString());
+            } else {
+                args.add(0, executablePath);
+            }
             args.addAll(0, Arrays.asList("cmd", "/c"));
         } else {
+            args.add(0, executablePath);
             String strArgs = join(" ", args);
             args = new ArrayList<String>() {{
                 add("/bin/sh");
@@ -200,6 +209,32 @@ private static Process runProcess(File execDir, List<String> args, List<String>
         return processBuilder.start();
     }
 
+    /**
+     * Generate a new mutable copy of environment variables map with the executable directory path inserted at the
+     * beginning of the Path.
+     * This is done to handle cases where the executable path contains spaces. In such scenarios, the "cmd" command used
+     * to execute this command in Windows may incorrectly parse the path, treating the section after the space as an
+     * argument for the command.
+     *
+     * @param execPath the executable path
+     * @param env      environment variables map
+     * @return a new environment variables map
+     */
+    static Map<String, String> generateWindowsEnv(Path execPath, Map<String, String> env) {
+        String execDirPath = execPath.getParent().toString();
+
+        // Insert the executable directory path to the beginning of the Path environment variable.
+        // Make sure to copy the environment variables map to avoid changing the original map or in case it is immutable.
+        Map<String, String> newEnv = Maps.newHashMap(env);
+        String windowsPathEnvKey = "Path";
+        if (newEnv.containsKey(windowsPathEnvKey)) {
+            newEnv.put(windowsPathEnvKey, execDirPath + File.pathSeparator + newEnv.get(windowsPathEnvKey));
+        } else {
+            newEnv.put(windowsPathEnvKey, execDirPath);
+        }
+        return newEnv;
+    }
+
     /**
      * Escape spaces in the input executable path and trim leading and trailing whitespaces.
      *

build-info-extractor/src/test/java/org/jfrog/build/extractor/executor/CommandExecutorTest.java

@@ -1,8 +1,10 @@
 package org.jfrog.build.extractor.executor;
 
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.SystemUtils;
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.jfrog.build.api.util.NullLog;
+import org.testng.SkipException;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import org.testng.collections.Lists;
@@ -12,8 +14,10 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 import static org.testng.Assert.*;
 
@@ -85,4 +89,20 @@ public void testExeCommandWithSpaces() throws IOException, InterruptedException
             FileUtils.forceDelete(tmpDir.toFile());
         }
     }
+
+    @Test
+    public void testGenerateWindowsEnv() throws IOException {
+        if (!SystemUtils.IS_OS_WINDOWS) {
+            throw new SkipException("Skipping test on non-Windows OS");
+        }
+        File projectDir = Files.createTempDirectory("").toFile();
+        try {
+            Map<String, String> systemEnv = System.getenv();
+            Path execPath = Paths.get("C:\\Program Files\\Go\\bin\\go");
+            Map<String, String> executorEnv = CommandExecutor.generateWindowsEnv(execPath, systemEnv);
+            assertTrue(executorEnv.get("Path").startsWith("C:\\Program Files\\Go\\bin"));
+        } finally {
+            FileUtils.deleteDirectory(projectDir);
+        }
+    }
 }