This is an automated email from the ASF dual-hosted git repository.
dongjoon-hyun pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new 950461d8a258 [SPARK-58156][CORE][UI] Escape the executor removal
reason in the timeline tooltip
950461d8a258 is described below
commit 950461d8a258bdf5635b4a55d7e43941478d758b
Author: Dongjoon Hyun <[email protected]>
AuthorDate: Wed Jul 15 15:30:14 2026 -0700
[SPARK-58156][CORE][UI] Escape the executor removal reason in the timeline
tooltip
### What changes were proposed in this pull request?
This PR HTML-escapes the executor removal reason before embedding it into
the executor event timeline tooltip, in both `AllJobsPage` and `JobPage`.
`Utility.escape` is applied twice, matching the adjacent job description and
stage name handling (`jsEscapedDescForTooltip` / `jsEscapedNameForTooltip`):
the value is parsed once as the `data-bs-title` attribute value, and once more
when Bootstrap assigns it as the tooltip `innerHTML`.
https://github.com/apache/spark/blob/a4dc5d9499fa8d40b02a42d5d54dfbc1a1986f37/core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala#L80-L81
This is similar to the following.
- #28317
### Why are the changes needed?
`removeReason` was the only value in the tooltip escaped with
`StringEscapeUtils.escapeEcmaScript` alone, without HTML escaping. That is a
JavaScript string literal escape, so `\"` evaluates back to `"` at runtime.
Since the timeline library treats the content as HTML, a reason like `"><img
src=x onerror=alert(1)>` closes the `data-bs-title` attribute and injects a tag.
The reason is not a fixed literal: it carries executor-side exception
messages, YARN container diagnostics, and Kubernetes pod status messages
verbatim.
### Does this PR introduce _any_ user-facing change?
No behavior change. Only a UI hardening.
### How was this patch tested?
Pass the CIs with the newly added test suite.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Fable 5
Closes #57288 from dongjoon-hyun/SPARK-58156.
Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 6b24038183f42d769d6ac88aebcea704de7ef9e3)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../org/apache/spark/ui/jobs/AllJobsPage.scala | 4 +-
.../scala/org/apache/spark/ui/jobs/JobPage.scala | 2 +-
.../spark/ui/jobs/ExecutorEventEscapingSuite.scala | 76 ++++++++++++++++++++++
3 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
b/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
index 75115ed4366e..64e17a18e53b 100644
--- a/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
+++ b/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
@@ -123,7 +123,7 @@ private[ui] class AllJobsPage(parent: JobsTab, store:
AppStatusStore) extends We
}
}
- private def makeExecutorEvent(executors: Seq[v1.ExecutorSummary]):
+ def makeExecutorEvent(executors: Seq[v1.ExecutorSummary]):
Seq[String] = {
val events = ListBuffer[String]()
executors.sortBy { e =>
@@ -158,7 +158,7 @@ private[ui] class AllJobsPage(parent: JobsTab, store:
AppStatusStore) extends We
| '${
e.removeReason.map { reason =>
s"""<br>Reason: ${StringEscapeUtils.escapeEcmaScript(
- reason.replace("\n", " "))}"""
+ Utility.escape(Utility.escape(reason.replace("\n", "
"))))}"""
}.getOrElse("")
}"' +
| 'data-bs-html="true">Executor ${e.id} removed</div>'
diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala
b/core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala
index a5c8aaab9ec0..da82afe34aed 100644
--- a/core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala
+++ b/core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala
@@ -138,7 +138,7 @@ private[ui] class JobPage(parent: JobsTab, store:
AppStatusStore) extends WebUIP
| '${
e.removeReason.map { reason =>
s"""<br>Reason: ${StringEscapeUtils.escapeEcmaScript(
- reason.replace("\n", " "))}"""
+ Utility.escape(Utility.escape(reason.replace("\n", "
"))))}"""
}.getOrElse("")
}"' +
| 'data-bs-html="true">Executor ${e.id} removed</div>'
diff --git
a/core/src/test/scala/org/apache/spark/ui/jobs/ExecutorEventEscapingSuite.scala
b/core/src/test/scala/org/apache/spark/ui/jobs/ExecutorEventEscapingSuite.scala
new file mode 100644
index 000000000000..50ab1a1550c3
--- /dev/null
+++
b/core/src/test/scala/org/apache/spark/ui/jobs/ExecutorEventEscapingSuite.scala
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.ui.jobs
+
+import java.util.Date
+
+import org.mockito.Mockito.when
+import org.scalatestplus.mockito.MockitoSugar
+
+import org.apache.spark.{SparkConf, SparkFunSuite}
+import org.apache.spark.status.AppStatusStore
+import org.apache.spark.status.api.v1.ExecutorSummary
+
+/**
+ * The executor removal reason originates outside the driver: it can carry an
executor-side
+ * exception message, YARN container diagnostics, or a Kubernetes pod status
message. The timeline
+ * library treats the generated content as HTML, so the reason must not be
able to break out of
+ * the `data-bs-title` attribute it is rendered into.
+ */
+class ExecutorEventEscapingSuite extends SparkFunSuite with MockitoSugar {
+
+ // Closes the data-bs-title attribute and injects a tag if the reason is not
HTML escaped.
+ private val payload = "\"><img src=x onerror=alert(1)>"
+
+ private def executorWithRemoveReason(reason: String): ExecutorSummary = {
+ new ExecutorSummary("1", "host:port", false, 1,
+ 10, 10, 1, 1, 1,
+ 0, 0, 1, 100,
+ 1, 100, 100,
+ 10, false, 20, new Date(1600984336352L),
+ Some(new Date(1600984336353L)), Some(reason), Map(), Option.empty,
Set(), Option.empty,
+ Map(), Map(), 1, false, Set())
+ }
+
+ private def newJobsTab(): JobsTab = {
+ val tab = mock[JobsTab]
+ when(tab.conf).thenReturn(new SparkConf())
+ tab
+ }
+
+ private def checkEscaped(event: String): Unit = {
+ // The payload must not survive as markup that the browser would parse as
a tag.
+ assert(!event.contains("<img"), s"Reason was not HTML escaped: $event")
+ // Two layers of escaping, matching the adjacent job/stage name handling:
the reason is parsed
+ // once as an attribute value and once more when Bootstrap assigns it as
tooltip innerHTML.
+ assert(event.contains("&lt;img src=x onerror=alert(1)&gt;"),
+ s"Reason was not escaped twice: $event")
+ }
+
+ test("AllJobsPage escapes the executor removal reason in the timeline
tooltip") {
+ val page = new AllJobsPage(newJobsTab(), mock[AppStatusStore])
+ val events = page.makeExecutorEvent(Seq(executorWithRemoveReason(payload)))
+ checkEscaped(events.filter(_.contains("Reason:")).mkString)
+ }
+
+ test("JobPage escapes the executor removal reason in the timeline tooltip") {
+ val page = new JobPage(newJobsTab(), mock[AppStatusStore])
+ val events = page.makeExecutorEvent(Seq(executorWithRemoveReason(payload)))
+ checkEscaped(events.filter(_.contains("Reason:")).mkString)
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
