Merge pull request #6026 from jamezp/WFCORE-6842

[WFCORE-6842] Do not add null environment variables to the ProcessBui…

launcher/src/main/java/org/wildfly/core/launcher/Launcher.java

@@ -192,27 +192,37 @@ public Launcher setDirectory(final String dir) {
     }
 
     /**
-     * Adds an environment variable to the process being created.
+     * Adds an environment variable to the process being created. If the key or value is {@code null}, the environment
+     * variable will not be added.
      *
      * @param key   they key for the variable
      * @param value the value for the variable
      *
      * @return the launcher
+     * @see ProcessBuilder#environment()
      */
     public Launcher addEnvironmentVariable(final String key, final String value) {
-        env.put(key, value);
+        if (key != null && value != null) {
+            env.put(key, value);
+        }
         return this;
     }
 
     /**
-     * Adds the environment variables to the process being created.
+     * Adds the environment variables to the process being created. Note that {@code null} keys or values will not be
+     * added.
      *
      * @param env the environment variables to add
      *
      * @return the launcher
+     * @see ProcessBuilder#environment()
      */
     public Launcher addEnvironmentVariables(final Map<String, String> env) {
-        this.env.putAll(env);
+        env.forEach((key, value) -> {
+            if (key != null && value != null) {
+                addEnvironmentVariable(key, value);
+            }
+        });
         return this;
     }
 

launcher/src/test/java/org/wildfly/core/launcher/LauncherTest.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright The WildFly Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.wildfly.core.launcher;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * @author <a href="mailto:[email protected]">James R. Perkins</a>
+ */
+public class LauncherTest {
+
+    private Path stdout;
+
+    @Before
+    public void setup() throws IOException {
+        stdout = Files.createTempFile("stdout", ".txt");
+    }
+
+    @After
+    public void deleteStdout() throws IOException {
+        if (stdout != null) {
+            Files.deleteIfExists(stdout);
+        }
+    }
+
+    @Test
+    public void checkSingleNullEnvironmentVariable() throws Exception {
+        final TestCommandBuilder commandBuilder = new TestCommandBuilder();
+        checkProcess(Launcher.of(commandBuilder).addEnvironmentVariable("TEST", null));
+    }
+
+    @Test
+    public void checkNullEnvironmentVariables() throws Exception {
+        final TestCommandBuilder commandBuilder = new TestCommandBuilder();
+        final Map<String, String> env = new HashMap<>();
+        env.put("TEST", null);
+        env.put("TEST_2", "test2");
+        checkProcess(Launcher.of(commandBuilder).addEnvironmentVariables(env));
+    }
+
+    private void checkProcess(final Launcher launcher) throws IOException, InterruptedException {
+        Process process = null;
+        try {
+            process = launcher.setRedirectErrorStream(true).redirectOutput(stdout).launch();
+            Assert.assertNotNull("Process should not be null", process);
+            Assert.assertTrue("Process should have exited within 5 seconds", process.waitFor(5, TimeUnit.SECONDS));
+            Assert.assertEquals(String.format("Process should have exited with an exit code of 0:%n%s", Files.readString(stdout)),
+                    0, process.exitValue());
+        } finally {
+            ProcessHelper.destroyProcess(process);
+        }
+    }
+
+    /**
+     * @author <a href="mailto:[email protected]">James R. Perkins</a>
+     */
+    private static class TestCommandBuilder implements CommandBuilder {
+        @Override
+        public List<String> buildArguments() {
+            return List.of();
+        }
+
+        @Override
+        public List<String> build() {
+            return List.of(Jvm.current().getCommand(), "-version");
+        }
+    }
+}