diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java index 3a516c46efe..6cad26e4c04 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java @@ -105,13 +105,13 @@ public final class DockerClient { "'=' found in entry for docker command file, key = " + entry .getKey() + "; value = " + entry.getValue()); } - if (entry.getValue().contains("\n")) { + String value = StringUtils.join(",", entry.getValue()); + if (value.contains("\n")) { throw new ContainerExecutionException( "'\\n' found in entry for docker command file, key = " + entry - .getKey() + "; value = " + entry.getValue()); + .getKey() + "; value = " + value); } - printWriter.println(" " + entry.getKey() + "=" + StringUtils - .join(",", entry.getValue())); + printWriter.println(" " + entry.getKey() + "=" + value); } if (cmd instanceof DockerRunCommand) { DockerRunCommand runCommand = (DockerRunCommand) cmd; diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java index 31645bcde44..bd01c7ab484 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java @@ -27,13 +27,16 @@ import org.apache.hadoop.yarn.api.records.ContainerId; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.server.nodemanager.Context; import org.apache.hadoop.yarn.server.nodemanager.LocalDirsHandlerService; +import org.apache.hadoop.yarn.server.nodemanager.containermanager.runtime.ContainerExecutionException; import org.junit.After; import org.junit.Before; import org.junit.Test; import java.io.File; +import java.util.Arrays; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -77,4 +80,47 @@ public class TestDockerClient { File tmpFile = new File(tmpPath); assertTrue(tmpFile + " was not created", tmpFile.exists()); } + + @Test + public void testCommandValidation() throws Exception { + String absRoot = TEST_ROOT_DIR.getAbsolutePath(); + ApplicationId appId = ApplicationId.newInstance(1, 1); + ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 1); + ContainerId cid = ContainerId.newContainerId(attemptId, 1); + Configuration conf = new Configuration(); + conf.set("hadoop.tmp.dir", absRoot); + conf.set(YarnConfiguration.NM_LOCAL_DIRS, absRoot); + conf.set(YarnConfiguration.NM_LOG_DIRS, absRoot); + LocalDirsHandlerService dirsHandler = new LocalDirsHandlerService(); + Context mockContext = mock(Context.class); + doReturn(conf).when(mockContext).getConf(); + doReturn(dirsHandler).when(mockContext).getLocalDirsHandler(); + + DockerClient dockerClient = new DockerClient(); + dirsHandler.init(conf); + dirsHandler.start(); + try { + DockerRunCommand dockerCmd = new DockerRunCommand(cid.toString(), "user", + "image"); + dockerCmd.addCommandArguments("prop=bad", "val"); + dockerClient.writeCommandToTempFile(dockerCmd, cid, + mockContext); + fail("Expected exception writing command file"); + } catch (ContainerExecutionException e) { + assertTrue("Expected key validation error", e.getMessage() + .contains("'=' found in entry for docker command file")); + } + + try { + DockerRunCommand dockerCmd = new DockerRunCommand(cid.toString(), "user", + "image"); + dockerCmd.setOverrideCommandWithArgs(Arrays.asList("sleep", "1000\n")); + dockerClient.writeCommandToTempFile(dockerCmd, cid, mockContext); + fail("Expected exception writing command file"); + } catch (ContainerExecutionException e) { + assertTrue("Expected value validation error", e.getMessage() + .contains("'\\n' found in entry for docker command file")); + } + dirsHandler.stop(); + } }