diff --git a/CHANGES b/CHANGES index f18da975..d6165b7c 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,12 @@ Note that ``RB_ID=#`` correspond to associated messages in commits. 1.x ----- +Bug Fixes +~~~~~~~~~ + + * twitter-server: escape user input that is rendered in HTML, and make + bin/travisci publish finagle-toggle. ``RB_ID=848579`` + New Features ~~~~~~~~~~~~ diff --git a/bin/travisci b/bin/travisci index 797f945c..e93e2784 100755 --- a/bin/travisci +++ b/bin/travisci @@ -38,6 +38,7 @@ if [ "$TWITTER_SERVER_BRANCH" != "master" ]; then # finagle-thrift, finagle-zipkin cd $TWITTER_SERVER_TMP_DIR/finagle ./sbt ++$SCALA_VERSION finagle-thrift/publishLocal + ./sbt ++$SCALA_VERSION finagle-toggle/publishLocal ./sbt ++$SCALA_VERSION finagle-zipkin-core/publishLocal ./sbt ++$SCALA_VERSION finagle-zipkin/publishLocal # clean up diff --git a/src/main/scala/com/twitter/server/handler/ClientRegistryHandler.scala b/src/main/scala/com/twitter/server/handler/ClientRegistryHandler.scala index da9b2d2b..b36cf36c 100644 --- a/src/main/scala/com/twitter/server/handler/ClientRegistryHandler.scala +++ b/src/main/scala/com/twitter/server/handler/ClientRegistryHandler.scala @@ -5,6 +5,7 @@ import com.twitter.finagle.client.{ClientRegistry, EndpointRegistry} import com.twitter.finagle.http.{Request, Response} import com.twitter.finagle.util.StackRegistry import com.twitter.io.Buf +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.{parse, new404, newResponse} import com.twitter.server.util.MetricSource import com.twitter.server.view.{EndpointRegistryView, StackRegistryView} @@ -32,22 +33,22 @@ private object ClientRegistryHandler { /** Renders `profiles` in an html template. */ def render(title: String, profiles: Seq[ClientProfile]): String = - s"""

${title}

+ s"""

${escapeHtml(title)}

${ (for (ClientProfile(name, addr, scope, sr, unavailable) <- profiles) yield { s"""
-

${name}

-

$addr

+

${escapeHtml(name)}

+

${escapeHtml(addr)}

${ if (unavailable == 0) "" else { s""" $unavailable unavailable endpoint(s)""" + aria-hidden="true"> ${escapeHtml(unavailable.toString)} unavailable endpoint(s)""" } }
diff --git a/src/main/scala/com/twitter/server/handler/EventRecordingHandler.scala b/src/main/scala/com/twitter/server/handler/EventRecordingHandler.scala index 550734fd..cd381db9 100644 --- a/src/main/scala/com/twitter/server/handler/EventRecordingHandler.scala +++ b/src/main/scala/com/twitter/server/handler/EventRecordingHandler.scala @@ -5,6 +5,7 @@ import com.twitter.finagle.http.{Request, Response} import com.twitter.io.Buf import com.twitter.util.Future import com.twitter.util.events.Sink +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.newResponse import java.util.logging.Logger @@ -33,7 +34,7 @@ private[server] class EventRecordingHandler( newResponse( contentType = "text/html;charset=UTF-8", - content = Buf.Utf8(reply)) + content = Buf.Utf8(escapeHtml(reply))) } private[handler] def updateRecording(uri: String): String = { diff --git a/src/main/scala/com/twitter/server/handler/HistogramQueryHandler.scala b/src/main/scala/com/twitter/server/handler/HistogramQueryHandler.scala index 78354298..d3f9e257 100644 --- a/src/main/scala/com/twitter/server/handler/HistogramQueryHandler.scala +++ b/src/main/scala/com/twitter/server/handler/HistogramQueryHandler.scala @@ -4,6 +4,7 @@ import com.twitter.finagle.http.{Request, Response} import com.twitter.finagle.Service import com.twitter.finagle.stats.{BucketAndCount, HistogramDetail, WithHistogramDetails} import com.twitter.io.Buf +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.{newResponse, parse} import com.twitter.server.util.JsonConverter import com.twitter.util.Future @@ -55,7 +56,7 @@ object HistogramQueryHandler { val statsTable = { def entry(name: String): String = { s""" - $name: + ${escapeHtml(name)}: """ } @@ -120,7 +121,7 @@ object HistogramQueryHandler {
diff --git a/src/main/scala/com/twitter/server/handler/IndexHandler.scala b/src/main/scala/com/twitter/server/handler/IndexHandler.scala index 30cb36c2..659ab0ca 100644 --- a/src/main/scala/com/twitter/server/handler/IndexHandler.scala +++ b/src/main/scala/com/twitter/server/handler/IndexHandler.scala @@ -3,6 +3,7 @@ package com.twitter.server.handler import com.twitter.finagle.Service import com.twitter.finagle.http.{HttpMuxer, Request, Response} import com.twitter.io.Buf +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.{expectsHtml, newOk, newResponse} import com.twitter.util.Future @@ -17,7 +18,7 @@ class IndexHandler( extends Service[Request, Response] { def apply(req: Request): Future[Response] = { val paths = patterns.filter(_.startsWith(prefix)) - val links = paths map { p => s"$p" } + val links = paths map { p => s"${escapeHtml(p)}" } if (!expectsHtml(req)) newOk(paths.mkString("\n")) else newResponse( contentType = "text/html;charset=UTF-8", diff --git a/src/main/scala/com/twitter/server/handler/LintHandler.scala b/src/main/scala/com/twitter/server/handler/LintHandler.scala index 9805f5be..e04f7ac0 100644 --- a/src/main/scala/com/twitter/server/handler/LintHandler.scala +++ b/src/main/scala/com/twitter/server/handler/LintHandler.scala @@ -4,6 +4,7 @@ import com.twitter.finagle.Service import com.twitter.finagle.http.{Response, Request} import com.twitter.io.Buf import com.twitter.server.handler.LintHandler.LintView +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils._ import com.twitter.server.util.JsonConverter import com.twitter.util.Future @@ -113,9 +114,6 @@ private object LintHandler { }); """ - private def escapeHtml(s: String): String = - xml.Utility.escape(s) - def summary: String = { val numIssues = nots.foldLeft(0) { case (total, (_, issues)) => total + issues.size @@ -132,19 +130,19 @@ private object LintHandler { Number of rules run - ${rules.size} + ${escapeHtml(rules.size.toString)} Number of rules ok - ${oks.size} + ${escapeHtml(oks.size.toString)} Number of rules failed - ${nots.size} + ${escapeHtml(nots.size.toString)} Number of issues found - $numIssues + ${escapeHtml(numIssues.toString)} @@ -174,7 +172,7 @@ private object LintHandler { def failedRows: String = { val data = nots.map { case (rule, issues) => issues.map { issue => - failedRow(rule, issue) + escapeHtml(failedRow(rule, issue)) }.mkString("") }.mkString("") diff --git a/src/main/scala/com/twitter/server/handler/LoggingHandler.scala b/src/main/scala/com/twitter/server/handler/LoggingHandler.scala index 19e2c4f1..81fa852b 100644 --- a/src/main/scala/com/twitter/server/handler/LoggingHandler.scala +++ b/src/main/scala/com/twitter/server/handler/LoggingHandler.scala @@ -4,6 +4,7 @@ import com.twitter.finagle.Service import com.twitter.finagle.http.{Request, Response} import com.twitter.io.Buf import com.twitter.logging.{Level, Logger} +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.{expectsHtml, newResponse, parse} import com.twitter.util.Future import java.net.URLEncoder @@ -27,14 +28,14 @@ private object LoggingHandler { def renderText(loggers: Seq[Logger], updateMsg: String): String = { val out = loggers.toSeq.map { logger => val loggerName = if (logger.name == "") "root" else logger.name - s"$loggerName ${getLevel(logger)}" + escapeHtml(s"$loggerName ${getLevel(logger)}") }.mkString("\n") - if (updateMsg.isEmpty) s"$out" else s"$updateMsg\n$out" + if (updateMsg.isEmpty) s"$out" else s"${escapeHtml(updateMsg)}\n$out" } def renderHtml(loggers: Seq[Logger], levels: Seq[Level], updateMsg: String): String = s""" - + @@ -45,7 +46,7 @@ private object LoggingHandler { (for (logger <- loggers) yield { val loggerName = if (logger.name == "") "root" else logger.name s""" - + - - + + """ }).mkString("\n") val modules = entry.modules s"""

- ${entry.name} - ${entry.addr} + ${escapeHtml(entry.name)} + ${escapeHtml(entry.addr)}

${ (for (scope <- statScope) yield { s""" - Watch metrics for ${entry.name} + Watch metrics for ${escapeHtml(entry.name)} """ }).getOrElse("") } @@ -47,7 +48,7 @@ private[server] object StackRegistryView { @@ -59,8 +60,8 @@ private[server] object StackRegistryView { s"""
-
${role}
-

${desc}

+
${escapeHtml(role)}
+

${escapeHtml(desc)}

${ if (params.isEmpty) "" else { s"""
$updateMsg${escapeHtml(updateMsg)}
com.twitter.logging.Logger
$loggerName
${escapeHtml(loggerName)}
${ (for (level <- levels) yield { @@ -93,10 +94,10 @@ class LoggingHandler extends Service[Request, Response] { logger <- Logger.iterator.find(_.name == name) } yield { logger.setLevel(level) - s"""Changed ${if (logger.name == "") "root" else logger.name} to Level.$level""" + escapeHtml(s"""Changed ${if (logger.name == "") "root" else logger.name} to Level.$level""") } - updated.getOrElse(s"Unable to change $name to Level.$level!") + escapeHtml(updated.getOrElse(s"Unable to change $name to Level.$level!")) case _ => "" } diff --git a/src/main/scala/com/twitter/server/handler/MetricQueryHandler.scala b/src/main/scala/com/twitter/server/handler/MetricQueryHandler.scala index d369365a..c67b16ce 100644 --- a/src/main/scala/com/twitter/server/handler/MetricQueryHandler.scala +++ b/src/main/scala/com/twitter/server/handler/MetricQueryHandler.scala @@ -3,6 +3,7 @@ package com.twitter.server.handler import com.twitter.finagle.Service import com.twitter.finagle.http.{Request, Response} import com.twitter.io.Buf +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.{newResponse, parse} import com.twitter.server.util.{JsonConverter, MetricSource} import com.twitter.util.Future @@ -16,7 +17,7 @@ private object MetricQueryHandler {
    ${ (for (key <- keys.toSeq.sorted) yield { - s"""
  • $key
    • """ + s"""
  • ${escapeHtml(key)}
    • """ }).mkString("\n") }
diff --git a/src/main/scala/com/twitter/server/handler/ServerRegistryHandler.scala b/src/main/scala/com/twitter/server/handler/ServerRegistryHandler.scala index bc5c0197..482b0edd 100644 --- a/src/main/scala/com/twitter/server/handler/ServerRegistryHandler.scala +++ b/src/main/scala/com/twitter/server/handler/ServerRegistryHandler.scala @@ -5,6 +5,7 @@ import com.twitter.finagle.http.{Request, Response} import com.twitter.finagle.server.ServerRegistry import com.twitter.finagle.util.StackRegistry import com.twitter.io.Buf +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.{new404, newResponse, parse} import com.twitter.server.util.MetricSource import com.twitter.server.view.StackRegistryView @@ -20,7 +21,7 @@ private object ServerRegistryHandler { (for { (scope, entry) <- servers } yield { - s"""
  • $scope
    • """ + s"""
  • ${escapeHtml(scope)}
    • """ }).mkString("\n") } diff --git a/src/main/scala/com/twitter/server/util/HtmlUtils.scala b/src/main/scala/com/twitter/server/util/HtmlUtils.scala new file mode 100644 index 00000000..7b245f7a --- /dev/null +++ b/src/main/scala/com/twitter/server/util/HtmlUtils.scala @@ -0,0 +1,8 @@ +package com.twitter.server.util + +private[server] object HtmlUtils { + + private[server] def escapeHtml(s: String) : String = + scala.xml.Utility.escape(s) + +} diff --git a/src/main/scala/com/twitter/server/view/IndexView.scala b/src/main/scala/com/twitter/server/view/IndexView.scala index ec1002b9..f0f6bf51 100644 --- a/src/main/scala/com/twitter/server/view/IndexView.scala +++ b/src/main/scala/com/twitter/server/view/IndexView.scala @@ -4,6 +4,7 @@ import com.twitter.concurrent.AsyncStream import com.twitter.finagle.{Service, SimpleFilter} import com.twitter.finagle.http.{Request, Response} import com.twitter.io.{Reader, Buf} +import com.twitter.server.util.HtmlUtils.escapeHtml import com.twitter.server.util.HttpUtils.{expectsHtml, newResponse} import com.twitter.util.Future @@ -35,7 +36,7 @@ object IndexView { sb ++= s"""
  • - ${id} + ${escapeHtml(id)}
    • """ @@ -55,7 +56,7 @@ object IndexView {
  • - ${id} + ${escapeHtml(id)}
      ${renderNav(links)}
    • """ diff --git a/src/main/scala/com/twitter/server/view/StackRegistryView.scala b/src/main/scala/com/twitter/server/view/StackRegistryView.scala index 48fc8ff0..e38f6c32 100644 --- a/src/main/scala/com/twitter/server/view/StackRegistryView.scala +++ b/src/main/scala/com/twitter/server/view/StackRegistryView.scala @@ -1,6 +1,7 @@ package com.twitter.server.view import com.twitter.finagle.util.StackRegistry +import com.twitter.server.util.HtmlUtils.escapeHtml private[server] object StackRegistryView { /** @@ -18,23 +19,23 @@ private[server] object StackRegistryView { def renderParams(params: Seq[(String, String)]): String = (for ((field, value) <- params) yield { s"""
    ${field}${value}${escapeHtml(field)}${escapeHtml(value)}
    diff --git a/src/main/scala/com/twitter/server/view/ThreadsView.scala b/src/main/scala/com/twitter/server/view/ThreadsView.scala index ace586f6..1830fdcc 100644 --- a/src/main/scala/com/twitter/server/view/ThreadsView.scala +++ b/src/main/scala/com/twitter/server/view/ThreadsView.scala @@ -1,6 +1,8 @@ package com.twitter.server.view import com.twitter.server.handler.ThreadsHandler.{StackTrace, ThreadInfo} +import com.twitter.server.util.HtmlUtils.escapeHtml +import com.twitter.server.view.ThreadsView._ private object ThreadsView { @@ -58,11 +60,6 @@ private[server] class ThreadsView( all: Seq[ThreadInfo], deadlockedIds: Seq[Long]) { - import com.twitter.server.view.ThreadsView._ - - private def escapeHtml(s: String): String = - xml.Utility.escape(s) - private def summary: String = { val filtered = all.filter { info => deadlockedIds.contains(info.thread.getId) @@ -71,7 +68,7 @@ private[server] class ThreadsView( val deadlockLinks = if (filtered.isEmpty) "none" else { filtered.map { info => val id = info.thread.getId - s"""$id""" + s"""${escapeHtml(id.toString)}""" }.mkString(", ") } @@ -119,7 +116,7 @@ private[server] class ThreadsView( val daemonText = if (thread.isDaemon) "Yes" else "No" val threadState = - s"${if (t.isIdle) "Idle" else "Active"} (${thread.getState})" + s"${if (t.isIdle) "Idle" else "Active"} (${escapeHtml(thread.getState.toString)})" val domId = s"threadId-${thread.getId}" val stackDomId = s"threadId-stack-${thread.getId}" @@ -130,11 +127,11 @@ private[server] class ThreadsView( Stack - - - - - + + + + +
    ${thread.getId}${escapeHtml(thread.getName)}$threadState$daemonText${thread.getPriority}${thread.getId.toString}${thread.getName}${threadState}${daemonText}${thread.getPriority.toString}