TEZ-4755: AMWebController: rendering improvements in StaticAMView

pom.xml

@@ -74,6 +74,7 @@
     <commons-collections4.version>4.4</commons-collections4.version>
     <commons-io.version>2.22.0</commons-io.version>
     <commons-lang3.version>3.19.0</commons-lang3.version>
+    <commons-text.version>1.15.0</commons-text.version>
     <curator.version>5.9.0</curator.version>
     <dependency-check-maven.version>3.2.0</dependency-check-maven.version>
     <spotbugs.version>4.9.3</spotbugs.version>
@@ -328,6 +329,11 @@
         <artifactId>commons-lang3</artifactId>
         <version>${commons-lang3.version}</version>
       </dependency>
+      <dependency>
+        <groupId>org.apache.commons</groupId>
+        <artifactId>commons-text</artifactId>
+        <version>${commons-text.version}</version>
+      </dependency>
       <dependency>
         <groupId>io.netty</groupId>
         <artifactId>netty-all</artifactId>

tez-dag/pom.xml

@@ -111,6 +111,10 @@
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-text</artifactId>
+    </dependency>
     <dependency>
       <groupId>commons-codec</groupId>
       <artifactId>commons-codec</artifactId>

tez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java

@@ -36,6 +36,7 @@
 
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.commons.text.StringEscapeUtils;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.yarn.webapp.Controller;
 import org.apache.hadoop.yarn.webapp.MimeType;
@@ -909,7 +910,8 @@ public void render() {
       }
     }
 
-    private void render(PrintWriter pw) {
+    @VisibleForTesting
+    void render(PrintWriter pw) {
       pw.write("<html>");
       pw.write("<head>");
       pw.write("<meta charset=\"utf-8\">");
@@ -921,12 +923,17 @@ private void render(PrintWriter pw) {
                 "<p>To enable tracking url pointing to Tez UI, set the config <b>" +
                 TezConfiguration.TEZ_HISTORY_URL_BASE + "</b> in the tez-site.xml.</p>");
       } else {
+        // historyUrl is derived from a submitter-supplied AM configuration
+        // property (tez.tez-ui.history-url.base). Escape it before splicing
+        // into the HTML attribute and the inline JS string literal so a
+        // value like ' or " cannot break out and run script in the browser
+        // of whoever opens the AM tracking URL.
         pw.write("<h1>Redirecting to Tez UI</h1>. <p>If you are not redirected shortly, click " +
-                "<a href='" + historyUrl + "'><b>here</b></a></p>"
+                "<a href=\"" + StringEscapeUtils.escapeHtml4(historyUrl) + "\"><b>here</b></a></p>"
         );
         pw.write("<script type='text/javascript'>setTimeout(function() { " +
-          "window.location.replace('" + historyUrl + "');" +
-          "}, 0); </script>");
+            "window.location.replace('" + StringEscapeUtils.escapeEcmaScript(historyUrl) + "');" +
+            "}, 0); </script>");
       }
       pw.write("</body>");
       pw.write("</html>");

tez-dag/src/test/java/org/apache/tez/dag/app/web/TestAMWebController.java

@@ -35,6 +35,8 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -890,4 +892,46 @@ private void verifySingleAttemptResult(TaskAttempt mockTask, Map<String, String>
     assertEquals(Float.toString(mockTask.getProgress()), taskResult.get("progress"));
   }
 
+  private static String renderStaticAMView(String historyUrl) {
+    AMWebController.StaticAMView view = new AMWebController.StaticAMView();
+    view.historyUrl = historyUrl;
+    StringWriter out = new StringWriter();
+    view.render(new PrintWriter(out));
+    return out.toString();
+  }
+
+  @Test
+  public void testStaticAMViewEscapesHistoryUrl() {
+    // Angle brackets and quotes in the submitter-controlled history url
+    // must not break out of the rendered attribute or JS literal.
+    String rendered = renderStaticAMView("http://ui/'\"><script>alert(1)</script>");
+
+    // Double-quoted href: " < > & are entity-encoded, a bare ' is harmless.
+    assertTrue(rendered.contains(
+        "<a href=\"http://ui/'&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;\">"),
+        "Unexpected href encoding: " + rendered);
+    // Single-quoted JS literal: ' and " are backslash-escaped and / becomes \/,
+    // so the injected </script> cannot close the enclosing script element.
+    assertTrue(rendered.contains(
+        "window.location.replace('http:\\/\\/ui\\/\\'\\\"><script>alert(1)<\\/script>');"),
+        "Unexpected JS encoding: " + rendered);
+    assertEquals(1, rendered.split("</script>", -1).length - 1,
+        "Only the page's own </script> may appear: " + rendered);
+  }
+
+  @Test
+  public void testStaticAMViewEscapesJsLineTerminators() {
+    // U+2028/U+2029 are JS line terminators: raw, they break the string
+    // literal pre-ES2019 and the redirect never runs. Built numerically —
+    // a source \\u2028 escape is decoded by javac before string parsing.
+    String ls = String.valueOf((char) 0x2028);
+    String ps = String.valueOf((char) 0x2029);
+    String rendered = renderStaticAMView("http://ui/" + ls + ps + "x");
+    String script = rendered.substring(rendered.indexOf("<script"));
+    assertFalse(script.contains(ls), "U+2028 must be escaped: " + script);
+    assertFalse(script.contains(ps), "U+2029 must be escaped: " + script);
+    assertTrue(script.contains("http:\\/\\/ui\\/\\u2028\\u2029x"),
+        "Expected \\u2028\\u2029 escapes: " + script);
+  }
+
 }