From b2ce40ab57ba7550f109daa73970370ab9957a7d Mon Sep 17 00:00:00 2001 From: "John A. De Goes" Date: Thu, 27 Jul 2023 13:13:37 +0100 Subject: [PATCH 1/2] cancel HttpError --- .../http/benchmarks/EndpointBenchmark.scala | 2 +- .../src/main/scala/zio/http/Handler.scala | 47 ++++-- .../main/scala/zio/http/HandlerAspect.scala | 22 +-- .../src/main/scala/zio/http/HttpError.scala | 146 ------------------ .../src/main/scala/zio/http/Response.scala | 40 ----- zio-http/src/main/scala/zio/http/Route.scala | 18 +-- zio-http/src/main/scala/zio/http/Routes.scala | 2 +- .../netty/server/ServerInboundHandler.scala | 5 +- .../test/scala/zio/http/HttpErrorSpec.scala | 41 ----- .../internal/middlewares/MetricsSpec.scala | 2 +- .../middlewares/RequestLoggingSpec.scala | 2 +- .../http/internal/middlewares/WebSpec.scala | 8 +- 12 files changed, 56 insertions(+), 279 deletions(-) delete mode 100644 zio-http/src/main/scala/zio/http/HttpError.scala delete mode 100644 zio-http/src/test/scala/zio/http/HttpErrorSpec.scala diff --git a/zio-http-benchmarks/src/main/scala-2.13/zio/http/benchmarks/EndpointBenchmark.scala b/zio-http-benchmarks/src/main/scala-2.13/zio/http/benchmarks/EndpointBenchmark.scala index 381f827ca6..1087b8c1f0 100644 --- a/zio-http-benchmarks/src/main/scala-2.13/zio/http/benchmarks/EndpointBenchmark.scala +++ b/zio-http-benchmarks/src/main/scala-2.13/zio/http/benchmarks/EndpointBenchmark.scala @@ -227,7 +227,7 @@ class EndpointBenchmark { Method.GET / "first" / int("id1") / "second" / int("id2") / "third" / int("id3") / "fourth" / int( "id4", ) / "fifth" / int("id5") / "sixth" / int("id6") / "seventh" / int("id7") -> - handler { (id1: Int, id2: Int, id3: Int, id4: Int, id5: Int, id6: Int, id7: Int, req: Request) => + handler { (_: Int, _: Int, _: Int, _: Int, _: Int, _: Int, _: Int, _: Request) => ZIO.succeed(Response.ok) }, ).toHttpApp diff --git a/zio-http/src/main/scala/zio/http/Handler.scala b/zio-http/src/main/scala/zio/http/Handler.scala index 11101b1054..3ab42b383c 100644 --- a/zio-http/src/main/scala/zio/http/Handler.scala +++ b/zio-http/src/main/scala/zio/http/Handler.scala @@ -709,11 +709,17 @@ object Handler { } } + /** + * Creates a handler which always responds with a 400 status code. + */ + def badRequest: Handler[Any, Nothing, Any, Response] = + error(Status.BadRequest) + /** * Creates a handler which always responds with a 400 status code. */ def badRequest(message: => String): Handler[Any, Nothing, Any, Response] = - error(HttpError.BadRequest(message)) + error(Status.BadRequest, message) /** * Returns a handler that dies with the specified `Throwable`. This method can @@ -733,16 +739,16 @@ object Handler { die(new RuntimeException(message)) /** - * Creates a handler with HttpError. + * Creates a handler with an error and the default error message. */ - def error(error: => HttpError): Handler[Any, Nothing, Any, Response] = - response(Response.fromHttpError(error)) + def error(status: => Status): Handler[Any, Nothing, Any, Response] = + response(Response.error(status)) /** - * Creates a handler that responds with 500 status code + * Creates a handler with an error and the specified error message. */ - def errorMessage(message: => String): Handler[Any, Nothing, Any, Response] = - error(HttpError.InternalServerError(message)) + def error(status: => Status, message: => String): Handler[Any, Nothing, Any, Response] = + response(Response.error(status, message)) /** * Creates a Handler that always fails @@ -767,11 +773,17 @@ object Handler { } } + /** + * Creates a handler that responds with 403 - Forbidden status code + */ + def forbidden: Handler[Any, Nothing, Any, Response] = + error(Status.Forbidden) + /** * Creates a handler that responds with 403 - Forbidden status code */ def forbidden(message: => String): Handler[Any, Nothing, Any, Response] = - error(HttpError.Forbidden(message)) + error(Status.Forbidden, message) def from[H](handler: => H)(implicit h: ToHandler[H]): Handler[h.Env, h.Err, h.In, h.Out] = h.toHandler(handler) @@ -978,11 +990,23 @@ object Handler { override def apply(in: A): ZIO[Any, Nothing, A] = Exit.succeed(in) } + def internalServerError: Handler[Any, Nothing, Any, Response] = + error(Status.InternalServerError) + + def internalServerError(message: => String): Handler[Any, Nothing, Any, Response] = + error(Status.InternalServerError, message) + + /** + * Creates a handler which always responds with a 405 status code. + */ + def methodNotAllowed: Handler[Any, Nothing, Any, Response] = + error(Status.MethodNotAllowed) + /** * Creates a handler which always responds with a 405 status code. */ def methodNotAllowed(message: => String): Handler[Any, Nothing, Any, Response] = - error(HttpError.MethodNotAllowed(message)) + error(Status.MethodNotAllowed, message) /** * Creates a handler that fails with a NotFound exception. @@ -990,9 +1014,12 @@ object Handler { def notFound: Handler[Any, Nothing, Request, Response] = Handler .fromFunctionHandler[Request] { request => - error(HttpError.NotFound(request.url.path.encode)) + error(Status.NotFound, request.url.path.encode) } + def notFound(message: => String): Handler[Any, Nothing, Any, Response] = + error(Status.NotFound, message) + /** * Creates a handler which always responds with a 200 status code. */ diff --git a/zio-http/src/main/scala/zio/http/HandlerAspect.scala b/zio-http/src/main/scala/zio/http/HandlerAspect.scala index 8b0a62fa2b..aae3bf3ef5 100644 --- a/zio-http/src/main/scala/zio/http/HandlerAspect.scala +++ b/zio-http/src/main/scala/zio/http/HandlerAspect.scala @@ -906,7 +906,7 @@ private[http] trait HandlerAspects extends zio.http.internal.HeaderModifier[Hand private def replaceErrorResponse(request: Request, response: Response): Response = { def htmlResponse: Body = { - val message: String = response.httpError.map(_.message).getOrElse("") + val message: String = response.header(Header.Warning).map(_.text).getOrElse("") val data = Template.container(s"${response.status}") { div( div( @@ -914,11 +914,6 @@ private[http] trait HandlerAspects extends zio.http.internal.HeaderModifier[Hand div(s"${response.status.code}", styles := Seq("font-size" -> "20em")), div(message), ), - div( - response.httpError.get.foldCause(div()) { throwable => - div(h3("Cause:"), pre(prettify(throwable))) - }, - ), ) } Body.fromString("" + data.encode) @@ -947,23 +942,12 @@ private[http] trait HandlerAspects extends zio.http.internal.HeaderModifier[Hand response } - private def prettify(throwable: Throwable): String = { - val sw = new StringWriter - throwable.printStackTrace(new PrintWriter(sw)) - s"${sw.toString}" - } - - private def formatCause(response: Response): String = - response.httpError.get.foldCause("") { throwable => - s"${scala.Console.BOLD}Cause: ${scala.Console.RESET}\n ${prettify(throwable)}" - } - private def formatErrorMessage(response: Response) = { - val errorMessage: String = response.httpError.map(_.message).getOrElse("") + val errorMessage: String = response.header(Header.Warning).map(_.text).getOrElse("") val status = response.status.code s"${scala.Console.BOLD}${scala.Console.RED}${response.status} ${scala.Console.RESET} - " + s"${scala.Console.BOLD}${scala.Console.CYAN}$status ${scala.Console.RESET} - " + - s"$errorMessage\n${formatCause(response)}" + s"$errorMessage" } private[http] val defaultBoundaries = MetricKeyType.Histogram.Boundaries.fromChunk( diff --git a/zio-http/src/main/scala/zio/http/HttpError.scala b/zio-http/src/main/scala/zio/http/HttpError.scala deleted file mode 100644 index 860713536c..0000000000 --- a/zio-http/src/main/scala/zio/http/HttpError.scala +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Copyright 2021 - 2023 Sporta Technologies PVT LTD & the ZIO HTTP contributors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package zio.http - -import zio.http.HttpError.HTTPErrorWithCause -import zio.http.Response -import zio.http.internal.OutputEncoder - -sealed abstract class HttpError(val status: Status, val message: String) extends Throwable(message) { self => - def foldCause[A](a: A)(f: Throwable => A): A = self match { - case error: HTTPErrorWithCause => - error.cause match { - case Some(throwable) => f(throwable) - case None => a - } - case _ => a - - } - - def toResponse: Response = Response.fromHttpError(self) -} - -object HttpError { - def unapply(err: Throwable): Option[(Status, String)] = err match { - case err: HttpError => Option((err.status, err.getMessage)) - case _ => None - } - - sealed abstract class HTTPErrorWithCause(status: Status, msg: String) extends HttpError(status, msg) { - def cause: Option[Throwable] - cause.foreach(initCause) - } - - final case class BadRequest(msg: String = "Bad Request") extends HttpError(Status.BadRequest, msg) - - final case class Unauthorized(msg: String = "Unauthorized") extends HttpError(Status.Unauthorized, msg) - - final case class PaymentRequired(msg: String = "Payment Required") extends HttpError(Status.PaymentRequired, msg) - - final case class Forbidden(msg: String = "Forbidden") extends HttpError(Status.Forbidden, msg) - - final case class NotFound(path: String) - extends HttpError( - Status.NotFound, - s"""The requested URI "${OutputEncoder.encodeHtml(path)}" was not found on this server\n""", - ) - - final case class MethodNotAllowed(msg: String = "Method Not Allowed") extends HttpError(Status.MethodNotAllowed, msg) - - final case class NotAcceptable(msg: String = "Not Acceptable") extends HttpError(Status.NotAcceptable, msg) - - final case class ProxyAuthenticationRequired(msg: String = "Proxy Authentication Required") - extends HttpError(Status.ProxyAuthenticationRequired, msg) - - final case class Conflict(msg: String = "Conflict") extends HttpError(Status.Conflict, msg) - - final case class Gone(msg: String = "Gone") extends HttpError(Status.Gone, msg) - - final case class LengthRequired(msg: String = "Length Required") extends HttpError(Status.LengthRequired, msg) - - final case class PreconditionFailed(msg: String = "Precondition Failed") - extends HttpError(Status.PreconditionFailed, msg) - - final case class RequestTimeout(msg: String = "Request Timeout") extends HttpError(Status.RequestTimeout, msg) - - final case class RequestEntityTooLarge(msg: String = "Request Entity Too Large") - extends HttpError(Status.RequestEntityTooLarge, msg) - - final case class RequestUriTooLong(msg: String = "Request-URI Too Long") - extends HttpError(Status.RequestUriTooLong, msg) - - final case class UnsupportedMediaType(msg: String = "Unsupported Media Type") - extends HttpError(Status.UnsupportedMediaType, msg) - - final case class RequestedRangeNotSatisfiable(msg: String = "Requested Range Not Satisfiable") - extends HttpError(Status.RequestedRangeNotSatisfiable, msg) - - final case class ExpectationFailed(msg: String = "Expectation Failed") - extends HttpError(Status.ExpectationFailed, msg) - - final case class MisdirectedRequest(msg: String = "Misdirected Request") - extends HttpError(Status.MisdirectedRequest, msg) - - final case class UnprocessableEntity(msg: String = "Unprocessable Entity") - extends HttpError(Status.UnprocessableEntity, msg) - - final case class Locked(msg: String = "Locked") extends HttpError(Status.Locked, msg) - - final case class FailedDependency(msg: String = "Failed Dependency") extends HttpError(Status.FailedDependency, msg) - - final case class UnorderedCollection(msg: String = "Unordered Collection") - extends HttpError(Status.UnorderedCollection, msg) - - final case class UpgradeRequired(msg: String = "Upgrade Required") extends HttpError(Status.UpgradeRequired, msg) - - final case class PreconditionRequired(msg: String = "Precondition Required") - extends HttpError(Status.PreconditionRequired, msg) - - final case class TooManyRequests(msg: String = "Too Many Requests") extends HttpError(Status.TooManyRequests, msg) - - final case class RequestHeaderFieldsTooLarge(msg: String = "Request Header Fields Too Large") - extends HttpError(Status.RequestHeaderFieldsTooLarge, msg) - - final case class GatewayTimeout(msg: String = "Gateway Timeout") extends HttpError(Status.GatewayTimeout, msg) - - final case class VariantAlsoNegotiates(msg: String = "Variant Also Negotiates") - extends HttpError(Status.VariantAlsoNegotiates, msg) - - final case class InsufficientStorage(msg: String = "Insufficient Storage") - extends HttpError(Status.InsufficientStorage, msg) - - final case class NotExtended(msg: String = "Not Extended") extends HttpError(Status.NotExtended, msg) - - final case class NetworkAuthenticationRequired(msg: String = "Network Authentication Required") - extends HttpError(Status.NetworkAuthenticationRequired, msg) - - final case class InternalServerError(msg: String = "Internal Server Error", cause: Option[Throwable] = None) - extends HTTPErrorWithCause(Status.InternalServerError, msg) - - final case class NotImplemented(msg: String = "Not Implemented") extends HttpError(Status.NotImplemented, msg) - - final case class HttpVersionNotSupported(msg: String = "HTTP Version Not Supported") - extends HttpError(Status.HttpVersionNotSupported, msg) - - final case class ServiceUnavailable(msg: String = "Service Unavailable") - extends HttpError(Status.ServiceUnavailable, msg) - - final case class BadGateway(msg: String = "Bad Gateway") extends HttpError(Status.BadGateway, msg) - - final case class Custom(code: Int, reason: String) extends HttpError(Status.Custom(code), reason) - -} diff --git a/zio-http/src/main/scala/zio/http/Response.scala b/zio-http/src/main/scala/zio/http/Response.scala index 4bd5f7efb5..5db103aa3f 100644 --- a/zio-http/src/main/scala/zio/http/Response.scala +++ b/zio-http/src/main/scala/zio/http/Response.scala @@ -94,11 +94,6 @@ sealed trait Response extends HeaderOps[Response] { self => def headers: Headers - private[zio] final def httpError: Option[HttpError] = self match { - case Response.GetError(error) => Some(error) - case _ => None - } - /** Consumes the streaming body fully and then drops it */ final def ignoreBody: ZIO[Any, Throwable, Response] = self.collect.map(_.copy(body = Body.empty)) @@ -150,13 +145,6 @@ object Response { } } - object GetError { - def unapply(response: Response): Option[HttpError] = response match { - case resp: ErrorResponse => Some(resp.httpError0) - case _ => None - } - } - private[zio] trait CloseableResponse extends Response { def close(implicit trace: Trace): Task[Unit] } @@ -228,32 +216,6 @@ object Response { } - private[zio] class ErrorResponse(val body: Body, val headers: Headers, val httpError0: HttpError, val status: Status) - extends Response { self => - - override def copy(status: Status, headers: Headers, body: Body): Response = - new ErrorResponse(body, headers, httpError0, status) with InternalState { - override val parent: Response = self - } - - override def freeze: Response = new ErrorResponse(body, headers, httpError0, status) with InternalState { - override val parent: Response = self - override def frozen: Boolean = true - } - - override def toString(): String = - s"ErrorResponse(status = $status, headers = $headers, body = $body, error = $httpError0)" - - override final def updateHeaders(update: Headers => Headers): Response = copy(headers = update(headers)) - - override final def serverTime: Response = new ErrorResponse(body, headers, httpError0, status) with InternalState { - - override val parent: Response = self - - override def addServerTime: Boolean = true - } - } - private[zio] class NativeResponse( val body: Body, val headers: Headers, @@ -383,8 +345,6 @@ object Response { } } - def fromHttpError(error: HttpError): Response = new ErrorResponse(Body.empty, Headers.empty, error, error.status) - /** * Creates a response with content-type set to text/event-stream * @param data diff --git a/zio-http/src/main/scala/zio/http/Route.scala b/zio-http/src/main/scala/zio/http/Route.scala index 2168b1aca7..ec97f7a173 100644 --- a/zio-http/src/main/scala/zio/http/Route.scala +++ b/zio-http/src/main/scala/zio/http/Route.scala @@ -34,14 +34,6 @@ import zio.http.Route.Provided sealed trait Route[-Env, +Err] { self => import Route.{Augmented, Handled, Provided, Unhandled} - /** - * Augments this route with the specified aspect. - */ - def @@[Env1 <: Env]( - aspect: Handler[Env1, Response, Request, Response] => Handler[Env1, Response, Request, Response], - ): Route[Env1, Err] = - Route.Augmented[Env1, Err](self, aspect) - /** * Applies the route to the specified request. The route must be defined for * the request, or else this method will fail fatally. Note that you may only @@ -53,11 +45,6 @@ sealed trait Route[-Env, +Err] { self => def asErrorType[Err2](implicit ev: Err <:< Err2): Route[Env, Err2] = self.asInstanceOf[Route[Env, Err2]] - def augmented[Env1 <: Env]( - f: Handler[Env1, Response, Request, Response] => Handler[Env1, Response, Request, Response], - ): Route[Env1, Err] = - Route.Augmented(self, f) - /** * Handles the error of the route. This method can be used to convert a route * that does not handle its errors into one that does handle its errors. @@ -126,6 +113,11 @@ sealed trait Route[-Env, +Err] { self => def toHandler(implicit ev: Err <:< Response): Handler[Env, Response, Request, Response] final def toHttpApp(implicit ev: Err <:< Response): HttpApp[Env] = Routes(self).toHttpApp + + def transform[Env1 <: Env]( + f: Handler[Env1, Response, Request, Response] => Handler[Env1, Response, Request, Response], + ): Route[Env1, Err] = + Route.Augmented(self, f) } object Route { diff --git a/zio-http/src/main/scala/zio/http/Routes.scala b/zio-http/src/main/scala/zio/http/Routes.scala index cceab2d5e7..3af43d49c5 100644 --- a/zio-http/src/main/scala/zio/http/Routes.scala +++ b/zio-http/src/main/scala/zio/http/Routes.scala @@ -111,7 +111,7 @@ final class Routes[-Env, +Err] private (val routes: Chunk[zio.http.Route[Env, Er def transform[Env1 <: Env]( f: Handler[Env1, Response, Request, Response] => Handler[Env1, Response, Request, Response], ): Routes[Env1, Err] = - new Routes(routes.map(_.@@(f))) + new Routes(routes.map(_.transform(f))) } object Routes { diff --git a/zio-http/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala b/zio-http/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala index a8ab82b28a..e77742a0aa 100644 --- a/zio-http/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala +++ b/zio-http/src/main/scala/zio/http/netty/server/ServerInboundHandler.scala @@ -319,7 +319,7 @@ private[zio] final case class ServerInboundHandler( // TODO: this can be done without ZIO runtime.run(ctx, ensured) { for { - response <- ZIO.succeed(HttpError.NotFound(jReq.uri()).toResponse) + response <- ZIO.succeed(Response.notFound(jReq.uri())) done <- ZIO.attempt(attemptFastWrite(ctx, response, time)) _ <- attemptFullWrite(ctx, response, jReq, time).unless(done) } yield () @@ -381,7 +381,8 @@ private[zio] final case class ServerInboundHandler( }.unit.orDie private def withDefaultErrorResponse(responseOrNull: Response, cause: Option[Throwable]): Response = - if (responseOrNull ne null) responseOrNull else HttpError.InternalServerError(cause = cause).toResponse + if (responseOrNull ne null) responseOrNull + else Response.internalServerError(cause.map(_.getMessage()).getOrElse("")) } object ServerInboundHandler { diff --git a/zio-http/src/test/scala/zio/http/HttpErrorSpec.scala b/zio-http/src/test/scala/zio/http/HttpErrorSpec.scala deleted file mode 100644 index a8c8ccb13d..0000000000 --- a/zio-http/src/test/scala/zio/http/HttpErrorSpec.scala +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2021 - 2023 Sporta Technologies PVT LTD & the ZIO HTTP contributors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package zio.http - -import zio.test.Assertion.equalTo -import zio.test.{ZIOSpecDefault, assert} - -object HttpErrorSpec extends ZIOSpecDefault { - def spec = suite("HttpError")( - suite("foldCause")( - test("should fold the cause") { - val error = HttpError.InternalServerError(cause = Option(new Error("Internal server error"))) - val result = error.foldCause("")(cause => cause.getMessage) - assert(result)(equalTo("Internal server error")) - }, - test("should fold with no cause") { - val error = HttpError.NotFound(Root.encode) - val result = error.foldCause("Page not found")(cause => cause.getMessage) - assert(result)(equalTo("Page not found")) - }, - test("should create custom error") { - val error = HttpError.Custom(451, "Unavailable for legal reasons.") - assert(error.status)(equalTo(Status.Custom(451))) - }, - ), - ) -} diff --git a/zio-http/src/test/scala/zio/http/internal/middlewares/MetricsSpec.scala b/zio-http/src/test/scala/zio/http/internal/middlewares/MetricsSpec.scala index e75f03eb93..bfad221f69 100644 --- a/zio-http/src/test/scala/zio/http/internal/middlewares/MetricsSpec.scala +++ b/zio-http/src/test/scala/zio/http/internal/middlewares/MetricsSpec.scala @@ -31,7 +31,7 @@ object MetricsSpec extends ZIOSpecDefault with HttpAppTestExtensions { test("http_requests_total & http_errors_total") { val app = Routes( Method.GET / "ok" -> Handler.ok, - Method.GET / "error" -> Handler.error(HttpError.InternalServerError()), + Method.GET / "error" -> Handler.internalServerError, Method.GET / "fail" -> Handler.fail(Response.status(Status.Forbidden)), Method.GET / "defect" -> Handler.die(new Throwable("boom")), ).toHttpApp @@ metrics( diff --git a/zio-http/src/test/scala/zio/http/internal/middlewares/RequestLoggingSpec.scala b/zio-http/src/test/scala/zio/http/internal/middlewares/RequestLoggingSpec.scala index 1e29ee6c7d..358ad423d2 100644 --- a/zio-http/src/test/scala/zio/http/internal/middlewares/RequestLoggingSpec.scala +++ b/zio-http/src/test/scala/zio/http/internal/middlewares/RequestLoggingSpec.scala @@ -27,7 +27,7 @@ object RequestLoggingSpec extends ZIOSpecDefault with HttpAppTestExtensions { private val app = Routes( Method.GET / "ok" -> Handler.ok, - Method.GET / "error" -> Handler.error(HttpError.InternalServerError()), + Method.GET / "error" -> Handler.internalServerError, Method.GET / "fail" -> Handler.fail(Response.status(Status.Forbidden)), Method.GET / "defect" -> Handler.die(new Throwable("boom")), ).sandbox.toHttpApp diff --git a/zio-http/src/test/scala/zio/http/internal/middlewares/WebSpec.scala b/zio-http/src/test/scala/zio/http/internal/middlewares/WebSpec.scala index ccba4388c9..2a186c92ca 100644 --- a/zio-http/src/test/scala/zio/http/internal/middlewares/WebSpec.scala +++ b/zio-http/src/test/scala/zio/http/internal/middlewares/WebSpec.scala @@ -278,12 +278,12 @@ object WebSpec extends ZIOSpecDefault with HttpAppTestExtensions { self => ), suite("prettify error")( test("should not add anything to the body as request do not have an accept header") { - val app = (Handler.errorMessage("Error !!!") @@ beautifyErrors) rawHeader "content-type" + val app = (Handler.internalServerError("Error !!!") @@ beautifyErrors) rawHeader "content-type" assertZIO(app.runZIO(Request.get(URL.empty)))(isNone) }, test("should return a html body as the request has accept header set to text/html.") { val app = (Handler - .errorMessage("Error !!!") @@ beautifyErrors) rawHeader "content-type" + .internalServerError("Error !!!") @@ beautifyErrors) rawHeader "content-type" assertZIO( app.runZIO( Request.get(URL.empty).copy(headers = Headers(Header.Accept(MediaType.text.`html`))), @@ -292,7 +292,7 @@ object WebSpec extends ZIOSpecDefault with HttpAppTestExtensions { self => }, test("should return a plain body as the request has accept header set to */*.") { val app = (Handler - .errorMessage("Error !!!") @@ beautifyErrors) rawHeader "content-type" + .internalServerError("Error !!!") @@ beautifyErrors) rawHeader "content-type" assertZIO( app.runZIO( Request @@ -303,7 +303,7 @@ object WebSpec extends ZIOSpecDefault with HttpAppTestExtensions { self => }, test("should not add anything to the body as the request has accept header set to application/json.") { val app = (Handler - .errorMessage("Error !!!") @@ beautifyErrors) rawHeader "content-type" + .internalServerError("Error !!!") @@ beautifyErrors) rawHeader "content-type" assertZIO( app.runZIO( Request.get(URL.empty).copy(headers = Headers(Header.Accept(MediaType.application.json))), From 59ead11277306342f86109c0441f991bd15a3383 Mon Sep 17 00:00:00 2001 From: "John A. De Goes" Date: Thu, 27 Jul 2023 14:09:36 +0100 Subject: [PATCH 2/2] fixees --- docs/dsl/response.md | 6 +- docs/dsl/routes.md | 4 +- .../src/main/scala/zio/http/Handler.scala | 4 +- .../src/main/scala/zio/http/Response.scala | 4 +- zio-http/src/main/scala/zio/http/Status.scala | 119 +++++++++--------- .../test/scala/zio/http/SecuritySpec.scala | 35 ------ 6 files changed, 72 insertions(+), 100 deletions(-) delete mode 100644 zio-http/src/test/scala/zio/http/SecuritySpec.scala diff --git a/docs/dsl/response.md b/docs/dsl/response.md index 3934c044e0..24c6825e23 100644 --- a/docs/dsl/response.md +++ b/docs/dsl/response.md @@ -66,12 +66,12 @@ Response.text("Hello World!").status(Status.NOT_FOUND) Response.ok.updateHeaders(_ => Headers("key", "value")) ``` -### Response from HttpError +### Response from Http Errors -`fromHttpError` creates a response with provided `HttpError` +`error` creates a response with a provided status code and message. ```scala mdoc - Response.fromHttpError(HttpError.BadRequest()) + Response.error(Status.BadRequest, "It's not good!") ``` ## Adding Cookie to Response diff --git a/docs/dsl/routes.md b/docs/dsl/routes.md index dd75fcca67..dbd442e86a 100644 --- a/docs/dsl/routes.md +++ b/docs/dsl/routes.md @@ -93,10 +93,10 @@ Handler.status(Status.Ok) ### Handler.error -Creates a `Handler` that always fails with the given `HttpError`. +Creates a `Handler` that always fails with the given error. ```scala mdoc:silent -Handler.error(HttpError.Forbidden()) +Handler.error(Status.Forbidden) ``` ### Handler.response diff --git a/zio-http/src/main/scala/zio/http/Handler.scala b/zio-http/src/main/scala/zio/http/Handler.scala index 3ab42b383c..74d3ce721d 100644 --- a/zio-http/src/main/scala/zio/http/Handler.scala +++ b/zio-http/src/main/scala/zio/http/Handler.scala @@ -741,13 +741,13 @@ object Handler { /** * Creates a handler with an error and the default error message. */ - def error(status: => Status): Handler[Any, Nothing, Any, Response] = + def error(status: => Status.Error): Handler[Any, Nothing, Any, Response] = response(Response.error(status)) /** * Creates a handler with an error and the specified error message. */ - def error(status: => Status, message: => String): Handler[Any, Nothing, Any, Response] = + def error(status: => Status.Error, message: => String): Handler[Any, Nothing, Any, Response] = response(Response.error(status, message)) /** diff --git a/zio-http/src/main/scala/zio/http/Response.scala b/zio-http/src/main/scala/zio/http/Response.scala index 5db103aa3f..88296bb88c 100644 --- a/zio-http/src/main/scala/zio/http/Response.scala +++ b/zio-http/src/main/scala/zio/http/Response.scala @@ -303,7 +303,7 @@ object Response { def badRequest(message: String): Response = error(Status.BadRequest, message) - def error(status: Status, message: String): Response = { + def error(status: Status.Error, message: String): Response = { import zio.http.internal.OutputEncoder val message2 = OutputEncoder.encodeHtml(if (message == null) status.text else message) @@ -311,7 +311,7 @@ object Response { Response(status = status, headers = Headers(Header.Warning(status.code, "ZIO HTTP", message2))) } - def error(status: Status): Response = + def error(status: Status.Error): Response = error(status, status.text) def forbidden: Response = error(Status.Forbidden) diff --git a/zio-http/src/main/scala/zio/http/Status.scala b/zio-http/src/main/scala/zio/http/Status.scala index ae34e38823..ebd5f3cdcf 100644 --- a/zio-http/src/main/scala/zio/http/Status.scala +++ b/zio-http/src/main/scala/zio/http/Status.scala @@ -48,117 +48,124 @@ sealed trait Status extends Product with Serializable { self => } object Status { - case object Continue extends Status { override val code: Int = 100 } + sealed trait Informational extends Status // 100 – 199 + sealed trait Success extends Status // 200 – 299 + sealed trait Redirection extends Status // 300 – 399 + sealed trait Error extends Status // 400 – 499 + sealed trait ClientError extends Error // 400 – 499 + sealed trait ServerError extends Error // 500 – 599 - case object SwitchingProtocols extends Status { override val code: Int = 101 } + case object Continue extends Informational { override val code: Int = 100 } - case object Processing extends Status { override val code: Int = 102 } + case object SwitchingProtocols extends Informational { override val code: Int = 101 } - case object Ok extends Status { override val code: Int = 200 } + case object Processing extends Informational { override val code: Int = 102 } - case object Created extends Status { override val code: Int = 201 } + case object Ok extends Success { override val code: Int = 200 } - case object Accepted extends Status { override val code: Int = 202 } + case object Created extends Success { override val code: Int = 201 } - case object NonAuthoritativeInformation extends Status { override val code: Int = 203 } + case object Accepted extends Success { override val code: Int = 202 } - case object NoContent extends Status { override val code: Int = 204 } + case object NonAuthoritativeInformation extends Success { override val code: Int = 203 } - case object ResetContent extends Status { override val code: Int = 205 } + case object NoContent extends Success { override val code: Int = 204 } - case object PartialContent extends Status { override val code: Int = 206 } + case object ResetContent extends Success { override val code: Int = 205 } - case object MultiStatus extends Status { override val code: Int = 207 } + case object PartialContent extends Success { override val code: Int = 206 } - case object MultipleChoices extends Status { override val code: Int = 300 } + case object MultiStatus extends Success { override val code: Int = 207 } - case object MovedPermanently extends Status { override val code: Int = 301 } + case object MultipleChoices extends Redirection { override val code: Int = 300 } - case object Found extends Status { override val code: Int = 302 } + case object MovedPermanently extends Redirection { override val code: Int = 301 } - case object SeeOther extends Status { override val code: Int = 303 } + case object Found extends Redirection { override val code: Int = 302 } - case object NotModified extends Status { override val code: Int = 304 } + case object SeeOther extends Redirection { override val code: Int = 303 } - case object UseProxy extends Status { override val code: Int = 305 } + case object NotModified extends Redirection { override val code: Int = 304 } - case object TemporaryRedirect extends Status { override val code: Int = 307 } + case object UseProxy extends Redirection { override val code: Int = 305 } - case object PermanentRedirect extends Status { override val code: Int = 308 } + case object TemporaryRedirect extends Redirection { override val code: Int = 307 } - case object BadRequest extends Status { override val code: Int = 400 } + case object PermanentRedirect extends Redirection { override val code: Int = 308 } - case object Unauthorized extends Status { override val code: Int = 401 } + case object BadRequest extends ClientError { override val code: Int = 400 } - case object PaymentRequired extends Status { override val code: Int = 402 } + case object Unauthorized extends ClientError { override val code: Int = 401 } - case object Forbidden extends Status { override val code: Int = 403 } + case object PaymentRequired extends ClientError { override val code: Int = 402 } - case object NotFound extends Status { override val code: Int = 404 } + case object Forbidden extends ClientError { override val code: Int = 403 } - case object MethodNotAllowed extends Status { override val code: Int = 405 } + case object NotFound extends ClientError { override val code: Int = 404 } - case object NotAcceptable extends Status { override val code: Int = 406 } + case object MethodNotAllowed extends ClientError { override val code: Int = 405 } - case object ProxyAuthenticationRequired extends Status { override val code: Int = 407 } + case object NotAcceptable extends ClientError { override val code: Int = 406 } - case object RequestTimeout extends Status { override val code: Int = 408 } + case object ProxyAuthenticationRequired extends ClientError { override val code: Int = 407 } - case object Conflict extends Status { override val code: Int = 409 } + case object RequestTimeout extends ClientError { override val code: Int = 408 } - case object Gone extends Status { override val code: Int = 410 } + case object Conflict extends ClientError { override val code: Int = 409 } - case object LengthRequired extends Status { override val code: Int = 411 } + case object Gone extends ClientError { override val code: Int = 410 } - case object PreconditionFailed extends Status { override val code: Int = 412 } + case object LengthRequired extends ClientError { override val code: Int = 411 } - case object RequestEntityTooLarge extends Status { override val code: Int = 413 } + case object PreconditionFailed extends ClientError { override val code: Int = 412 } - case object RequestUriTooLong extends Status { override val code: Int = 414 } + case object RequestEntityTooLarge extends ClientError { override val code: Int = 413 } - case object UnsupportedMediaType extends Status { override val code: Int = 415 } + case object RequestUriTooLong extends ClientError { override val code: Int = 414 } - case object RequestedRangeNotSatisfiable extends Status { override val code: Int = 416 } + case object UnsupportedMediaType extends ClientError { override val code: Int = 415 } - case object ExpectationFailed extends Status { override val code: Int = 417 } + case object RequestedRangeNotSatisfiable extends ClientError { override val code: Int = 416 } - case object MisdirectedRequest extends Status { override val code: Int = 421 } + case object ExpectationFailed extends ClientError { override val code: Int = 417 } - case object UnprocessableEntity extends Status { override val code: Int = 422 } + case object MisdirectedRequest extends ClientError { override val code: Int = 421 } - case object Locked extends Status { override val code: Int = 423 } + case object UnprocessableEntity extends ClientError { override val code: Int = 422 } - case object FailedDependency extends Status { override val code: Int = 424 } + case object Locked extends ClientError { override val code: Int = 423 } - case object UnorderedCollection extends Status { override val code: Int = 425 } + case object FailedDependency extends ClientError { override val code: Int = 424 } - case object UpgradeRequired extends Status { override val code: Int = 426 } + case object UnorderedCollection extends ClientError { override val code: Int = 425 } - case object PreconditionRequired extends Status { override val code: Int = 428 } + case object UpgradeRequired extends ClientError { override val code: Int = 426 } - case object TooManyRequests extends Status { override val code: Int = 429 } + case object PreconditionRequired extends ClientError { override val code: Int = 428 } - case object RequestHeaderFieldsTooLarge extends Status { override val code: Int = 431 } + case object TooManyRequests extends ClientError { override val code: Int = 429 } - case object InternalServerError extends Status { override val code: Int = 500 } + case object RequestHeaderFieldsTooLarge extends ClientError { override val code: Int = 431 } - case object NotImplemented extends Status { override val code: Int = 501 } + case object InternalServerError extends ServerError { override val code: Int = 500 } - case object BadGateway extends Status { override val code: Int = 502 } + case object NotImplemented extends ServerError { override val code: Int = 501 } - case object ServiceUnavailable extends Status { override val code: Int = 503 } + case object BadGateway extends ServerError { override val code: Int = 502 } - case object GatewayTimeout extends Status { override val code: Int = 504 } + case object ServiceUnavailable extends ServerError { override val code: Int = 503 } - case object HttpVersionNotSupported extends Status { override val code: Int = 505 } + case object GatewayTimeout extends ServerError { override val code: Int = 504 } - case object VariantAlsoNegotiates extends Status { override val code: Int = 506 } + case object HttpVersionNotSupported extends ServerError { override val code: Int = 505 } - case object InsufficientStorage extends Status { override val code: Int = 507 } + case object VariantAlsoNegotiates extends ServerError { override val code: Int = 506 } - case object NotExtended extends Status { override val code: Int = 510 } + case object InsufficientStorage extends ServerError { override val code: Int = 507 } - case object NetworkAuthenticationRequired extends Status { override val code: Int = 511 } + case object NotExtended extends ServerError { override val code: Int = 510 } + + case object NetworkAuthenticationRequired extends ServerError { override val code: Int = 511 } final case class Custom(override val code: Int) extends Status diff --git a/zio-http/src/test/scala/zio/http/SecuritySpec.scala b/zio-http/src/test/scala/zio/http/SecuritySpec.scala deleted file mode 100644 index 3c8a81d361..0000000000 --- a/zio-http/src/test/scala/zio/http/SecuritySpec.scala +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2021 - 2023 Sporta Technologies PVT LTD & the ZIO HTTP contributors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package zio.http - -import zio.test.Assertion.equalTo -import zio.test.{Spec, ZIOSpecDefault, assert} - -object SecuritySpec extends ZIOSpecDefault { - def spec: Spec[Any, Nothing] = suite("HttpError")( - suite("security")( - test("should encode HTML output, to protect against XSS") { - val error = HttpError.NotFound("").message - assert(error)( - equalTo( - "The requested URI \"<script>alert("xss")</script>\" was not found on this server\n", - ), - ) - }, - ), - ) -}