TEZ-4754: RelocalizationUtils: input handling improvements

tez-common/src/main/java/org/apache/tez/dag/utils/RelocalizationUtils.java

@@ -21,6 +21,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.net.URL;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -58,13 +59,43 @@ public static void addUrlsToClassPath(List<URL> urls) {
     ReflectionUtils.addResourcesToSystemClassLoader(urls);
   }
 
+  /** Validate all names up front, so callers can reject before mutating state. */
+  public static void validateDestNames(Collection<String> destNames) {
+    if (destNames == null) {
+      return;
+    }
+    for (String destName : destNames) {
+      validateDestName(destName);
+    }
+  }
+
   private static Path downloadResource(String destName, URI uri, Configuration conf, String destDir)
       throws IOException {
+    validateDestName(destName);
     FileSystem fs = FileSystem.get(uri, conf);
     Path cwd = new Path(destDir);
     Path dFile = new Path(cwd, destName);
     Path srcPath = new Path(uri);
     fs.copyToLocalFile(srcPath, dFile);
     return dFile.makeQualified(FileSystem.getLocal(conf).getUri(), cwd);
   }
+
+  static void validateDestName(String destName) {
+    if (destName == null || destName.isEmpty()) {
+      throw new IllegalArgumentException("Resource name must not be empty");
+    }
+    if (destName.indexOf('/') >= 0 || destName.indexOf('\\') >= 0
+        || destName.indexOf('\0') >= 0) {
+      throw new IllegalArgumentException(
+          "Resource name must not contain path separators: " + destName);
+    }
+    if (destName.equals(".") || destName.equals("..")) {
+      throw new IllegalArgumentException(
+          "Resource name must not be a parent-directory reference: " + destName);
+    }
+    if (new Path(destName).isAbsolute()) {
+      throw new IllegalArgumentException(
+          "Resource name must not be absolute: " + destName);
+    }
+  }
 }

tez-common/src/test/java/org/apache/tez/dag/utils/TestRelocalizationUtils.java

@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tez.dag.utils;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import org.apache.hadoop.fs.Path;
+
+import org.junit.jupiter.api.Test;
+
+public class TestRelocalizationUtils {
+
+  @Test
+  public void plainFileNameIsAccepted() {
+    assertDoesNotThrow(() -> RelocalizationUtils.validateDestName("lib.jar"));
+    assertDoesNotThrow(() -> RelocalizationUtils.validateDestName("some-name_1.tar.gz"));
+  }
+
+  @Test
+  public void traversalIsRejected() {
+    // Any path separator or parent-directory reference must be refused: those
+    // are the shapes that let a submitter's key redirect the AM download to
+    // a location outside its working directory.
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName("../evil.jar"));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName("dir/child.jar"));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName("dir\\child.jar"));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName("/absolute/path.jar"));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName("."));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName(".."));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName(""));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestName(null));
+  }
+
+  @Test
+  public void validateDestNamesRejectsAnyBadNameInBatch() {
+    // One bad name must fail the whole batch, before any download starts.
+    assertDoesNotThrow(() -> RelocalizationUtils.validateDestNames(
+        Arrays.asList("a.jar", "b.jar")));
+    assertDoesNotThrow(() -> RelocalizationUtils.validateDestNames(null));
+    assertDoesNotThrow(
+        () -> RelocalizationUtils.validateDestNames(Collections.emptyList()));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestNames(
+            Arrays.asList("ok.jar", "../evil.jar")));
+    assertThrows(IllegalArgumentException.class,
+        () -> RelocalizationUtils.validateDestNames(
+            Arrays.asList("a.jar", "/absolute.jar", "b.jar")));
+  }
+
+  /**
+   * A URI-scheme opaque form such as {@code "file:.."} is rejected today: the
+   * {@code new Path(destName)} call inside {@link RelocalizationUtils#validateDestName}
+   * throws {@link IllegalArgumentException} from Hadoop's URI parser
+   * ("Relative path in absolute URI"). And {@code new Path(cwd, "file:..")}
+   * — the very next construction in {@code downloadResource} — throws
+   * identically, so the value cannot reach {@code copyToLocalFile} and
+   * cannot resolve outside {@code cwd}.
+   */
+  @Test
+  public void uriSchemeOpaqueFormIsRejected() {
+    for (String s : new String[]{"file:..", "file:../evil.jar", "file:evil.jar",
+        "mailto:x", "C:evil.jar"}) {
+      assertThrows(IllegalArgumentException.class,
+          () -> RelocalizationUtils.validateDestName(s),
+          "validateDestName should reject " + s);
+      assertThrows(IllegalArgumentException.class,
+          () -> new Path(new Path("/tmp/work"), s),
+          "new Path(cwd, s) should also throw for " + s);
+    }
+  }
+}

tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java

@@ -1348,6 +1348,15 @@ public Void run() throws Exception {
 
   public String submitDAGToAppMaster(DAGPlan dagPlan,
       Map<String, LocalResource> additionalResources) throws TezException {
+    // Validate before startDAGExecution mutates currentDAG and amResources:
+    // a later failure would leave the session stuck "already running a DAG".
+    if (additionalResources != null) {
+      try {
+        RelocalizationUtils.validateDestNames(additionalResources.keySet());
+      } catch (IllegalArgumentException e) {
+        throw new TezException(e);
+      }
+    }
     appMasterReadinessService.waitToBeReady();
 
     if (sessionStopped.get()) {