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 a3e12b3
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 26 deletions.
45 changes: 21 additions & 24 deletions cask/src/cask/main/Main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import io.undertow.server.handlers.BlockingHandler
import io.undertow.util.HttpString

import scala.concurrent.ExecutionContext
import geny.Generator

/**
* A combination of [[cask.Main]] and [[cask.Routes]], ideal for small
Expand Down Expand Up @@ -74,7 +75,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 +102,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 +123,8 @@ object Main{
)
None
}
case _ => Main.writeResponse(exchange, handleMethodNotAllowed())
}
// println("Completed Request: " + exchange.getRequestPath)
}catch{case e: Throwable =>
e.printStackTrace()
}
Expand All @@ -149,23 +144,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 a3e12b3

Please sign in to comment.