[GitHub] [incubator-dolphinscheduler] Eights-Li commented on a change in pull request #4042: [FIX-3900][server] kill multi yarn app in one job

[GitBox <gi...@apache.org>]

Eights-Li commented on a change in pull request #4042:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4042#discussion_r524224257



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java
##########
@@ -38,368 +41,406 @@
 import java.util.regex.Pattern;
 
 /**
- *  mainly used to get the start command line of a process.
+ * mainly used to get the start command line of a process.
  */
 public class ProcessUtils {
-  /**
-   * logger.
-   */
-  private static final Logger logger = LoggerFactory.getLogger(ProcessUtils.class);
-
-  /**
-   * Initialization regularization, solve the problem of pre-compilation performance,
-   * avoid the thread safety problem of multi-thread operation.
-   */
-  private static final Pattern MACPATTERN = Pattern.compile("-[+|-]-\\s(\\d+)");
-
-  private static final Pattern WINDOWSATTERN = Pattern.compile("(\\d+)");
-
-  /**
-   * build command line characters.
-   * @param commandList command list
-   * @return command
-   */
-  public static String buildCommandStr(List<String> commandList) {
-    String cmdstr;
-    String[] cmd = commandList.toArray(new String[commandList.size()]);
-    SecurityManager security = System.getSecurityManager();
-    boolean allowAmbiguousCommands = false;
-    if (security == null) {
-      allowAmbiguousCommands = true;
-      String value = System.getProperty("jdk.lang.Process.allowAmbiguousCommands");
-      if (value != null) {
-        allowAmbiguousCommands = !"false".equalsIgnoreCase(value);
-      }
+    /**
+     * logger.
+     */
+    private static final Logger logger = LoggerFactory.getLogger(ProcessUtils.class);
+
+    /**
+     * Initialization regularization, solve the problem of pre-compilation performance,
+     * avoid the thread safety problem of multi-thread operation.
+     */
+    private static final Pattern MACPATTERN = Pattern.compile("-[+|-]-\\s(\\d+)");
+
+    private static final Pattern WINDOWSATTERN = Pattern.compile("(\\d+)");
+
+    /**
+     * build command line characters.
+     *
+     * @param commandList command list
+     * @return command
+     */
+    public static String buildCommandStr(List<String> commandList) {
+        String cmdstr;
+        String[] cmd = commandList.toArray(new String[0]);
+        SecurityManager security = System.getSecurityManager();
+        boolean allowAmbiguousCommands = isAllowAmbiguousCommands(security);
+        if (allowAmbiguousCommands) {
+
+            String executablePath = new File(cmd[0]).getPath();
+
+            if (needsEscaping(VERIFICATION_LEGACY, executablePath)) {
+                executablePath = quoteString(executablePath);
+            }
+
+            cmdstr = createCommandLine(
+                VERIFICATION_LEGACY, executablePath, cmd);
+        } else {
+            String executablePath;
+            try {
+                executablePath = getExecutablePath(cmd[0]);
+            } catch (IllegalArgumentException e) {
+
+                StringBuilder join = new StringBuilder();
+                for (String s : cmd) {
+                    join.append(s).append(' ');
+                }
+
+                cmd = getTokensFromCommand(join.toString());
+                executablePath = getExecutablePath(cmd[0]);
+
+                // Check new executable name once more
+                if (security != null) {
+                    security.checkExec(executablePath);
+                }
+            }
+
+            cmdstr = createCommandLine(
+
+                isShellFile(executablePath) ? VERIFICATION_CMD_BAT : VERIFICATION_WIN32, quoteString(executablePath), cmd);
+        }
+        return cmdstr;
     }
-    if (allowAmbiguousCommands) {
-
-      String executablePath = new File(cmd[0]).getPath();
-
-      if (needsEscaping(VERIFICATION_LEGACY, executablePath)) {
-        executablePath = quoteString(executablePath);
-      }
-
-      cmdstr = createCommandLine(
-              VERIFICATION_LEGACY, executablePath, cmd);
-    } else {
-      String executablePath;
-      try {
-        executablePath = getExecutablePath(cmd[0]);
-      } catch (IllegalArgumentException e) {
 
-        StringBuilder join = new StringBuilder();
-        for (String s : cmd) {
-          join.append(s).append(' ');
+    /**
+     * check is allow ambiguous commands
+     *
+     * @param security security manager
+     * @return allow ambiguous command flag
+     */
+    private static boolean isAllowAmbiguousCommands(SecurityManager security) {
+        boolean allowAmbiguousCommands = false;
+        if (security == null) {
+            allowAmbiguousCommands = true;
+            String value = System.getProperty("jdk.lang.Process.allowAmbiguousCommands");
+            if (value != null) {
+                allowAmbiguousCommands = !"false".equalsIgnoreCase(value);
+            }
         }
+        return allowAmbiguousCommands;
+    }
 
-        cmd = getTokensFromCommand(join.toString());
-        executablePath = getExecutablePath(cmd[0]);
+    /**
+     * get executable path.
+     *
+     * @param path path
+     * @return executable path
+     */
+    private static String getExecutablePath(String path) {
+        boolean pathIsQuoted = isQuoted(true, path, "Executable name has embedded quote, split the arguments");

Review comment:
       This string is error message




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org