diff --git a/finagle-benchmark/src/main/scala/com/twitter/finagle/http/HttpMuxerBenchmark.scala b/finagle-benchmark/src/main/scala/com/twitter/finagle/http/HttpMuxerBenchmark.scala new file mode 100644 index 0000000000..c133561693 --- /dev/null +++ b/finagle-benchmark/src/main/scala/com/twitter/finagle/http/HttpMuxerBenchmark.scala @@ -0,0 +1,52 @@ +package com.twitter.finagle.http + +import com.twitter.finagle.benchmark.StdBenchAnnotations +import org.openjdk.jmh.annotations.{Benchmark, Scope, State} + +@State(Scope.Benchmark) +class HttpMuxerBenchmark extends StdBenchAnnotations { + + private[this] var i = 0 + + private[this] val alreadyNormalized = IndexedSeq( + "/foo/bar", + "/cool/cool/cool", + "/" + ) + + private[this] val excessiveSlashes = IndexedSeq( + "/foo///bar", + "/cool//cool////cool", + "////" + ) + + private[this] val missingLeadingSlash = IndexedSeq( + "foo/bar", + "cool/cool/cool" + ) + + @Benchmark + def normalize_alreadyNormalized: String = { + i += 1 + if (i < 0) i = 0 + val path = alreadyNormalized(i % alreadyNormalized.size) + HttpMuxer.normalize(path) + } + + @Benchmark + def normalize_excessiveSlashes: String = { + i += 1 + if (i < 0) i = 0 + val path = excessiveSlashes(i % excessiveSlashes.size) + HttpMuxer.normalize(path) + } + + @Benchmark + def normalize_missingLeadingSlash: String = { + i += 1 + if (i < 0) i = 0 + val path = missingLeadingSlash(i % missingLeadingSlash.size) + HttpMuxer.normalize(path) + } + +} diff --git a/finagle-http/src/main/scala/com/twitter/finagle/http/HttpMuxer.scala b/finagle-http/src/main/scala/com/twitter/finagle/http/HttpMuxer.scala index bdf5cfd236..c0a45d89f4 100644 --- a/finagle-http/src/main/scala/com/twitter/finagle/http/HttpMuxer.scala +++ b/finagle-http/src/main/scala/com/twitter/finagle/http/HttpMuxer.scala @@ -2,8 +2,9 @@ package com.twitter.finagle.http import com.twitter.finagle.util.LoadService import com.twitter.finagle.Service -import com.twitter.util.{Future, Time, Closable} +import com.twitter.util.{Closable, Future, Time} import java.util.logging.Logger +import scala.annotation.tailrec /** * A service that dispatches incoming requests to registered handlers. @@ -23,6 +24,7 @@ import java.util.logging.Logger * NOTE: When multiple pattern matches exist, the longest pattern wins. */ class HttpMuxer(_routes: Seq[Route]) extends Service[Request, Response] { + import HttpMuxer.normalize def this() = this(Seq.empty[Route]) @@ -77,18 +79,6 @@ class HttpMuxer(_routes: Seq[Route]) extends Service[Request, Response] { case None => Future.value(Response(request.version, Status.NotFound)) } - /** - * - ensure path starts with "/" - * - get rid of excessive "/"s. For example "/a//b///c/" => "/a/b/c/" - * - return "" if path is "" - * - return "/" if path is "/" or "///" etc - */ - private[this] def normalize(path: String) = { - val suffix = if (path.endsWith("/")) "/" else "" - val p = path.split("/").filterNot(_.isEmpty).mkString("/") - if (p == "") suffix else "/" + p + suffix - } - override def close(deadline: Time): Future[Unit] = Closable.all(routes.map(_.handler): _*).close(deadline) } @@ -101,6 +91,45 @@ class HttpMuxer(_routes: Seq[Route]) extends Service[Request, Response] { object HttpMuxer extends HttpMuxer { @volatile private[this] var underlying = new HttpMuxer() + private[this] def addLeadingAndStripDuplicateSlashes(s: String): String = { + val b = new java.lang.StringBuilder(s.length) + b.append('/') + + @tailrec + def go(i: Int, lastSlash: Boolean): Unit = { + if (i < s.length) { + val c = s.charAt(i) + if (lastSlash && c == '/') go(i + 1, true) + else { + b.append(c) + go(i + 1, c == '/') + } + } + } + go(0, true) + + b.toString + } + + /** + * - ensure path starts with "/" (unless path is "") + * - get rid of excessive "/"s. For example "/a//b///c/" => "/a/b/c/" + * - return "" if path is "" + * - return "/" if path is "/" or "///" etc + * + * @note exposed for testing. + */ + private[http] def normalize(path: String): String = { + if (path.isEmpty) { + "" + } else if (!path.contains("//")) { + if (path.startsWith("/")) path + else "/" + path + } else { + addLeadingAndStripDuplicateSlashes(path) + } + } + /** * add handlers to mutate dispatching strategies. */ diff --git a/finagle-http/src/test/scala/com/twitter/finagle/http/HttpMuxerTest.scala b/finagle-http/src/test/scala/com/twitter/finagle/http/HttpMuxerTest.scala index 388f59111b..9f097e638b 100644 --- a/finagle-http/src/test/scala/com/twitter/finagle/http/HttpMuxerTest.scala +++ b/finagle-http/src/test/scala/com/twitter/finagle/http/HttpMuxerTest.scala @@ -32,6 +32,20 @@ class HttpMuxerTest extends FunSuite { assert(Await.result(muxService(Request("/foo/bar/blah?j={}"))).contentString == fooBarPrefix) } + test("normalize basics") { + assert(HttpMuxer.normalize("") == "") + assert(HttpMuxer.normalize("/") == "/") + assert(HttpMuxer.normalize("/foo") == "/foo") + assert(HttpMuxer.normalize("/foo/") == "/foo/") + assert(HttpMuxer.normalize("foo") == "/foo") + } + + test("normalize duplicate slashes") { + assert(HttpMuxer.normalize("/foo//bar") == "/foo/bar") + assert(HttpMuxer.normalize("///foo//bar") == "/foo/bar") + assert(HttpMuxer.normalize("/foo//bar///") == "/foo/bar/") + } + test("prefix matching is handled correctly") { assert(await(muxService(Request("/foo/"))).contentString == percentEncode)