From ec7156698aa2efd13732b3f89bd96f30da27efcb Mon Sep 17 00:00:00 2001 From: 2beaucoup Date: Wed, 10 Dec 2014 12:35:50 +0100 Subject: [PATCH] !htc #16494 provide content negotiation fallback to exception handlers --- .../akka/http/model/japi/headers/Accept.java | 2 ++ .../scala/akka/http/model/MediaType.scala | 2 ++ .../akka/http/model/headers/headers.scala | 1 + .../directives/ExecutionDirectivesSpec.scala | 24 ++++++++++++++++++- .../akka/http/server/RequestContext.scala | 2 +- .../akka/http/server/RequestContextImpl.scala | 16 +++++++++++-- .../directives/ExecutionDirectives.scala | 4 ++-- 7 files changed, 45 insertions(+), 6 deletions(-) diff --git a/akka-http-core/src/main/java/akka/http/model/japi/headers/Accept.java b/akka-http-core/src/main/java/akka/http/model/japi/headers/Accept.java index 7f2b6b73c8..c1088067cf 100644 --- a/akka-http-core/src/main/java/akka/http/model/japi/headers/Accept.java +++ b/akka-http-core/src/main/java/akka/http/model/japi/headers/Accept.java @@ -13,6 +13,8 @@ import akka.http.model.japi.MediaRange; public abstract class Accept extends akka.http.model.HttpHeader { public abstract Iterable getMediaRanges(); + public abstract boolean acceptsAll(); + public static Accept create(MediaRange... mediaRanges) { return new akka.http.model.headers.Accept(akka.http.model.japi.Util.convertArray(mediaRanges)); } diff --git a/akka-http-core/src/main/scala/akka/http/model/MediaType.scala b/akka-http-core/src/main/scala/akka/http/model/MediaType.scala index 85f2d694ac..66649e5ceb 100644 --- a/akka-http-core/src/main/scala/akka/http/model/MediaType.scala +++ b/akka-http-core/src/main/scala/akka/http/model/MediaType.scala @@ -22,6 +22,7 @@ sealed abstract class MediaRange extends japi.MediaRange with Renderable with Wi def isMultipart = false def isText = false def isVideo = false + def isWildcard = mainType == "*" /** * Returns a copy of this instance with the params replaced by the given ones. @@ -123,6 +124,7 @@ object MediaRanges extends ObjectRegistry[String, MediaRange] { def matches(mediaType: MediaType) = true def specimen = MediaTypes.`text/plain` } + val `*/*(minQ)` = `*/*`.withQValue(Float.MinPositiveValue) val `application/*` = new PredefinedMediaRange("application/*") { def matches(mediaType: MediaType) = mediaType.isApplication override def isApplication = true diff --git a/akka-http-core/src/main/scala/akka/http/model/headers/headers.scala b/akka-http-core/src/main/scala/akka/http/model/headers/headers.scala index 96cbc0ae58..1e21b6afba 100644 --- a/akka-http-core/src/main/scala/akka/http/model/headers/headers.scala +++ b/akka-http-core/src/main/scala/akka/http/model/headers/headers.scala @@ -129,6 +129,7 @@ final case class Accept(mediaRanges: immutable.Seq[MediaRange]) extends japi.hea import Accept.mediaRangesRenderer def renderValue[R <: Rendering](r: R): r.type = r ~~ mediaRanges protected def companion = Accept + def acceptsAll = mediaRanges.exists(mr ⇒ mr.isWildcard && mr.qValue > 0f) /** Java API */ def getMediaRanges: Iterable[japi.MediaRange] = mediaRanges.asJava diff --git a/akka-http-tests/src/test/scala/akka/http/server/directives/ExecutionDirectivesSpec.scala b/akka-http-tests/src/test/scala/akka/http/server/directives/ExecutionDirectivesSpec.scala index dca4bf3172..61e20cdbce 100644 --- a/akka-http-tests/src/test/scala/akka/http/server/directives/ExecutionDirectivesSpec.scala +++ b/akka-http-tests/src/test/scala/akka/http/server/directives/ExecutionDirectivesSpec.scala @@ -5,7 +5,8 @@ package akka.http.server package directives -import akka.http.model.StatusCodes +import akka.http.model.{ MediaTypes, MediaRanges, StatusCodes } +import akka.http.model.headers._ import scala.concurrent.Future @@ -72,6 +73,27 @@ class ExecutionDirectivesSpec extends RoutingSpec { responseAs[String] shouldEqual "There was an internal server error." } } + "always fall back to a default content type" in { + Get("/abc") ~> Accept(MediaTypes.`application/json`) ~> + get { + handleExceptions(handler) { + throw new RuntimeException + } + } ~> check { + status shouldEqual StatusCodes.InternalServerError + responseAs[String] shouldEqual "There was an internal server error." + } + + Get("/abc") ~> Accept(MediaTypes.`text/xml`, MediaRanges.`*/*`.withQValue(0f)) ~> + get { + handleExceptions(handler) { + throw new RuntimeException + } + } ~> check { + status shouldEqual StatusCodes.InternalServerError + responseAs[String] shouldEqual "There was an internal server error." + } + } } def exceptionShouldBeHandled(route: Route) = diff --git a/akka-http/src/main/scala/akka/http/server/RequestContext.scala b/akka-http/src/main/scala/akka/http/server/RequestContext.scala index 6973e5b656..2604bf1dc7 100644 --- a/akka-http/src/main/scala/akka/http/server/RequestContext.scala +++ b/akka-http/src/main/scala/akka/http/server/RequestContext.scala @@ -112,5 +112,5 @@ trait RequestContext { /** * Removes a potentially existing Accept header from the request headers. */ - def withContentNegotiationDisabled: RequestContext + def withAcceptAll: RequestContext } diff --git a/akka-http/src/main/scala/akka/http/server/RequestContextImpl.scala b/akka-http/src/main/scala/akka/http/server/RequestContextImpl.scala index f25f9fd548..0190255f0f 100644 --- a/akka-http/src/main/scala/akka/http/server/RequestContextImpl.scala +++ b/akka-http/src/main/scala/akka/http/server/RequestContextImpl.scala @@ -69,8 +69,20 @@ private[http] class RequestContextImpl( override def mapUnmatchedPath(f: Uri.Path ⇒ Uri.Path): RequestContext = copy(unmatchedPath = f(unmatchedPath)) - override def withContentNegotiationDisabled: RequestContext = - copy(request = request.withHeaders(request.headers filterNot (_.isInstanceOf[headers.Accept]))) + override def withAcceptAll: RequestContext = request.header[headers.Accept] match { + case Some(accept @ headers.Accept(mediaRanges)) if !accept.acceptsAll ⇒ + mapRequest(_.mapHeaders(_.map { + case `accept` ⇒ + val acceptAll = + if (mediaRanges.exists(_.isWildcard)) + mediaRanges.map(mr ⇒ if (mr.isWildcard) mr.withQValue(Float.MinPositiveValue) else mr) + else + mediaRanges :+ MediaRanges.`*/*(minQ)` + accept.copy(mediaRanges = acceptAll) + case x ⇒ x + })) + case _ ⇒ this + } private def copy(request: HttpRequest = request, unmatchedPath: Uri.Path = unmatchedPath, diff --git a/akka-http/src/main/scala/akka/http/server/directives/ExecutionDirectives.scala b/akka-http/src/main/scala/akka/http/server/directives/ExecutionDirectives.scala index 23b89d8dda..2a06b5fe14 100644 --- a/akka-http/src/main/scala/akka/http/server/directives/ExecutionDirectives.scala +++ b/akka-http/src/main/scala/akka/http/server/directives/ExecutionDirectives.scala @@ -23,7 +23,7 @@ trait ExecutionDirectives { ctx ⇒ import ctx.executionContext def handleException: PartialFunction[Throwable, Future[RouteResult]] = - handler andThen (_(ctx.withContentNegotiationDisabled)) + handler andThen (_(ctx.withAcceptAll)) try innerRouteBuilder(())(ctx).fast.recoverWith(handleException) catch { case NonFatal(e) ⇒ handleException.applyOrElse[Throwable, Future[RouteResult]](e, throw _) @@ -42,7 +42,7 @@ trait ExecutionDirectives { val errorMsg = "The RejectionHandler for %s must not itself produce rejections (received %s)!" recoverRejections(r ⇒ sys.error(errorMsg.format(filteredRejections, r))) { handler(filteredRejections) - }(ctx.withContentNegotiationDisabled) + }(ctx.withAcceptAll) } else FastFuture.successful(RouteResult.Rejected(filteredRejections)) } }