Skip to content

Commit

Permalink
[SPARK-15209] Fix display of job descriptions with single quotes in w…
Browse files Browse the repository at this point in the history
…eb UI timeline

## What changes were proposed in this pull request?

This patch fixes an escaping bug in the Web UI's event timeline that caused Javascript errors when displaying timeline entries whose descriptions include single quotes.

The original bug can be reproduced by running

```scala
sc.setJobDescription("double quote: \" ")
sc.parallelize(1 to 10).count()

sc.setJobDescription("single quote: ' ")
sc.parallelize(1 to 10).count()
```

and then browsing to the driver UI. Previously, this resulted in an "Uncaught SyntaxError" because the single quote from the description was not escaped and ended up closing a Javascript string literal too early.

The fix implemented here is to change the relevant Javascript to define its string literals using double-quotes. Our escaping logic already properly escapes double quotes in the description, so this is safe to do.

## How was this patch tested?

Tested manually in `spark-shell` using the following cases:

```scala
sc.setJobDescription("double quote: \" ")
sc.parallelize(1 to 10).count()

sc.setJobDescription("single quote: ' ")
sc.parallelize(1 to 10).count()

sc.setJobDescription("ampersand: &")
sc.parallelize(1 to 10).count()

sc.setJobDescription("newline: \n text after newline ")
sc.parallelize(1 to 10).count()

sc.setJobDescription("carriage return: \r text after return ")
sc.parallelize(1 to 10).count()
```

/cc sarutak for review.

Author: Josh Rosen <joshrosen@databricks.com>

Closes #12995 from JoshRosen/SPARK-15209.
  • Loading branch information
JoshRosen authored and sarutak committed May 9, 2016
1 parent c3350ca commit 3323d0f
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 8 deletions.
11 changes: 7 additions & 4 deletions core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import javax.servlet.http.HttpServletRequest
import scala.collection.mutable.{HashMap, ListBuffer}
import scala.xml._

import org.apache.commons.lang3.StringEscapeUtils

import org.apache.spark.JobExecutionStatus
import org.apache.spark.ui.{ToolTips, UIUtils, WebUIPage}
import org.apache.spark.ui.jobs.UIData.{ExecutorUIData, JobUIData}
Expand Down Expand Up @@ -87,9 +89,10 @@ private[ui] class AllJobsPage(parent: JobsTab) extends WebUIPage("") {
case JobExecutionStatus.UNKNOWN => "unknown"
}

// The timeline library treats contents as HTML, so we have to escape them; for the
// data-title attribute string we have to escape them twice since that's in a string.
// The timeline library treats contents as HTML, so we have to escape them. We need to add
// extra layers of escaping in order to embed this in a Javascript string literal.
val escapedDesc = Utility.escape(displayJobDescription)
val jsEscapedDesc = StringEscapeUtils.escapeEcmaScript(escapedDesc)
val jobEventJsonAsStr =
s"""
|{
Expand All @@ -99,7 +102,7 @@ private[ui] class AllJobsPage(parent: JobsTab) extends WebUIPage("") {
| 'end': new Date(${completionTime}),
| 'content': '<div class="application-timeline-content"' +
| 'data-html="true" data-placement="top" data-toggle="tooltip"' +
| 'data-title="${Utility.escape(escapedDesc)} (Job ${jobId})<br>' +
| 'data-title="${jsEscapedDesc} (Job ${jobId})<br>' +
| 'Status: ${status}<br>' +
| 'Submitted: ${UIUtils.formatDate(new Date(submissionTime))}' +
| '${
Expand All @@ -109,7 +112,7 @@ private[ui] class AllJobsPage(parent: JobsTab) extends WebUIPage("") {
""
}
}">' +
| '${escapedDesc} (Job ${jobId})</div>'
| '${jsEscapedDesc} (Job ${jobId})</div>'
|}
""".stripMargin
jobEventJsonAsStr
Expand Down
11 changes: 7 additions & 4 deletions core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import javax.servlet.http.HttpServletRequest
import scala.collection.mutable.{Buffer, HashMap, ListBuffer}
import scala.xml.{Node, NodeSeq, Unparsed, Utility}

import org.apache.commons.lang3.StringEscapeUtils

import org.apache.spark.JobExecutionStatus
import org.apache.spark.scheduler.StageInfo
import org.apache.spark.ui.{ToolTips, UIUtils, WebUIPage}
Expand Down Expand Up @@ -63,9 +65,10 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
val submissionTime = stage.submissionTime.get
val completionTime = stage.completionTime.getOrElse(System.currentTimeMillis())

// The timeline library treats contents as HTML, so we have to escape them; for the
// data-title attribute string we have to escape them twice since that's in a string.
// The timeline library treats contents as HTML, so we have to escape them. We need to add
// extra layers of escaping in order to embed this in a Javascript string literal.
val escapedName = Utility.escape(name)
val jsEscapedName = StringEscapeUtils.escapeEcmaScript(escapedName)
s"""
|{
| 'className': 'stage job-timeline-object ${status}',
Expand All @@ -74,7 +77,7 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
| 'end': new Date(${completionTime}),
| 'content': '<div class="job-timeline-content" data-toggle="tooltip"' +
| 'data-placement="top" data-html="true"' +
| 'data-title="${Utility.escape(escapedName)} (Stage ${stageId}.${attemptId})<br>' +
| 'data-title="${jsEscapedName} (Stage ${stageId}.${attemptId})<br>' +
| 'Status: ${status.toUpperCase}<br>' +
| 'Submitted: ${UIUtils.formatDate(new Date(submissionTime))}' +
| '${
Expand All @@ -84,7 +87,7 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
""
}
}">' +
| '${escapedName} (Stage ${stageId}.${attemptId})</div>',
| '${jsEscapedName} (Stage ${stageId}.${attemptId})</div>',
|}
""".stripMargin
}
Expand Down

0 comments on commit 3323d0f

Please sign in to comment.