From c0cc34d7ec3f31a862ba85ff3345619f3f40b40f Mon Sep 17 00:00:00 2001 From: Laszlo Bodor Date: Fri, 18 Sep 2026 13:05:27 +0200 Subject: [PATCH] TEZ-4755: AMWebController: rendering improvements in StaticAMView Encode the configured history-url value when splicing it into the StaticAMView redirect page so unexpected characters cannot alter the surrounding HTML or JavaScript context. Adds a unit test. Co-Authored-By: Claude Code --- .../tez/dag/app/web/AMWebController.java | 60 ++++++++++++++++++- .../tez/dag/app/web/TestAMWebController.java | 38 ++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java b/tez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java index e547dad520..b1a028fa07 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java @@ -921,17 +921,71 @@ private void render(PrintWriter pw) { "

To enable tracking url pointing to Tez UI, set the config " + TezConfiguration.TEZ_HISTORY_URL_BASE + " in the tez-site.xml.

"); } 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("

Redirecting to Tez UI

.

If you are not redirected shortly, click " + - "here

" + "here

" ); pw.write(""); + "window.location.replace('" + escapeJsString(historyUrl) + "');" + + "}, 0); "); } pw.write(""); pw.write(""); pw.flush(); } + + static String escapeHtmlAttribute(String s) { + StringBuilder sb = new StringBuilder(s.length() + 16); + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + switch (c) { + case '&': sb.append("&"); break; + case '<': sb.append("<"); break; + case '>': sb.append(">"); break; + case '"': sb.append("""); break; + case '\'': sb.append("'"); break; + default: sb.append(c); + } + } + return sb.toString(); + } + + static String escapeJsString(String s) { + StringBuilder sb = new StringBuilder(s.length() + 16); + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + switch (c) { + case '\\': sb.append("\\\\"); break; + case '\'': sb.append("\\'"); break; + case '"': sb.append("\\\""); break; + case '\n': sb.append("\\n"); break; + case '\r': sb.append("\\r"); break; + case '\t': sb.append("\\t"); break; + case '\b': sb.append("\\b"); break; + case '\f': sb.append("\\f"); break; + case '<': sb.append("\\u003c"); break; + case '>': sb.append("\\u003e"); break; + case '&': sb.append("\\u0026"); break; + case '/': sb.append("\\/"); break; + // JS line terminators: illegal raw inside a string literal pre-ES2019. + // Written numerically because a \\u2028 source escape would be decoded + // by javac's lexer into a real line break and split this file. + case 0x2028: sb.append("\\u2028"); break; + case 0x2029: sb.append("\\u2029"); break; + default: + if (c < 0x20) { + sb.append(String.format("\\u%04x", (int) c)); + } else { + sb.append(c); + } + } + } + return sb.toString(); + } } @VisibleForTesting diff --git a/tez-dag/src/test/java/org/apache/tez/dag/app/web/TestAMWebController.java b/tez-dag/src/test/java/org/apache/tez/dag/app/web/TestAMWebController.java index e5a8c0e0a7..f6cf853315 100644 --- a/tez-dag/src/test/java/org/apache/tez/dag/app/web/TestAMWebController.java +++ b/tez-dag/src/test/java/org/apache/tez/dag/app/web/TestAMWebController.java @@ -890,4 +890,42 @@ private void verifySingleAttemptResult(TaskAttempt mockTask, Map assertEquals(Float.toString(mockTask.getProgress()), taskResult.get("progress")); } + @Test + public void testStaticAMViewEscapesHistoryUrl() { + // Angle brackets and quotes in the submitter-controlled history url + // must not appear literally in the rendered attribute or JS literal. + String malicious = "http://ui/'\">"; + String htmlEscaped = AMWebController.StaticAMView.escapeHtmlAttribute(malicious); + assertFalse(htmlEscaped.contains(" that would end the enclosing script element + // must be neutralised too. + assertFalse(jsEscaped.contains(""), + "Closing must be neutralised: " + jsEscaped); + } + + @Test + public void testEscapeJsStringEscapesLineTerminators() { + // 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 escaped = AMWebController.StaticAMView.escapeJsString("http://ui/" + ls + ps + "x"); + assertFalse(escaped.contains(ls), "U+2028 must be escaped: " + escaped); + assertFalse(escaped.contains(ps), "U+2029 must be escaped: " + escaped); + assertTrue(escaped.contains("\\u2028"), "Expected \\u2028 escape: " + escaped); + assertTrue(escaped.contains("\\u2029"), "Expected \\u2029 escape: " + escaped); + } + }