From 7d222e2339a6e3292cc50e1206cfa471540741e6 Mon Sep 17 00:00:00 2001 From: Billy Autrey <40704452+BillyAutrey@users.noreply.github.com> Date: Tue, 31 May 2022 17:41:49 -0500 Subject: [PATCH 01/10] Change obj refs to DefaultHttpErrorHandler to class refs --- .../src/main/scala/play/core/server/AkkaHttpServer.scala | 2 +- .../scala/play/core/server/netty/PlayRequestHandler.scala | 2 +- .../play-server/src/main/scala/play/core/server/Server.scala | 2 +- .../scala/play/core/server/common/ServerResultUtils.scala | 2 +- .../src/main/scala/play/filters/cors/CORSActionBuilder.scala | 2 +- .../src/main/scala/play/filters/cors/CORSFilter.scala | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala index e7ccfc07c01..5a73ee0c432 100644 --- a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala +++ b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala @@ -345,7 +345,7 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server { // Get the app's HttpErrorHandler or fallback to a default value val errorHandler: HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => DefaultHttpErrorHandler + case Failure(_) => new DefaultHttpErrorHandler() } // default execution context used for executing the action diff --git a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala index e176f31fcfd..149f66c0ded 100644 --- a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala +++ b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala @@ -330,7 +330,7 @@ private[play] class PlayRequestHandler( private def errorHandler(tryApp: Try[Application]): HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => DefaultHttpErrorHandler + case Failure(_) => new DefaultHttpErrorHandler() } /** diff --git a/transport/server/play-server/src/main/scala/play/core/server/Server.scala b/transport/server/play-server/src/main/scala/play/core/server/Server.scala index 10dfe6a1ef7..8e61d81f88f 100644 --- a/transport/server/play-server/src/main/scala/play/core/server/Server.scala +++ b/transport/server/play-server/src/main/scala/play/core/server/Server.scala @@ -129,7 +129,7 @@ object Server { handleErrors(application.errorHandler, enrichedRequest) } } catch { - handleErrors(DefaultHttpErrorHandler, request) + handleErrors(new DefaultHttpErrorHandler(), request) } } diff --git a/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala b/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala index 965f8205626..390c66723b4 100644 --- a/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala +++ b/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala @@ -176,7 +176,7 @@ private[play] final class ServerResultUtils( // Convert errorResult using normal conversion logic. This time use // the DefaultErrorHandler if there are any problems, e.g. if the // current HttpErrorHandler returns an invalid Result. - resultConversionWithErrorHandling(requestHeader, errorResult, DefaultHttpErrorHandler)(resultConverter)( + resultConversionWithErrorHandling(requestHeader, errorResult, new DefaultHttpErrorHandler())(resultConverter)( fallbackResponse ) } diff --git a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala index 0a2cef0f633..0405606b489 100644 --- a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala +++ b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala @@ -82,7 +82,7 @@ object CORSActionBuilder { */ def apply( config: Configuration, - errorHandler: HttpErrorHandler = DefaultHttpErrorHandler, + errorHandler: HttpErrorHandler = new DefaultHttpErrorHandler(), configPath: String = "play.filters.cors", parserConfig: ParserConfiguration = ParserConfiguration(), tempFileCreator: TemporaryFileCreator = SingletonTemporaryFileCreator diff --git a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala index 259ee2252e7..fbec7762eed 100644 --- a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala +++ b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala @@ -43,7 +43,7 @@ import play.api.mvc._ */ class CORSFilter( protected override val corsConfig: CORSConfig = CORSConfig(), - protected override val errorHandler: HttpErrorHandler = DefaultHttpErrorHandler, + protected override val errorHandler: HttpErrorHandler = new DefaultHttpErrorHandler(), private val pathPrefixes: Seq[String] = Seq("/") ) extends EssentialFilter with AbstractCORSPolicy { @@ -94,7 +94,7 @@ object CORSFilter { def apply( corsConfig: CORSConfig = CORSConfig(), - errorHandler: HttpErrorHandler = DefaultHttpErrorHandler, + errorHandler: HttpErrorHandler = new DefaultHttpErrorHandler(), pathPrefixes: Seq[String] = Seq("/") )(implicit mat: Materializer) = new CORSFilter(corsConfig, errorHandler, pathPrefixes) From b096dc1354f5aed9f499081918f9ce636217bd2f Mon Sep 17 00:00:00 2001 From: Billy Autrey <40704452+BillyAutrey@users.noreply.github.com> Date: Wed, 1 Jun 2022 12:22:00 -0500 Subject: [PATCH 02/10] Introduced a DevErrorHandler, and made the DefaultHttpErrorHandler not leak dev errors --- .../play/api/http/HttpErrorHandler.scala | 21 ++++++++++++++++++- .../play/core/server/AkkaHttpServer.scala | 13 +++++------- .../server/netty/PlayRequestHandler.scala | 8 +++---- .../main/scala/play/core/server/Server.scala | 2 +- .../server/common/ServerResultUtils.scala | 2 +- .../play/filters/cors/CORSActionBuilder.scala | 2 +- .../scala/play/filters/cors/CORSFilter.scala | 2 +- 7 files changed, 33 insertions(+), 17 deletions(-) diff --git a/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala b/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala index 4313b6ae3a0..fbbc04374b3 100644 --- a/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala +++ b/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala @@ -124,6 +124,15 @@ object HttpErrorHandler { case class HttpErrorConfig(showDevErrors: Boolean = false, playEditor: Option[String] = None) +object HttpErrorConfig { + def apply(mode: Mode, playEditor: Option[String] = None): HttpErrorConfig = { + mode match { + case Mode.Prod => HttpErrorConfig(showDevErrors = false, playEditor) + case _ => HttpErrorConfig(showDevErrors = true, playEditor) + } + } +} + /** * The default HTTP error handler. * @@ -521,7 +530,7 @@ class JsonHttpErrorHandler(environment: Environment, sourceMapper: Option[Source * Note: this HttpErrorHandler should ONLY be used in DEV or TEST. The way this displays errors to the user is * generally not suitable for a production environment. */ -object DefaultHttpErrorHandler +object DevHttpErrorHandler extends DefaultHttpErrorHandler(HttpErrorConfig(showDevErrors = true, playEditor = None), None, None) { private val logger = Logger(getClass) private lazy val setEditor: Unit = @@ -545,6 +554,16 @@ object DefaultHttpErrorHandler } } +/** + * A fallback default HTTP error handler that can be used when there's no application available. + * + * Note: this HttpErrorHandler uses the default `HttpErrorConfig`, which does not `showDevErrors`. + * It is largely here to preserve binary compatibility, but should be overridden with an injected + * HttpErrorHandler. + */ +object DefaultHttpErrorHandler + extends DefaultHttpErrorHandler(HttpErrorConfig(), None, None) + /** * A Java error handler that's provided when a Scala one is configured, so that Java code can still have the error * handler injected. diff --git a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala index 5a73ee0c432..9c90a7650fe 100644 --- a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala +++ b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala @@ -5,7 +5,6 @@ package play.core.server import java.net.InetSocketAddress - import akka.Done import akka.actor.ActorSystem import akka.actor.CoordinatedShutdown @@ -26,14 +25,10 @@ import akka.stream.scaladsl._ import akka.util.ByteString import com.typesafe.config.Config import com.typesafe.config.ConfigMemorySize + import javax.net.ssl._ import play.api._ -import play.api.http.DefaultHttpErrorHandler -import play.api.http.HeaderNames -import play.api.http.HttpErrorHandler -import play.api.http.HttpErrorInfo -import play.api.http.{ HttpProtocol => PlayHttpProtocol } -import play.api.http.Status +import play.api.http.{DefaultHttpErrorHandler, DevHttpErrorHandler, HeaderNames, HttpErrorConfig, HttpErrorHandler, HttpErrorInfo, Status, HttpProtocol => PlayHttpProtocol} import play.api.internal.libs.concurrent.CoordinatedShutdownSupport import play.api.libs.streams.Accumulator import play.api.mvc._ @@ -345,7 +340,9 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server { // Get the app's HttpErrorHandler or fallback to a default value val errorHandler: HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => new DefaultHttpErrorHandler() + case Failure(_) => + if (mode == Mode.Prod) DefaultHttpErrorHandler + else DevHttpErrorHandler } // default execution context used for executing the action diff --git a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala index 149f66c0ded..487b60df36c 100644 --- a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala +++ b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala @@ -6,7 +6,6 @@ package play.core.server.netty import java.io.IOException import java.util.concurrent.atomic.AtomicLong - import akka.stream.Materializer import com.typesafe.config.ConfigMemorySize import com.typesafe.netty.http.DefaultWebSocketHttpResponse @@ -18,8 +17,7 @@ import io.netty.handler.timeout.IdleStateEvent import play.api.http._ import play.api.libs.streams.Accumulator import play.api.mvc._ -import play.api.Application -import play.api.Logger +import play.api.{Application, Logger, Mode} import play.core.server.NettyServer import play.core.server.Server import play.core.server.common.ReloadCache @@ -330,7 +328,9 @@ private[play] class PlayRequestHandler( private def errorHandler(tryApp: Try[Application]): HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => new DefaultHttpErrorHandler() + case Failure(_) => + if (server.mode == Mode.Prod) DefaultHttpErrorHandler + else DevHttpErrorHandler } /** diff --git a/transport/server/play-server/src/main/scala/play/core/server/Server.scala b/transport/server/play-server/src/main/scala/play/core/server/Server.scala index 8e61d81f88f..10dfe6a1ef7 100644 --- a/transport/server/play-server/src/main/scala/play/core/server/Server.scala +++ b/transport/server/play-server/src/main/scala/play/core/server/Server.scala @@ -129,7 +129,7 @@ object Server { handleErrors(application.errorHandler, enrichedRequest) } } catch { - handleErrors(new DefaultHttpErrorHandler(), request) + handleErrors(DefaultHttpErrorHandler, request) } } diff --git a/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala b/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala index 390c66723b4..965f8205626 100644 --- a/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala +++ b/transport/server/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala @@ -176,7 +176,7 @@ private[play] final class ServerResultUtils( // Convert errorResult using normal conversion logic. This time use // the DefaultErrorHandler if there are any problems, e.g. if the // current HttpErrorHandler returns an invalid Result. - resultConversionWithErrorHandling(requestHeader, errorResult, new DefaultHttpErrorHandler())(resultConverter)( + resultConversionWithErrorHandling(requestHeader, errorResult, DefaultHttpErrorHandler)(resultConverter)( fallbackResponse ) } diff --git a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala index 0405606b489..0a2cef0f633 100644 --- a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala +++ b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSActionBuilder.scala @@ -82,7 +82,7 @@ object CORSActionBuilder { */ def apply( config: Configuration, - errorHandler: HttpErrorHandler = new DefaultHttpErrorHandler(), + errorHandler: HttpErrorHandler = DefaultHttpErrorHandler, configPath: String = "play.filters.cors", parserConfig: ParserConfiguration = ParserConfiguration(), tempFileCreator: TemporaryFileCreator = SingletonTemporaryFileCreator diff --git a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala index fbec7762eed..b9392409baa 100644 --- a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala +++ b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala @@ -43,7 +43,7 @@ import play.api.mvc._ */ class CORSFilter( protected override val corsConfig: CORSConfig = CORSConfig(), - protected override val errorHandler: HttpErrorHandler = new DefaultHttpErrorHandler(), + protected override val errorHandler: HttpErrorHandler = DefaultHttpErrorHandler, private val pathPrefixes: Seq[String] = Seq("/") ) extends EssentialFilter with AbstractCORSPolicy { From 209dcd8be28ccd628f37dd729a89b9f9795af792 Mon Sep 17 00:00:00 2001 From: Billy Autrey <40704452+BillyAutrey@users.noreply.github.com> Date: Wed, 1 Jun 2022 12:45:03 -0500 Subject: [PATCH 03/10] Fixing whitespace/compile errors --- .../src/main/scala/play/api/http/HttpErrorHandler.scala | 9 --------- .../src/main/scala/play/core/server/AkkaHttpServer.scala | 4 ++-- .../play/core/server/netty/PlayRequestHandler.scala | 1 + 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala b/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala index fbbc04374b3..765f30ac1fa 100644 --- a/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala +++ b/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala @@ -124,15 +124,6 @@ object HttpErrorHandler { case class HttpErrorConfig(showDevErrors: Boolean = false, playEditor: Option[String] = None) -object HttpErrorConfig { - def apply(mode: Mode, playEditor: Option[String] = None): HttpErrorConfig = { - mode match { - case Mode.Prod => HttpErrorConfig(showDevErrors = false, playEditor) - case _ => HttpErrorConfig(showDevErrors = true, playEditor) - } - } -} - /** * The default HTTP error handler. * diff --git a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala index 9c90a7650fe..2d0630639b8 100644 --- a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala +++ b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala @@ -5,6 +5,7 @@ package play.core.server import java.net.InetSocketAddress + import akka.Done import akka.actor.ActorSystem import akka.actor.CoordinatedShutdown @@ -25,10 +26,9 @@ import akka.stream.scaladsl._ import akka.util.ByteString import com.typesafe.config.Config import com.typesafe.config.ConfigMemorySize - import javax.net.ssl._ import play.api._ -import play.api.http.{DefaultHttpErrorHandler, DevHttpErrorHandler, HeaderNames, HttpErrorConfig, HttpErrorHandler, HttpErrorInfo, Status, HttpProtocol => PlayHttpProtocol} +import play.api.http.{DefaultHttpErrorHandler, DevHttpErrorHandler, HeaderNames, HttpErrorHandler, HttpErrorInfo, Status, HttpProtocol => PlayHttpProtocol} import play.api.internal.libs.concurrent.CoordinatedShutdownSupport import play.api.libs.streams.Accumulator import play.api.mvc._ diff --git a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala index 487b60df36c..5b12ab0126b 100644 --- a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala +++ b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala @@ -6,6 +6,7 @@ package play.core.server.netty import java.io.IOException import java.util.concurrent.atomic.AtomicLong + import akka.stream.Materializer import com.typesafe.config.ConfigMemorySize import com.typesafe.netty.http.DefaultWebSocketHttpResponse From bbd6c7cab5fd92a99f5e88bf287dc1ef57376222 Mon Sep 17 00:00:00 2001 From: Billy Autrey <40704452+BillyAutrey@users.noreply.github.com> Date: Wed, 1 Jun 2022 12:46:19 -0500 Subject: [PATCH 04/10] Removing stray change from first commit --- .../src/main/scala/play/filters/cors/CORSFilter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala index b9392409baa..259ee2252e7 100644 --- a/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala +++ b/web/play-filters-helpers/src/main/scala/play/filters/cors/CORSFilter.scala @@ -94,7 +94,7 @@ object CORSFilter { def apply( corsConfig: CORSConfig = CORSConfig(), - errorHandler: HttpErrorHandler = new DefaultHttpErrorHandler(), + errorHandler: HttpErrorHandler = DefaultHttpErrorHandler, pathPrefixes: Seq[String] = Seq("/") )(implicit mat: Materializer) = new CORSFilter(corsConfig, errorHandler, pathPrefixes) From ebe23d8931d57cfa793a6f2cbe0e93d2f7184647 Mon Sep 17 00:00:00 2001 From: Billy Autrey <40704452+BillyAutrey@users.noreply.github.com> Date: Wed, 1 Jun 2022 12:59:13 -0500 Subject: [PATCH 05/10] Formatting --- .../main/scala/play/api/http/HttpErrorHandler.scala | 3 +-- .../main/scala/play/core/server/AkkaHttpServer.scala | 10 ++++++++-- .../play/core/server/netty/PlayRequestHandler.scala | 6 ++++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala b/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala index 765f30ac1fa..410b5a3f870 100644 --- a/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala +++ b/core/play/src/main/scala/play/api/http/HttpErrorHandler.scala @@ -552,8 +552,7 @@ object DevHttpErrorHandler * It is largely here to preserve binary compatibility, but should be overridden with an injected * HttpErrorHandler. */ -object DefaultHttpErrorHandler - extends DefaultHttpErrorHandler(HttpErrorConfig(), None, None) +object DefaultHttpErrorHandler extends DefaultHttpErrorHandler(HttpErrorConfig(), None, None) /** * A Java error handler that's provided when a Scala one is configured, so that Java code can still have the error diff --git a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala index 2d0630639b8..1d8bb0866fa 100644 --- a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala +++ b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala @@ -28,7 +28,13 @@ import com.typesafe.config.Config import com.typesafe.config.ConfigMemorySize import javax.net.ssl._ import play.api._ -import play.api.http.{DefaultHttpErrorHandler, DevHttpErrorHandler, HeaderNames, HttpErrorHandler, HttpErrorInfo, Status, HttpProtocol => PlayHttpProtocol} +import play.api.http.DefaultHttpErrorHandler +import play.api.http.DevHttpErrorHandler +import play.api.http.HeaderNames +import play.api.http.HttpErrorHandler +import play.api.http.HttpErrorInfo +import play.api.http.Status +import play.api.http.{ HttpProtocol => PlayHttpProtocol } import play.api.internal.libs.concurrent.CoordinatedShutdownSupport import play.api.libs.streams.Accumulator import play.api.mvc._ @@ -340,7 +346,7 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server { // Get the app's HttpErrorHandler or fallback to a default value val errorHandler: HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => + case Failure(_) => if (mode == Mode.Prod) DefaultHttpErrorHandler else DevHttpErrorHandler } diff --git a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala index 5b12ab0126b..62216a18a09 100644 --- a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala +++ b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala @@ -18,7 +18,9 @@ import io.netty.handler.timeout.IdleStateEvent import play.api.http._ import play.api.libs.streams.Accumulator import play.api.mvc._ -import play.api.{Application, Logger, Mode} +import play.api.Application +import play.api.Logger +import play.api.Mode import play.core.server.NettyServer import play.core.server.Server import play.core.server.common.ReloadCache @@ -329,7 +331,7 @@ private[play] class PlayRequestHandler( private def errorHandler(tryApp: Try[Application]): HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => + case Failure(_) => if (server.mode == Mode.Prod) DefaultHttpErrorHandler else DevHttpErrorHandler } From 37125771538a7e5d2b9e7a00ea39f7b804010e06 Mon Sep 17 00:00:00 2001 From: Billy Autrey <40704452+BillyAutrey@users.noreply.github.com> Date: Wed, 1 Jun 2022 15:14:59 -0500 Subject: [PATCH 06/10] Preserving Server.scala behavior from before --- .../src/main/scala/play/core/server/Server.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/transport/server/play-server/src/main/scala/play/core/server/Server.scala b/transport/server/play-server/src/main/scala/play/core/server/Server.scala index 10dfe6a1ef7..abb361bf6ea 100644 --- a/transport/server/play-server/src/main/scala/play/core/server/Server.scala +++ b/transport/server/play-server/src/main/scala/play/core/server/Server.scala @@ -5,14 +5,13 @@ package play.core.server import java.util.function.{ Function => JFunction } - import akka.actor.CoordinatedShutdown import akka.annotation.ApiMayChange import com.typesafe.config.Config import com.typesafe.config.ConfigFactory import play.api.ApplicationLoader.Context import play.api._ -import play.api.http.DefaultHttpErrorHandler +import play.api.http.DevHttpErrorHandler import play.api.http.HttpErrorHandler import play.api.http.Port import play.api.inject.ApplicationLifecycle @@ -129,7 +128,9 @@ object Server { handleErrors(application.errorHandler, enrichedRequest) } } catch { - handleErrors(DefaultHttpErrorHandler, request) + // TODO: Preserving old functionality here. Don't want to break dev mode, but we *also* should attempt + // to keep dev mode errors out of prod in the future. + handleErrors(DevHttpErrorHandler, request) } } From a5d83283669dd69dbdba1407d231113aee3a02e5 Mon Sep 17 00:00:00 2001 From: Billy Autrey <40704452+BillyAutrey@users.noreply.github.com> Date: Wed, 1 Jun 2022 15:55:26 -0500 Subject: [PATCH 07/10] Changing Server.getHandlerFor to take a parameterized fallback error handler --- .../main/scala/play/core/server/AkkaHttpServer.scala | 11 +++++++---- .../play/core/server/netty/PlayRequestHandler.scala | 11 +++++++---- .../src/main/scala/play/core/server/Server.scala | 10 ++++++---- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala index 1d8bb0866fa..d1eb458abee 100644 --- a/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala +++ b/transport/server/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala @@ -293,6 +293,11 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server { } } + private lazy val fallbackErrorHandler = mode match { + case Mode.Prod => DefaultHttpErrorHandler + case _ => DevHttpErrorHandler + } + private def resultUtils(tryApp: Try[Application]): ServerResultUtils = reloadCache.cachedFrom(tryApp).resultUtils private def modelConversion(tryApp: Try[Application]): AkkaModelConversion = @@ -315,7 +320,7 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server { val debugInfo: Option[ServerDebugInfo] = reloadCache.cachedFrom(tryApp).serverDebugInfo ServerDebugInfo.attachToRequestHeader(convertedRequestHeader, debugInfo) } - val (taggedRequestHeader, handler) = Server.getHandlerFor(debugInfoRequestHeader, tryApp) + val (taggedRequestHeader, handler) = Server.getHandlerFor(debugInfoRequestHeader, tryApp, fallbackErrorHandler) val responseFuture = executeHandler( tryApp, decodedRequest, @@ -346,9 +351,7 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server { // Get the app's HttpErrorHandler or fallback to a default value val errorHandler: HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => - if (mode == Mode.Prod) DefaultHttpErrorHandler - else DevHttpErrorHandler + case Failure(_) => fallbackErrorHandler } // default execution context used for executing the action diff --git a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala index 62216a18a09..b7b94eca6ba 100644 --- a/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala +++ b/transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala @@ -126,7 +126,7 @@ private[play] class PlayRequestHandler( clientError(Status.REQUEST_ENTITY_TOO_LARGE, "Request Entity Too Large") } else { val debugHeader: RequestHeader = attachDebugInfo(untagged) - Server.getHandlerFor(debugHeader, tryApp) + Server.getHandlerFor(debugHeader, tryApp, fallbackErrorHandler) } } @@ -331,11 +331,14 @@ private[play] class PlayRequestHandler( private def errorHandler(tryApp: Try[Application]): HttpErrorHandler = tryApp match { case Success(app) => app.errorHandler - case Failure(_) => - if (server.mode == Mode.Prod) DefaultHttpErrorHandler - else DevHttpErrorHandler + case Failure(_) => fallbackErrorHandler } + private lazy val fallbackErrorHandler = server.mode match { + case Mode.Prod => DefaultHttpErrorHandler + case _ => DevHttpErrorHandler + } + /** * Sends a simple response with no body, then closes the connection. */ diff --git a/transport/server/play-server/src/main/scala/play/core/server/Server.scala b/transport/server/play-server/src/main/scala/play/core/server/Server.scala index abb361bf6ea..92b31a75ba3 100644 --- a/transport/server/play-server/src/main/scala/play/core/server/Server.scala +++ b/transport/server/play-server/src/main/scala/play/core/server/Server.scala @@ -97,7 +97,11 @@ object Server { * i.e. if there's an error loading the application. * - If an exception is thrown. */ - private[server] def getHandlerFor(request: RequestHeader, tryApp: Try[Application]): (RequestHeader, Handler) = { + private[server] def getHandlerFor( + request: RequestHeader, + tryApp: Try[Application], + fallbackErrorHandler: HttpErrorHandler + ): (RequestHeader, Handler) = { @inline def handleErrors( errorHandler: HttpErrorHandler, req: RequestHeader @@ -128,9 +132,7 @@ object Server { handleErrors(application.errorHandler, enrichedRequest) } } catch { - // TODO: Preserving old functionality here. Don't want to break dev mode, but we *also* should attempt - // to keep dev mode errors out of prod in the future. - handleErrors(DevHttpErrorHandler, request) + handleErrors(fallbackErrorHandler, request) } } From 0fefe5e0b6f3fc58b43b93e085bc354bb312e2e7 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 2 Jun 2022 01:07:54 +0200 Subject: [PATCH 08/10] Update dependencies --- project/Dependencies.scala | 8 ++++---- project/plugins.sbt | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 9be032d4ceb..b478f207ab8 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -162,10 +162,10 @@ object Dependencies { sslConfig ) ++ scalaParserCombinators(scalaVersion) ++ specs2Deps.map(_ % Test) ++ javaTestDeps - val nettyVersion = "4.1.75.Final" + val nettyVersion = "4.1.77.Final" val netty = Seq( - "com.typesafe.netty" % "netty-reactive-streams-http" % "2.0.5", + "com.typesafe.netty" % "netty-reactive-streams-http" % "2.0.6", ("io.netty" % "netty-transport-native-epoll" % nettyVersion).classifier("linux-x86_64") ) ++ specs2Deps.map(_ % Test) @@ -173,7 +173,7 @@ object Dependencies { val jimfs = "com.google.jimfs" % "jimfs" % "1.1" - val okHttp = "com.squareup.okhttp3" % "okhttp" % "4.2.2" + val okHttp = "com.squareup.okhttp3" % "okhttp" % "4.2.3" def routesCompilerDependencies(scalaVersion: String) = { val deps = CrossVersion.partialVersion(scalaVersion) match { @@ -288,7 +288,7 @@ object Dependencies { "com.github.ben-manes.caffeine" % "jcache" % caffeineVersion ) ++ jcacheApi - val playWsStandaloneVersion = "2.1.7" + val playWsStandaloneVersion = "2.1.10" val playWsDeps = Seq( "com.typesafe.play" %% "play-ws-standalone" % playWsStandaloneVersion, "com.typesafe.play" %% "play-ws-standalone-xml" % playWsStandaloneVersion, diff --git a/project/plugins.sbt b/project/plugins.sbt index 9d1ec1fcdff..420428b9118 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -5,7 +5,7 @@ enablePlugins(BuildInfoPlugin) // when updating sbtNativePackager version, be sure to also update the documentation links in // documentation/manual/working/commonGuide/production/Deploying.md val sbtNativePackager = "1.5.2" -val mima = "0.8.1" +val mima = "0.9.0" val sbtJavaAgent = "0.1.5" val sbtJavaFormatter = "0.5.0" val sbtJmh = "0.3.7" From 2744c6c0b8600e742f4d22f2cf413f334dc5e051 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 2 Jun 2022 01:39:58 +0200 Subject: [PATCH 09/10] Use DevHttpErrorHandler to check if play.editor link gets rendered --- .../src/it/scala/play/it/views/DevErrorPageSpec.scala | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/play-integration-test/src/it/scala/play/it/views/DevErrorPageSpec.scala b/core/play-integration-test/src/it/scala/play/it/views/DevErrorPageSpec.scala index dfa6febc21d..5fd538bad79 100644 --- a/core/play-integration-test/src/it/scala/play/it/views/DevErrorPageSpec.scala +++ b/core/play-integration-test/src/it/scala/play/it/views/DevErrorPageSpec.scala @@ -4,10 +4,8 @@ package play.it.views -import play.api.Configuration -import play.api.Environment -import play.api.Mode import play.api.http.DefaultHttpErrorHandler +import play.api.http.DevHttpErrorHandler import play.api.test._ class DevErrorPageSpec extends PlaySpecification { @@ -20,8 +18,8 @@ class DevErrorPageSpec extends PlaySpecification { } "link the error line if play.editor is configured" in { - DefaultHttpErrorHandler.setPlayEditor("someEditorLinkWith %s:%s") - val result = DefaultHttpErrorHandler.onServerError(FakeRequest(), testExceptionSource) + DevHttpErrorHandler.setPlayEditor("someEditorLinkWith %s:%s") + val result = DevHttpErrorHandler.onServerError(FakeRequest(), testExceptionSource) contentAsString(result) must contain("""href="someEditorLinkWith someSourceFile:100" """) } From 200a86af8b4d904a62a0b2e1b0f7d34d79f602a2 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 2 Jun 2022 01:47:45 +0200 Subject: [PATCH 10/10] Revert dependency --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index b478f207ab8..16d85250799 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -173,7 +173,7 @@ object Dependencies { val jimfs = "com.google.jimfs" % "jimfs" % "1.1" - val okHttp = "com.squareup.okhttp3" % "okhttp" % "4.2.3" + val okHttp = "com.squareup.okhttp3" % "okhttp" % "4.2.2" def routesCompilerDependencies(scalaVersion: String) = { val deps = CrossVersion.partialVersion(scalaVersion) match {