Skip to content

TEZ-4755: AMWebController: rendering improvements in StaticAMView - #538

Open
abstractdog wants to merge 1 commit into
apache:masterfrom
abstractdog:TEZ-4755
Open

abstractdog wants to merge 1 commit into
apache:masterfrom
abstractdog:TEZ-4755

Conversation

@abstractdog

Copy link
Copy Markdown
Contributor

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.

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 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

U+2028 and U+2029 can still break the inline JavaScript redirect.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates StaticAMView to safely encode configured history URLs in HTML and JavaScript contexts.

Changes:

  • Adds context-specific escaping helpers.
  • Applies escaping to rendered redirects.
  • Adds regression tests for injection characters.
File summaries
File Summary
tez-dag/src/test/java/org/apache/tez/dag/app/web/TestAMWebController.java Adds escaping regression coverage.
tez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java Escapes history URLs before rendering; JavaScript line terminators remain unhandled.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +974 to +979
default:
if (c < 0x20) {
sb.append(String.format("\\u%04x", (int) c));
} else {
sb.append(c);
}
@tez-yetus

Copy link
Copy Markdown

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 6m 16s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 1 new or modified test files.
_ master Compile Tests _
+1 💚 mvninstall 4m 49s master passed
+1 💚 compile 4m 2s master passed
+1 💚 checkstyle 0m 46s master passed
+1 💚 javadoc 0m 31s master passed
+0 🆗 spotbugs 1m 43s tez-dag in master has 537 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚 mvninstall 3m 40s the patch passed
+1 💚 codespell 1m 34s No new issues.
+1 💚 compile 4m 2s the patch passed
+1 💚 javac 4m 2s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 0m 38s /results-checkstyle-tez-dag.txt tez-dag: The patch generated 1 new + 26 unchanged - 1 fixed = 27 total (was 27)
+1 💚 javadoc 0m 30s the patch passed
+1 💚 spotbugs 1m 55s the patch passed
_ Other Tests _
+1 💚 unit 76m 45s root in the patch passed.
+1 💚 asflicense 0m 33s The patch does not generate ASF License warnings.
109m 23s
Subsystem Report/Notes
Docker ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-538/1/artifact/out/Dockerfile
Optional Tests dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
uname Linux 13d3c75da8b6 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality tez-personality.sh
git revision master / 0e9f756
Default Java Eclipse Adoptium-21.0.12+8-LTS
Test Results https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-538/1/testReport/
Max. process+thread count 2125 (vs. ulimit of 5500)
modules C: tez-dag U: tez-dag
Console output https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-538/1/console
versions git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered by Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants