Copilot commented on code in PR #538:
URL: https://github.com/apache/tez/pull/538#discussion_r4046779722


##########
tez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java:
##########
@@ -921,17 +921,66 @@ 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=\"" + escapeHtmlAttribute(historyUrl) + 
"\"><b>here</b></a></p>"
         );
         pw.write("<script type='text/javascript'>setTimeout(function() { " +
-          "window.location.replace('" + historyUrl + "');" +
+          "window.location.replace('" + escapeJsString(historyUrl) + "');" +
           "}, 0); </script>");
       }
       pw.write("</body>");
       pw.write("</html>");
       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("&amp;"); break;
+          case '<': sb.append("&lt;"); break;
+          case '>': sb.append("&gt;"); break;
+          case '"': sb.append("&quot;"); break;
+          case '\'': sb.append("&#39;"); 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;
+          default:
+            if (c < 0x20) {
+              sb.append(String.format("\\u%04x", (int) c));
+            } else {
+              sb.append(c);
+            }

Review Comment:
   This JS encoder leaves U+2028 and U+2029 unchanged. Those characters are 
JavaScript line terminators, so a history URL containing either one makes the 
inline `window.location.replace(...)` script a syntax error and prevents the 
redirect. Encode both characters as `\\u2028`/`\\u2029` (or otherwise use a 
serializer that produces a valid JS string literal).



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to