Escape environment variables before pass it to docker run

Jenkinsfile

@@ -1,4 +1,5 @@
 buildPlugin(platforms: [
     'linux',
+    'docker',
     'maven-windows' // TODO Docker-based tests fail when using Docker on Windows. The maven-windows agents do not have Docker installed so tests that require Docker are skipped.
 ])

src/main/java/org/jenkinsci/plugins/docker/workflow/client/DockerClient.java

@@ -29,6 +29,7 @@
 import hudson.FilePath;
 import hudson.Launcher;
 import hudson.model.Node;
+import hudson.os.WindowsUtil;
 import hudson.util.ArgumentListBuilder;
 import hudson.util.VersionNumber;
 import java.io.BufferedReader;
@@ -134,7 +135,7 @@ public String run(@Nonnull EnvVars launchEnv, @Nonnull String image, @CheckForNu
         }
         for (Map.Entry<String, String> variable : containerEnv.entrySet()) {
             argb.add("-e");
-            argb.addMasked(variable.getKey()+"="+variable.getValue());
+            argb.addMasked(quoteArgument(variable));
         }
         argb.add(image).add(command);
 
@@ -388,4 +389,14 @@ public List<String> getVolumes(@Nonnull EnvVars launchEnv, String containerID) t
         }
         return Arrays.asList(volumes.replace("\\", "/").split("\\n"));
     }
+
+    /**
+     * Convert key-value pair (variable) to Docker container environment variable argument.
+     * It handles some difficult cases like a variable containing spaces and quotes.
+     * @param variable a variable as a key-value pair.
+     * @return a string representation of the passed variable.
+     */
+    private static String quoteArgument(Map.Entry<String, String> variable) {
+        return WindowsUtil.quoteArgument(variable.getKey() + "=" + variable.getValue());
+    }
 }

src/test/java/org/jenkinsci/plugins/docker/workflow/DockerDSLTest.java

@@ -400,4 +400,21 @@ private static void grep(File dir, String text, String prefix, Set<String> match
             }
         });
     }
+
+    @Test public void insideContextOfEnvironmentVariablesWithSpecialCharacters(){
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                assumeDocker();
+                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "prj");
+                p.setDefinition(new CpsFlowDefinition(
+                    "node {\n" +
+                    "  withEnv(['ENV_VAR=\"VARIABLE = VALUE WITH QUOTES \\\" AND SPACES \\\"\"']) {\n" +
+                    "    docker.image('busybox').inside { sh 'env' }\n" +
+                    "  }\n" +
+                    "}", true));
+                WorkflowRun r = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+                story.j.assertLogContains("ENV_VAR=\"VARIABLE = VALUE WITH QUOTES \" AND SPACES \"\"", r);
+            }
+        });
+    }
 }

src/test/java/org/jenkinsci/plugins/docker/workflow/client/DockerClientTest.java

@@ -36,6 +36,7 @@
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.Map;
 
 /**
  * @author <a href="mailto:tom.fennelly@gmail.com">tom.fennelly@gmail.com</a>
@@ -75,6 +76,41 @@ public void test_run() throws IOException, InterruptedException {
         Assert.assertNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
     }
 
+    @Test
+    public void test_run_with_env_vars() throws IOException, InterruptedException {
+        EnvVars launchEnv = DockerTestUtil.newDockerLaunchEnv();
+        EnvVars containerEnv = new EnvVars(
+            "VAR", "A",
+            "VAR_WITH_SPACE", "A B",
+            "VAR_WITH_EQUALS", "A=B",
+            "VAR_WITH_QUOTE", "A\"B",
+            "VAR_WITH_SPACE_AND_EQUALS", "A=B C",
+            "VAR_WITH_SPACE_EQUALS_AND_QUOTE", "A=\"B C\""
+        );
+        String containerId =
+                dockerClient.run(launchEnv, "learn/tutorial", null, null, Collections.<String, String>emptyMap(), Collections.<String>emptyList(), containerEnv,
+                        dockerClient.whoAmI(), "cat");
+        Assert.assertEquals(64, containerId.length());
+        ContainerRecord containerRecord = dockerClient.getContainerRecord(launchEnv, containerId);
+        Assert.assertEquals(dockerClient.inspect(launchEnv, "learn/tutorial", ".Id"), containerRecord.getImageId());
+        Assert.assertTrue(containerRecord.getContainerName().length() > 0);
+        Assert.assertTrue(containerRecord.getHost().length() > 0);
+        Assert.assertTrue(containerRecord.getCreated() > 1000000000000L);
+        Assert.assertEquals(Collections.<String>emptyList(), dockerClient.getVolumes(launchEnv, containerId));
+
+        String envVarsString = dockerClient.inspect(launchEnv, containerId, ".Config.Env");
+        Assert.assertNotNull(envVarsString);
+
+        for (Map.Entry<String, String> variable : containerEnv.entrySet()) {
+            Assert.assertTrue(envVarsString.contains(variable.getKey() + "=" + variable.getValue()));
+        }
+
+        // Also test that the stop works and cleans up after itself
+        Assert.assertNotNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
+        dockerClient.stop(launchEnv, containerId);
+        Assert.assertNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
+    }
+
     @Test
     public void test_valid_version() {
         VersionNumber dockerVersion = DockerClient.parseVersionNumber("Docker version 1.5.0, build a8a31ef");

src/test/java/org/jenkinsci/plugins/docker/workflow/client/WindowsDockerClientTest.java

@@ -12,6 +12,7 @@
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.Map;
 
 public class WindowsDockerClientTest {
 
@@ -54,4 +55,46 @@ public void test_run() throws IOException, InterruptedException {
         dockerClient.stop(launchEnv, containerId);
         Assert.assertNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
     }
+
+    @Test
+    public void test_run_with_env_vars() throws IOException, InterruptedException {
+        EnvVars launchEnv = DockerTestUtil.newDockerLaunchEnv();
+        EnvVars containerEnv = new EnvVars(
+            "VAR", "A",
+            "VAR_WITH_SPACE", "A B",
+            "VAR_WITH_EQUALS", "A=B",
+            "VAR_WITH_QUOTE", "A\"B",
+            "VAR_WITH_SPACE_AND_EQUALS", "A=B C",
+            "VAR_WITH_SPACE_EQUALS_AND_QUOTE", "A=\"B C\""
+        );
+        String containerId = dockerClient.run(
+            launchEnv,
+            "learn/tutorial",
+            null,
+            null,
+            Collections.emptyMap(),
+            Collections.emptyList(),
+            containerEnv,
+            dockerClient.whoAmI(),
+            "cat");
+        Assert.assertEquals(64, containerId.length());
+        ContainerRecord containerRecord = dockerClient.getContainerRecord(launchEnv, containerId);
+        Assert.assertEquals(dockerClient.inspect(launchEnv, "learn/tutorial", ".Id"), containerRecord.getImageId());
+        Assert.assertTrue(containerRecord.getContainerName().length() > 0);
+        Assert.assertTrue(containerRecord.getHost().length() > 0);
+        Assert.assertTrue(containerRecord.getCreated() > 1000000000000L);
+        Assert.assertEquals(Collections.<String>emptyList(), dockerClient.getVolumes(launchEnv, containerId));
+
+        String envVarsString = dockerClient.inspect(launchEnv, containerId, ".Config.Env");
+        Assert.assertNotNull(envVarsString);
+
+        for (Map.Entry<String, String> variable : containerEnv.entrySet()) {
+            Assert.assertTrue(envVarsString.contains(variable.getKey() + "=" + variable.getValue()));
+        }
+
+        // Also test that the stop works and cleans up after itself
+        Assert.assertNotNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
+        dockerClient.stop(launchEnv, containerId);
+        Assert.assertNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
+    }
 }