This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new e79fd9b228dc [MINOR][UI][4.2] Encode query parameters when generating
log page scripts
e79fd9b228dc is described below
commit e79fd9b228dcc369fb7494796c9abc17f2ae1c2f
Author: Claude <Holden's Claude Robot [email protected]>
AuthorDate: Fri Jul 10 12:59:53 2026 -0700
[MINOR][UI][4.2] Encode query parameters when generating log page scripts
### What changes were proposed in this pull request?
The log viewer pages assemble a query string from request parameters and
embed it into an inline script literal. Encode that value for a JavaScript
string context using the bundled commons-text StringEscapeUtils, matching the
existing pattern in AllJobsPage, so the generated script stays well-formed
regardless of the parameter contents. Also sanitize the parameters on the
client side in log-view.js, and add a regression test.
Backport to 4.2 of https://github.com/apache/spark/pull/57165
### Why are the changes needed?
Formatting can get funky with unescaped parameters.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
New test
### Was this patch authored or co-authored using generative AI tooling?
Primarily Claude 4.8
Closes #57166 from holdenk/log-page-encode-4.x.
Lead-authored-by: Claude <Holden's Claude Robot
holden.karau+claudegmail.com>
(cherry picked from commit 2f75218fb6ad63c40bc48057f2a03ff39e6f1f5f)
Closes #57188 from huaxingao/log-page-encode-4.2.
Authored-by: Claude <Holden's Claude Robot [email protected]>
Signed-off-by: Huaxin Gao <[email protected]>
---
.../org/apache/spark/deploy/history/LogPage.scala | 6 ++--
.../apache/spark/deploy/master/ui/LogPage.scala | 6 ++--
.../apache/spark/deploy/worker/ui/LogPage.scala | 15 +++++---
.../scala/org/apache/spark/ui/DriverLogPage.scala | 6 ++--
.../main/scala/org/apache/spark/ui/UIUtils.scala | 30 +++++++++++++++-
.../apache/spark/deploy/history/LogPageSuite.scala | 42 ++++++++++++++++++++++
.../spark/deploy/worker/ui/LogPageSuite.scala | 28 +++++++++++++++
7 files changed, 118 insertions(+), 15 deletions(-)
diff --git a/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala
b/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala
index 1b809b80ee32..9b14e76db37a 100644
--- a/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala
@@ -56,9 +56,9 @@ private[history] class LogPage(conf: SparkConf) extends
WebUIPage("logPage") wit
End of Log
</div>
- val logParams = "?self&logType=%s".format(logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte,
$logLength, $byteLength);"
+ val logParams = "?self&logType=%s".format(UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
<script type="module" src={UIUtils.prependBaseUri(request,
"/static/utils.js")}></script> ++
diff --git
a/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala
b/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala
index 780f8a2b6c53..7da873622202 100644
--- a/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala
@@ -55,9 +55,9 @@ private[ui] class LogPage(parent: MasterWebUI) extends
WebUIPage("logPage") with
End of Log
</div>
- val logParams = "?self&logType=%s".format(logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte,
$logLength, $byteLength);"
+ val logParams = "?self&logType=%s".format(UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
<script type="module" src={UIUtils.prependBaseUri(request,
"/static/utils.js")}></script> ++
diff --git
a/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
b/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
index adfe9670f371..4c3c27c0d9f9 100644
--- a/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
@@ -71,11 +71,16 @@ private[ui] class LogPage(parent: WorkerWebUI) extends
WebUIPage("logPage") with
val byteLength = Option(request.getParameter("byteLength")).map(_.toInt)
.getOrElse(defaultBytes)
+ // The identifiers below flow both into a filesystem path (logDir, kept
raw and guarded by
+ // Utils.isInDirectory in getLog) and into the query string of the
generated /log requests
+ // (params). URL-encode the query-string copy so request-supplied values
cannot inject extra
+ // parameters or truncate the request.
val (logDir, params, pageName) = (appId, executorId, driverId, self) match
{
case (Some(a), Some(e), None, None) =>
- (s"${workDir.getPath}/$a/$e/", s"appId=$a&executorId=$e", s"$a/$e")
+ (s"${workDir.getPath}/$a/$e/",
+
s"appId=${UIUtils.encodeLogParam(a)}&executorId=${UIUtils.encodeLogParam(e)}",
s"$a/$e")
case (None, None, Some(d), None) =>
- (s"${workDir.getPath}/$d/", s"driverId=$d", d)
+ (s"${workDir.getPath}/$d/", s"driverId=${UIUtils.encodeLogParam(d)}",
d)
case (None, None, None, Some(_)) =>
(s"${sys.env.getOrElse("SPARK_LOG_DIR", workDir.getPath)}/", "self",
"worker")
case _ =>
@@ -105,9 +110,9 @@ private[ui] class LogPage(parent: WorkerWebUI) extends
WebUIPage("logPage") with
End of Log
</div>
- val logParams = "?%s&logType=%s".format(params, logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte,
$logLength, $byteLength);"
+ val logParams = "?%s&logType=%s".format(params,
UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
<script type="module" src={UIUtils.prependBaseUri(request,
"/static/utils.js")}></script> ++
diff --git a/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala
b/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala
index 7b427cffcc5e..9cd2fe50715f 100644
--- a/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala
+++ b/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala
@@ -70,9 +70,9 @@ private[ui] class DriverLogPage(
End of Log
</div>
- val logParams = "/?logType=%s".format(logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte,
$logLength, $byteLength);"
+ val logParams = "/?logType=%s".format(UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
<script type="module" src={UIUtils.prependBaseUri(request,
"/static/utils.js")}></script> ++
diff --git a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
index eedec979f715..c0d36ef3a8ac 100644
--- a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
+++ b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
@@ -19,7 +19,7 @@ package org.apache.spark.ui
import java.{util => ju}
import java.lang.{Long => JLong}
-import java.net.URLDecoder
+import java.net.{URLDecoder, URLEncoder}
import java.nio.charset.StandardCharsets.UTF_8
import java.time.{Instant, ZoneId}
import java.time.format.DateTimeFormatter
@@ -32,6 +32,7 @@ import scala.xml.transform.{RewriteRule, RuleTransformer}
import jakarta.servlet.http.HttpServletRequest
import jakarta.ws.rs.core.{MediaType, MultivaluedMap, Response}
+import org.apache.commons.text.StringEscapeUtils
import org.eclipse.jetty.server.Request
import org.glassfish.jersey.internal.util.collection.MultivaluedStringMap
@@ -751,6 +752,33 @@ private[spark] object UIUtils extends Logging {
def getTimeZoneOffset() : Int =
TimeZone.getDefault().getOffset(System.currentTimeMillis()) / 1000 / 60
+ /**
+ * URL-encode a single request-supplied value before embedding it in a log
page query string.
+ * This keeps characters such as '&', '=', '#' and whitespace from changing
the structure of the
+ * subsequent `/log` requests (parameter injection / query truncation).
+ */
+ def encodeLogParam(value: String): String =
+ URLEncoder.encode(Option(value).getOrElse("null"), UTF_8)
+
+ /**
+ * Build the inline `window.onload = initLogPage(...)` bootstrap script
shared by the log pages
+ * (history / master / worker / driver). Individual parameter values in
`logParams` must already
+ * be URL-encoded via [[encodeLogParam]] so the query string is unambiguous;
the assembled string
+ * is additionally escaped for the surrounding JavaScript string literal so
request-supplied
+ * values cannot break out of the inline <script> block.
+ */
+ def logPageOnloadScript(
+ logParams: String,
+ curLogLength: Long,
+ startByte: Long,
+ endByte: Long,
+ logLength: Long,
+ byteLength: Long): String = {
+ "window.onload = " +
+ s"initLogPage('${StringEscapeUtils.escapeEcmaScript(logParams)}', " +
+ s"$curLogLength, $startByte, $endByte, $logLength, $byteLength);"
+ }
+
/**
* Return the correct Href after checking if master is running in the
* reverse proxy mode or not.
diff --git
a/core/src/test/scala/org/apache/spark/deploy/history/LogPageSuite.scala
b/core/src/test/scala/org/apache/spark/deploy/history/LogPageSuite.scala
new file mode 100644
index 000000000000..f37eac47ccea
--- /dev/null
+++ b/core/src/test/scala/org/apache/spark/deploy/history/LogPageSuite.scala
@@ -0,0 +1,42 @@
+/*
+ * 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.deploy.history
+
+import jakarta.servlet.http.HttpServletRequest
+import org.mockito.Mockito.{mock, when}
+
+import org.apache.spark.{SparkConf, SparkFunSuite}
+
+class LogPageSuite extends SparkFunSuite {
+
+ test("render encodes the logType parameter embedded in the inline script") {
+ val page = new LogPage(new SparkConf(false))
+ val request = mock(classOf[HttpServletRequest])
+ // A value that would break out of the single-quoted JavaScript string
literal, close the
+ // script element, or start a new statement if it were emitted into the
page verbatim.
+
when(request.getParameter("logType")).thenReturn("stdout');alert('xss')</script>")
+ val html = page.render(request).mkString
+
+ // The value is URL-encoded, so the quotes, angle brackets and slash that
would break out of
+ // the inline <script> (or inject extra /log parameters) become inert
percent escapes.
+
assert(html.contains("logType=stdout%27%29%3Balert%28%27xss%27%29%3C%2Fscript%3E"))
+ // The raw, structure-breaking form must never reach the generated query
string. (The page
+ // title reflects logType too, but there the XML library HTML-escapes it,
so it is safe.)
+ assert(!html.contains("logType=stdout');"))
+ }
+}
diff --git
a/core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala
b/core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala
index c8b4e3372386..cd5e475b8771 100644
--- a/core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala
+++ b/core/src/test/scala/org/apache/spark/deploy/worker/ui/LogPageSuite.scala
@@ -19,6 +19,7 @@ package org.apache.spark.deploy.worker.ui
import java.io.{File, FileWriter}
+import jakarta.servlet.http.HttpServletRequest
import org.mockito.Mockito.{mock, when}
import org.scalatest.PrivateMethodTester
@@ -75,6 +76,33 @@ class LogPageSuite extends SparkFunSuite with
PrivateMethodTester {
assert(error4.startsWith("Error: invalid log directory"))
}
+ test("render encodes request parameters embedded in the inline script") {
+ val webui = mock(classOf[WorkerWebUI])
+ val worker = mock(classOf[Worker])
+ withTempDir { tmpDir =>
+ val workDir = new File(tmpDir, "work-dir")
+ workDir.mkdir()
+ when(webui.workDir).thenReturn(workDir)
+ when(webui.worker).thenReturn(worker)
+ when(worker.conf).thenReturn(new SparkConf())
+ when(worker.activeMasterWebUiUrl).thenReturn("http://master:8080";)
+ val logPage = new LogPage(webui)
+
+ val request = mock(classOf[HttpServletRequest])
+ // appId is user-controlled; a '&' in it would inject an extra parameter
into the /log
+ // requests, and the '</script>' would break out of the inline <script>
block, if emitted raw.
+
when(request.getParameter("appId")).thenReturn("app&byteLength=0</script>")
+ when(request.getParameter("executorId")).thenReturn("0")
+ when(request.getParameter("logType")).thenReturn("stdout")
+ val html = logPage.render(request).mkString
+
+ // The value is percent-encoded, so it stays a single opaque appId value
and cannot inject a
+ // parameter or close the script element.
+
assert(html.contains("appId=app%26byteLength%3D0%3C%2Fscript%3E&executorId=0"))
+ assert(!html.contains("app&byteLength=0</script>"))
+ }
+ }
+
/** Write the specified string to the file. */
private def write(f: File, s: String): Unit = {
val writer = new FileWriter(f)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
