From b2d7204ed0b64405a0adbe07e3eaf385e046efa1 Mon Sep 17 00:00:00 2001
From: Eric Yang <eyang@apache.org>
Date: Fri, 14 Dec 2018 17:52:26 -0500
Subject: [PATCH] YARN-9125.  Fixed Carriage Return detection in Docker
 container launch command.             Contributed by Billie Rinaldi

---
 .../linux/runtime/docker/DockerClient.java    |  8 ++--
 .../runtime/docker/TestDockerClient.java      | 46 +++++++++++++++++++
 2 files changed, 50 insertions(+), 4 deletions(-)

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 3a516c46ef..6cad26e4c0 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 String writeCommandToTempFile(DockerCommand cmd,
                 "'=' 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 31645bcde4..bd01c7ab48 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.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 void testWriteCommandToTempFile() throws Exception {
     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();
+  }
 }