Skip to content

YARN: escape the AM-supplied tracking url in the apps table blocks#8617

Open
nishat-06 wants to merge 1 commit into
apache:trunkfrom
nishat-06:escape-apps-table-tracking-url
Open

YARN: escape the AM-supplied tracking url in the apps table blocks#8617
nishat-06 wants to merge 1 commit into
apache:trunkfrom
nishat-06:escape-apps-table-tracking-url

Conversation

@nishat-06

Copy link
Copy Markdown

Description of PR

The apps tables in the RM, ApplicationHistory, FairScheduler and Router web UIs build their rows into a StringBuilder and emit the result inside a <script> element as var appsTableData=.... Every application-supplied column in that array (user, name, type, tags, queue) already goes through escapeEcmaScript(escapeHtml4(...)), but the tracking URL is concatenated straight into href='...', and ApplicationReport.getTrackingUrl() hands back RMAppAttemptImpl.originalTrackingUrl verbatim for an unmanaged AM, so the string a client passes to registerApplicationMaster reaches the page unfiltered. A tracking URL holding a quote or a </script> closes the attribute or the script element and runs in the browser of anyone listing applications, so the escaping the neighbouring columns already use is applied here too. The Router copy has an escape() helper of its own that this one call site was skipping.

How was this patch tested?

New TestAppsBlock#testTrackingUrlIsEscaped renders AppsBlock with a report whose tracking URL carries a </script><script> payload; it fails on trunk and passes with the change. TestAppsBlock, TestRMWebApp, TestRMWebAppFairScheduler, TestAppPage and TestFederationWebApp all pass, and checkstyle reports nothing new on the touched files.

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
  • Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

AI Tooling

If an AI tool was used:

The tracking url is concatenated raw into href='...' inside the appsTableData script block while every neighbouring app-supplied column is escaped.
@hadoop-yetus

Copy link
Copy Markdown

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 22s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+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.
_ trunk Compile Tests _
+0 🆗 mvndep 1m 48s Maven dependency ordering for branch
+1 💚 mvninstall 25m 59s trunk passed
+1 💚 compile 1m 34s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 compile 1m 37s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 checkstyle 1m 12s trunk passed
+1 💚 mvnsite 2m 23s trunk passed
+1 💚 javadoc 2m 11s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 2m 8s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 spotbugs 3m 37s trunk passed
+1 💚 shadedclient 16m 6s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 18s Maven dependency ordering for patch
+1 💚 mvninstall 1m 17s the patch passed
+1 💚 compile 1m 17s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javac 1m 17s the patch passed
+1 💚 compile 1m 19s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 javac 1m 19s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 52s the patch passed
+1 💚 mvnsite 1m 20s the patch passed
+1 💚 javadoc 1m 11s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 1m 6s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 spotbugs 3m 5s the patch passed
+1 💚 shadedclient 15m 46s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 3m 9s hadoop-yarn-server-common in the patch passed.
+1 💚 unit 91m 30s hadoop-yarn-server-resourcemanager in the patch passed.
-1 ❌ unit 7m 41s /patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-router.txt hadoop-yarn-server-router in the patch passed.
+1 💚 asflicense 0m 23s The patch does not generate ASF License warnings.
191m 42s
Reason Tests
Failed junit tests hadoop.yarn.server.router.webapp.TestRouterWebServicesREST
Subsystem Report/Notes
Docker ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8617/1/artifact/out/Dockerfile
GITHUB PR #8617
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux 53b4a3526f04 5.15.0-179-generic #189-Ubuntu SMP Tue May 5 18:20:56 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 5523d1d
Default Java Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
Multi-JDK versions /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8617/1/testReport/
Max. process+thread count 1296 (vs. ulimit of 10000)
modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8617/1/console
versions git=2.43.0 maven=3.9.15 spotbugs=4.9.7
Powered by Apache Yetus 0.14.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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants