You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by sr...@apache.org on 2015/04/17 12:02:46 UTC
spark git commit: SPARK-6846 [WEBUI] Stage kill URL easy to
accidentally trigger and possibility for security issue
Repository: spark
Updated Branches:
refs/heads/master 8220d5265 -> f7a25644e
SPARK-6846 [WEBUI] Stage kill URL easy to accidentally trigger and possibility for security issue
kill endpoints now only accept a POST (kill stage, master kill app, master kill driver); kill link now POSTs
Author: Sean Owen <so...@cloudera.com>
Closes #5528 from srowen/SPARK-6846 and squashes the following commits:
137ac9f [Sean Owen] Oops, fix scalastyle line length probelm
7c5f961 [Sean Owen] Add Imran's test of kill link
59f447d [Sean Owen] kill endpoints now only accept a POST (kill stage, master kill app, master kill driver); kill link now POSTs
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/f7a25644
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/f7a25644
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/f7a25644
Branch: refs/heads/master
Commit: f7a25644ed5b3b49fe7f33743bec3d95cdf7913e
Parents: 8220d52
Author: Sean Owen <so...@cloudera.com>
Authored: Fri Apr 17 11:02:31 2015 +0100
Committer: Sean Owen <so...@cloudera.com>
Committed: Fri Apr 17 11:02:31 2015 +0100
----------------------------------------------------------------------
.../org/apache/spark/ui/static/webui.css | 6 +--
.../spark/deploy/master/ui/MasterPage.scala | 28 ++++++++------
.../spark/deploy/master/ui/MasterWebUI.scala | 8 ++--
.../scala/org/apache/spark/ui/JettyUtils.scala | 17 ++++++++-
.../scala/org/apache/spark/ui/SparkUI.scala | 4 +-
.../org/apache/spark/ui/jobs/StageTable.scala | 27 +++++++------
.../org/apache/spark/ui/UISeleniumSuite.scala | 40 +++++++++++++-------
7 files changed, 78 insertions(+), 52 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/f7a25644/core/src/main/resources/org/apache/spark/ui/static/webui.css
----------------------------------------------------------------------
diff --git a/core/src/main/resources/org/apache/spark/ui/static/webui.css b/core/src/main/resources/org/apache/spark/ui/static/webui.css
index 6c37cc8..4910744 100644
--- a/core/src/main/resources/org/apache/spark/ui/static/webui.css
+++ b/core/src/main/resources/org/apache/spark/ui/static/webui.css
@@ -85,17 +85,13 @@ table.sortable td {
filter: progid:dximagetransform.microsoft.gradient(startColorstr='#FFA4EDFF', endColorstr='#FF94DDFF', GradientType=0);
}
-span.kill-link {
+a.kill-link {
margin-right: 2px;
margin-left: 20px;
color: gray;
float: right;
}
-span.kill-link a {
- color: gray;
-}
-
span.expand-details {
font-size: 10pt;
cursor: pointer;
http://git-wip-us.apache.org/repos/asf/spark/blob/f7a25644/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala
index 399f073..1f2c3fd 100644
--- a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala
@@ -190,12 +190,14 @@ private[ui] class MasterPage(parent: MasterWebUI) extends WebUIPage("") {
private def appRow(app: ApplicationInfo): Seq[Node] = {
val killLink = if (parent.killEnabled &&
(app.state == ApplicationState.RUNNING || app.state == ApplicationState.WAITING)) {
- val killLinkUri = s"app/kill?id=${app.id}&terminate=true"
- val confirm = "return window.confirm(" +
- s"'Are you sure you want to kill application ${app.id} ?');"
- <span class="kill-link">
- (<a href={killLinkUri} onclick={confirm}>kill</a>)
- </span>
+ val confirm =
+ s"if (window.confirm('Are you sure you want to kill application ${app.id} ?')) " +
+ "{ this.parentNode.submit(); return true; } else { return false; }"
+ <form action="app/kill/" method="POST" style="display:inline">
+ <input type="hidden" name="id" value={app.id.toString}/>
+ <input type="hidden" name="terminate" value="true"/>
+ <a href="#" onclick={confirm} class="kill-link">(kill)</a>
+ </form>
}
<tr>
<td>
@@ -223,12 +225,14 @@ private[ui] class MasterPage(parent: MasterWebUI) extends WebUIPage("") {
(driver.state == DriverState.RUNNING ||
driver.state == DriverState.SUBMITTED ||
driver.state == DriverState.RELAUNCHING)) {
- val killLinkUri = s"driver/kill?id=${driver.id}&terminate=true"
- val confirm = "return window.confirm(" +
- s"'Are you sure you want to kill driver ${driver.id} ?');"
- <span class="kill-link">
- (<a href={killLinkUri} onclick={confirm}>kill</a>)
- </span>
+ val confirm =
+ s"if (window.confirm('Are you sure you want to kill driver ${driver.id} ?')) " +
+ "{ this.parentNode.submit(); return true; } else { return false; }"
+ <form action="driver/kill/" method="POST" style="display:inline">
+ <input type="hidden" name="id" value={driver.id.toString}/>
+ <input type="hidden" name="terminate" value="true"/>
+ <a href="#" onclick={confirm} class="kill-link">(kill)</a>
+ </form>
}
<tr>
<td>{driver.id} {killLink}</td>
http://git-wip-us.apache.org/repos/asf/spark/blob/f7a25644/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
index 1b67041..bb11e06 100644
--- a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
@@ -43,10 +43,10 @@ class MasterWebUI(val master: Master, requestedPort: Int)
attachPage(new HistoryNotFoundPage(this))
attachPage(masterPage)
attachHandler(createStaticHandler(MasterWebUI.STATIC_RESOURCE_DIR, "/static"))
- attachHandler(
- createRedirectHandler("/app/kill", "/", masterPage.handleAppKillRequest))
- attachHandler(
- createRedirectHandler("/driver/kill", "/", masterPage.handleDriverKillRequest))
+ attachHandler(createRedirectHandler(
+ "/app/kill", "/", masterPage.handleAppKillRequest, httpMethod = "POST"))
+ attachHandler(createRedirectHandler(
+ "/driver/kill", "/", masterPage.handleDriverKillRequest, httpMethod = "POST"))
}
/** Attach a reconstructed UI to this Master UI. Only valid after bind(). */
http://git-wip-us.apache.org/repos/asf/spark/blob/f7a25644/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
index 95f254a..a091ca6 100644
--- a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
+++ b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
@@ -114,10 +114,23 @@ private[spark] object JettyUtils extends Logging {
srcPath: String,
destPath: String,
beforeRedirect: HttpServletRequest => Unit = x => (),
- basePath: String = ""): ServletContextHandler = {
+ basePath: String = "",
+ httpMethod: String = "GET"): ServletContextHandler = {
val prefixedDestPath = attachPrefix(basePath, destPath)
val servlet = new HttpServlet {
- override def doGet(request: HttpServletRequest, response: HttpServletResponse) {
+ override def doGet(request: HttpServletRequest, response: HttpServletResponse): Unit = {
+ httpMethod match {
+ case "GET" => doRequest(request, response)
+ case _ => response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED)
+ }
+ }
+ override def doPost(request: HttpServletRequest, response: HttpServletResponse): Unit = {
+ httpMethod match {
+ case "POST" => doRequest(request, response)
+ case _ => response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED)
+ }
+ }
+ private def doRequest(request: HttpServletRequest, response: HttpServletResponse): Unit = {
beforeRedirect(request)
// Make sure we don't end up with "//" in the middle
val newUrl = new URL(new URL(request.getRequestURL.toString), prefixedDestPath).toString
http://git-wip-us.apache.org/repos/asf/spark/blob/f7a25644/core/src/main/scala/org/apache/spark/ui/SparkUI.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/ui/SparkUI.scala b/core/src/main/scala/org/apache/spark/ui/SparkUI.scala
index adfa6bb..580ab8b 100644
--- a/core/src/main/scala/org/apache/spark/ui/SparkUI.scala
+++ b/core/src/main/scala/org/apache/spark/ui/SparkUI.scala
@@ -55,8 +55,8 @@ private[spark] class SparkUI private (
attachTab(new ExecutorsTab(this))
attachHandler(createStaticHandler(SparkUI.STATIC_RESOURCE_DIR, "/static"))
attachHandler(createRedirectHandler("/", "/jobs", basePath = basePath))
- attachHandler(
- createRedirectHandler("/stages/stage/kill", "/stages", stagesTab.handleKillRequest))
+ attachHandler(createRedirectHandler(
+ "/stages/stage/kill", "/stages", stagesTab.handleKillRequest, httpMethod = "POST"))
}
initialize()
http://git-wip-us.apache.org/repos/asf/spark/blob/f7a25644/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala b/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala
index 5865850..cb72890 100644
--- a/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala
+++ b/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala
@@ -73,20 +73,21 @@ private[ui] class StageTableBase(
}
private def makeDescription(s: StageInfo): Seq[Node] = {
- // scalastyle:off
+ val basePathUri = UIUtils.prependBaseUri(basePath)
+
val killLink = if (killEnabled) {
- val killLinkUri = "%s/stages/stage/kill?id=%s&terminate=true"
- .format(UIUtils.prependBaseUri(basePath), s.stageId)
- val confirm = "return window.confirm('Are you sure you want to kill stage %s ?');"
- .format(s.stageId)
- <span class="kill-link">
- (<a href={killLinkUri} onclick={confirm}>kill</a>)
- </span>
+ val killLinkUri = s"$basePathUri/stages/stage/kill/"
+ val confirm =
+ s"if (window.confirm('Are you sure you want to kill stage ${s.stageId} ?')) " +
+ "{ this.parentNode.submit(); return true; } else { return false; }"
+ <form action={killLinkUri} method="POST" style="display:inline">
+ <input type="hidden" name="id" value={s.stageId.toString}/>
+ <input type="hidden" name="terminate" value="true"/>
+ <a href="#" onclick={confirm} class="kill-link">(kill)</a>
+ </form>
}
- // scalastyle:on
- val nameLinkUri ="%s/stages/stage?id=%s&attempt=%s"
- .format(UIUtils.prependBaseUri(basePath), s.stageId, s.attemptId)
+ val nameLinkUri = s"$basePathUri/stages/stage?id=${s.stageId}&attempt=${s.attemptId}"
val nameLink = <a href={nameLinkUri}>{s.name}</a>
val cachedRddInfos = s.rddInfos.filter(_.numCachedPartitions > 0)
@@ -98,11 +99,9 @@ private[ui] class StageTableBase(
<div class="stage-details collapsed">
{if (cachedRddInfos.nonEmpty) {
Text("RDD: ") ++
- // scalastyle:off
cachedRddInfos.map { i =>
- <a href={"%s/storage/rdd?id=%d".format(UIUtils.prependBaseUri(basePath), i.id)}>{i.name}</a>
+ <a href={s"$basePathUri/storage/rdd?id=${i.id}"}>{i.name}</a>
}
- // scalastyle:on
}}
<pre>{s.details}</pre>
</div>
http://git-wip-us.apache.org/repos/asf/spark/blob/f7a25644/core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala
----------------------------------------------------------------------
diff --git a/core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala b/core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala
index 1cb5946..eb9db55 100644
--- a/core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala
+++ b/core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala
@@ -17,6 +17,7 @@
package org.apache.spark.ui
+import java.net.{HttpURLConnection, URL}
import javax.servlet.http.HttpServletRequest
import scala.collection.JavaConversions._
@@ -56,12 +57,13 @@ class UISeleniumSuite extends FunSuite with WebBrowser with Matchers with Before
* Create a test SparkContext with the SparkUI enabled.
* It is safe to `get` the SparkUI directly from the SparkContext returned here.
*/
- private def newSparkContext(): SparkContext = {
+ private def newSparkContext(killEnabled: Boolean = true): SparkContext = {
val conf = new SparkConf()
.setMaster("local")
.setAppName("test")
.set("spark.ui.enabled", "true")
.set("spark.ui.port", "0")
+ .set("spark.ui.killEnabled", killEnabled.toString)
val sc = new SparkContext(conf)
assert(sc.ui.isDefined)
sc
@@ -128,21 +130,12 @@ class UISeleniumSuite extends FunSuite with WebBrowser with Matchers with Before
}
test("spark.ui.killEnabled should properly control kill button display") {
- def getSparkContext(killEnabled: Boolean): SparkContext = {
- val conf = new SparkConf()
- .setMaster("local")
- .setAppName("test")
- .set("spark.ui.enabled", "true")
- .set("spark.ui.killEnabled", killEnabled.toString)
- new SparkContext(conf)
- }
-
def hasKillLink: Boolean = find(className("kill-link")).isDefined
def runSlowJob(sc: SparkContext) {
sc.parallelize(1 to 10).map{x => Thread.sleep(10000); x}.countAsync()
}
- withSpark(getSparkContext(killEnabled = true)) { sc =>
+ withSpark(newSparkContext(killEnabled = true)) { sc =>
runSlowJob(sc)
eventually(timeout(5 seconds), interval(50 milliseconds)) {
go to (sc.ui.get.appUIAddress.stripSuffix("/") + "/stages")
@@ -150,7 +143,7 @@ class UISeleniumSuite extends FunSuite with WebBrowser with Matchers with Before
}
}
- withSpark(getSparkContext(killEnabled = false)) { sc =>
+ withSpark(newSparkContext(killEnabled = false)) { sc =>
runSlowJob(sc)
eventually(timeout(5 seconds), interval(50 milliseconds)) {
go to (sc.ui.get.appUIAddress.stripSuffix("/") + "/stages")
@@ -233,7 +226,7 @@ class UISeleniumSuite extends FunSuite with WebBrowser with Matchers with Before
// because someone could change the error message and cause this test to pass by accident.
// Instead, it's safer to check that each row contains a link to a stage details page.
findAll(cssSelector("tbody tr")).foreach { row =>
- val link = row.underlying.findElement(By.xpath(".//a"))
+ val link = row.underlying.findElement(By.xpath("./td/div/a"))
link.getAttribute("href") should include ("stage")
}
}
@@ -356,4 +349,25 @@ class UISeleniumSuite extends FunSuite with WebBrowser with Matchers with Before
}
}
}
+
+ test("kill stage is POST only") {
+ def getResponseCode(url: URL, method: String): Int = {
+ val connection = url.openConnection().asInstanceOf[HttpURLConnection]
+ connection.setRequestMethod(method)
+ connection.connect()
+ val code = connection.getResponseCode()
+ connection.disconnect()
+ code
+ }
+
+ withSpark(newSparkContext(killEnabled = true)) { sc =>
+ sc.parallelize(1 to 10).map{x => Thread.sleep(10000); x}.countAsync()
+ eventually(timeout(5 seconds), interval(50 milliseconds)) {
+ val url = new URL(
+ sc.ui.get.appUIAddress.stripSuffix("/") + "/stages/stage/kill/?id=0&terminate=true")
+ getResponseCode(url, "GET") should be (405)
+ getResponseCode(url, "POST") should be (200)
+ }
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org