Skip to content

Commit

Permalink
Prevent cask from responding with 405 for an undefined route.
Browse files Browse the repository at this point in the history
Fixes #51.

Prior to this commit, `prepareRouteTries` created a mapping from
method-name to DispatchTrie (Map[String, DispatchTrie[…]]).

This commit instead creates a DispatchTrie[Map[String, …]],
basically an inversion of the previous result.

The updated tests in minimalApplication and minimalApplication2
have been updated to cover the differences.
  • Loading branch information
megri committed May 18, 2021
1 parent 156ddff commit cf806ca
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 26 deletions.
44 changes: 20 additions & 24 deletions cask/src/cask/main/Main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ abstract class Main{
}

object Main{
class DefaultHandler(routeTries: Map[String, DispatchTrie[(Routes, EndpointMetadata[_])]],
class DefaultHandler(routeTries: DispatchTrie[Map[String, (Routes, EndpointMetadata[_])]],
mainDecorators: Seq[Decorator[_, _, _]],
debugMode: Boolean,
handleNotFound: () => Response.Raw,
Expand All @@ -101,16 +101,10 @@ object Main{
(r: Any) => Main.writeResponse(exchange, r.asInstanceOf[Response.Raw])
)

val dispatchTrie: DispatchTrie[(Routes, EndpointMetadata[_])] = routeTries.get(effectiveMethod) match {
case None =>
Main.writeResponse(exchange, handleMethodNotAllowed())
return
case Some(trie) => trie
}

dispatchTrie.lookup(Util.splitPath(exchange.getRequestPath).toList, Map()) match {
routeTries.lookup(Util.splitPath(exchange.getRequestPath).toList, Map()) match{
case None => Main.writeResponse(exchange, handleNotFound())
case Some(((routes, metadata), routeBindings, remaining)) =>
case Some((methodMap, routeBindings, remaining)) if methodMap.contains(effectiveMethod) =>
val (routes, metadata) = methodMap(effectiveMethod)
Decorator.invoke(
Request(exchange, remaining),
metadata.endpoint,
Expand All @@ -128,8 +122,8 @@ object Main{
)
None
}
case _ => Main.writeResponse(exchange, handleMethodNotAllowed())
}
// println("Completed Request: " + exchange.getRequestPath)
}catch{case e: Throwable =>
e.printStackTrace()
}
Expand All @@ -149,23 +143,25 @@ object Main{
)
}

def prepareRouteTries(allRoutes: Seq[Routes]): Map[String, DispatchTrie[(Routes, EndpointMetadata[_])]] = {
val routeList = for{
def prepareRouteTries(allRoutes: Seq[Routes]): DispatchTrie[Map[String, (Routes, EndpointMetadata[_])]] = {
val flattenedRoutes = for {
routes <- allRoutes
route <- routes.caskMetadata.value.map(x => x: EndpointMetadata[_])
} yield (routes, route)
metadata <- routes.caskMetadata.value
} yield {
val segments = Util.splitPath(metadata.endpoint.path)
val methodMap = metadata.endpoint.methods.map(_ -> (routes, metadata: EndpointMetadata[_])).toMap
(segments, methodMap, metadata.endpoint.subpath)
}

val allMethods: Set[String] =
routeList.flatMap(_._2.endpoint.methods).map(_.toLowerCase).toSet
val dispatchInputs = flattenedRoutes.groupBy(_._1).map { case (segments, values) =>
val methodMap = values.map(_._2).flatten.toMap
val hasSubpath = values.map(_._3).contains(true)
(segments, methodMap, hasSubpath)
}.toSeq

allMethods
.map { method =>
method -> DispatchTrie.construct[(Routes, EndpointMetadata[_])](0,
for ((route, metadata) <- routeList if metadata.endpoint.methods.contains(method))
yield (Util.splitPath(metadata.endpoint.path): collection.IndexedSeq[String], (route, metadata), metadata.endpoint.subpath)
)
}.toMap
DispatchTrie.construct(0, dispatchInputs)
}

def writeResponse(exchange: HttpServerExchange, response: Response.Raw) = {
response.data.headers.foreach{case (k, v) =>
exchange.getResponseHeaders.put(new HttpString(k), v)
Expand Down
2 changes: 1 addition & 1 deletion example/minimalApplication/app/test/src/ExampleTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ object ExampleTests extends TestSuite{

requests.post(s"$host/do-thing", data = "hello").text() ==> "olleh"

requests.get(s"$host/do-thing", check = false).statusCode ==> 404
requests.delete(s"$host/do-thing", check = false).statusCode ==> 405
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ object ExampleTests extends TestSuite{

requests.post(s"$host/do-thing", data = "hello").text() ==> "olleh"

requests.get(s"$host/do-thing", check = false).statusCode ==> 404
requests.delete(s"$host/do-thing", check = false).statusCode ==> 405
}
}
}

0 comments on commit cf806ca

Please sign in to comment.