Add setting to configure max header value length in Akka HTTP Server (#…

…8162)

* Add setting to configure max header value length in Akka HTTP Server

* Configure mima filters

framework/src/play-akka-http-server/src/main/resources/reference.conf

@@ -54,6 +54,11 @@ play {
       # Play uses the concept of a `BodyParser` to enforce this limit, so we override it to infinite.
       max-content-length = infinite
 
+      # This setting is set in `akka.http.server.parsing.max-header-value-length`
+      # and it limits the length of HTTP header values.
+      max-header-value-length = 8k
+
+
       # Enables/disables inclusion of an Tls-Session-Info header in parsed
       # messages over Tls transports (i.e., HttpRequest on server side and
       # HttpResponse on client side).

framework/src/play-akka-http-server/src/main/scala/play/core/server/AkkaHttpServer.scala

@@ -96,6 +96,7 @@ class AkkaHttpServer(context: AkkaHttpServer.Context) extends Server {
   /** Called by Play when creating its Akka HTTP parser settings. Result stored in [[parserSettings]]. */
   protected def createParserSettings(): ParserSettings = ParserSettings(akkaHttpConfig)
     .withMaxContentLength(getPossiblyInfiniteBytes(akkaServerConfig.underlying, "max-content-length"))
+    .withMaxHeaderValueLength(akkaServerConfig.get[ConfigMemorySize]("max-header-value-length").toBytes.toInt)
     .withIncludeTlsSessionInfoHeader(akkaServerConfig.get[Boolean]("tls-session-info-header"))
     .withModeledHeaderParsing(false) // Disable most of Akka HTTP's header parsing; use RawHeaders instead
 

framework/src/play-integration-test/src/test/scala/play/it/http/RequestHeadersSpec.scala

@@ -3,7 +3,6 @@
  */
 package play.it.http
 
-import org.specs2.execute.AsResult
 import org.specs2.matcher.MatchResult
 import play.api.inject.guice.GuiceApplicationBuilder
 import play.api.mvc._
@@ -12,10 +11,14 @@ import play.api.{ Configuration, Mode }
 import play.core.server.ServerConfig
 import play.it._
 
-class NettyRequestHeadersSpec extends RequestHeadersSpec with NettyIntegrationSpecification
+class NettyRequestHeadersSpec extends RequestHeadersSpec with NettyIntegrationSpecification {
+  override def maxHeaderValueConfigurationKey: String = "play.server.netty.maxHeaderSize"
+}
 
 class AkkaHttpRequestHeadersSpec extends RequestHeadersSpec with AkkaHttpIntegrationSpecification {
 
+  override def maxHeaderValueConfigurationKey: String = "play.server.akka.max-header-value-length"
+
   "Akka HTTP request header handling" should {
 
     "not complain about invalid User-Agent headers" in {
@@ -56,7 +59,7 @@ trait RequestHeadersSpec extends PlaySpecification with ServerIntegrationSpecifi
 
   sequential
 
-  def withServerAndConfig[T](configuration: (String, Any)*)(action: (DefaultActionBuilder, PlayBodyParsers) => EssentialAction)(block: Port => T) = {
+  def withServerAndConfig[T](configuration: (String, Any)*)(action: (DefaultActionBuilder, PlayBodyParsers) => EssentialAction)(block: Port => T): T = {
     val port = testServerPort
 
     val serverConfig: ServerConfig = {
@@ -74,10 +77,12 @@ trait RequestHeadersSpec extends PlaySpecification with ServerIntegrationSpecifi
     }
   }
 
-  def withServer[T](action: (DefaultActionBuilder, PlayBodyParsers) => EssentialAction)(block: Port => T) = {
+  def withServer[T](action: (DefaultActionBuilder, PlayBodyParsers) => EssentialAction)(block: Port => T): T = {
     withServerAndConfig()(action)(block)
   }
 
+  def maxHeaderValueConfigurationKey: String
+
   "Play request header handling" should {
 
     "get request headers properly" in withServer((Action, _) => Action { rh =>
@@ -195,5 +200,24 @@ trait RequestHeadersSpec extends PlaySpecification with ServerIntegrationSpecifi
         """Mozilla/5.0 (iPhone; CPU iPhone OS 11_2_2 like Mac OS X) AppleWebKit/604.4.7 (KHTML, like Gecko) Mobile/15C202 [FBAN/FBIOS;FBAV/155.0.0.36.93;FBBV/87992437;FBDV/iPhone9,3;FBMD/iPhone;FBSN/iOS;FBSV/11.2.2;FBSS/2;FBCR/3Ireland;FBID/phone;FBLC/en_US;FBOP/5;FBRV/0]""")
     }
 
+    "respect max header value setting" in {
+      withServerAndConfig(maxHeaderValueConfigurationKey -> "64")((Action, _) => Action(Results.Ok)) { port =>
+        val responses = BasicHttpClient.makeRequests(port)(
+          // Only has valid headers that don't exceed 64 chars
+          BasicRequest("GET", "/", "HTTP/1.1", Map("h" -> "valid"), ""),
+          // Has a header that exceeds 64 bytes
+          BasicRequest("GET", "/", "HTTP/1.1", Map("h" -> "invalid" * 64), "")
+        )
+
+        responses.head.status must beEqualTo(OK)
+        responses.last.status must beOneOf(
+          // Akka-HTTP returns a "431 Request Header Fields Too Large" when the header value exceeds
+          // the max value length configured. And Netty returns a 414 URI Too Long.
+          REQUEST_HEADER_FIELDS_TOO_LARGE,
+          REQUEST_URI_TOO_LONG
+        )
+      }
+    }
+
   }
 }

framework/src/play-server/src/main/scala/play/core/server/DevServerStart.scala

@@ -46,7 +46,7 @@ object DevServerStart {
     buildLink: BuildLink,
     httpPort: Int,
     httpAddress: String): ReloadableServer = {
-    mainDev(buildLink, Some(httpPort), Option(System.getProperty("https.port")).map(Integer.parseInt(_)), httpAddress)
+    mainDev(buildLink, Some(httpPort), Option(System.getProperty("https.port")).map(Integer.parseInt), httpAddress)
   }
 
   private def mainDev(

framework/src/play-server/src/main/scala/play/core/server/ServerProvider.scala

@@ -54,23 +54,26 @@ object ServerProvider {
   def fromConfiguration(classLoader: ClassLoader, configuration: Configuration): ServerProvider = {
     val ClassNameConfigKey = "play.server.provider"
     val className: String = configuration.getOptional[String](ClassNameConfigKey)
-      .getOrElse(throw new ServerStartException(s"No ServerProvider configured with key '$ClassNameConfigKey'"))
+      .getOrElse(throw ServerStartException(s"No ServerProvider configured with key '$ClassNameConfigKey'"))
+
     val clazz = try classLoader.loadClass(className) catch {
-      case _: ClassNotFoundException => throw ServerStartException(s"Couldn't find ServerProvider class '$className'")
+      case ex: ClassNotFoundException => throw ServerStartException(s"Couldn't find ServerProvider class '$className'", cause = Some(ex))
     }
+
     if (!classOf[ServerProvider].isAssignableFrom(clazz)) throw ServerStartException(s"Class ${clazz.getName} must implement ServerProvider interface")
-    val ctor = try clazz.getConstructor() catch {
-      case _: NoSuchMethodException => throw ServerStartException(s"ServerProvider class ${clazz.getName} must have a public default constructor")
+    val constructor = try clazz.getConstructor() catch {
+      case ex: NoSuchMethodException => throw ServerStartException(s"ServerProvider class ${clazz.getName} must have a public default constructor", cause = Some(ex))
     }
-    ctor.newInstance().asInstanceOf[ServerProvider]
+
+    constructor.newInstance().asInstanceOf[ServerProvider]
   }
 
   /**
    * Load the default server provider.
    */
   implicit lazy val defaultServerProvider: ServerProvider = {
     val classLoader = this.getClass.getClassLoader
-    val config = Configuration.load(classLoader, System.getProperties, Map.empty, true)
+    val config = Configuration.load(classLoader, System.getProperties, Map.empty, allowMissingApplicationConf = true)
     fromConfiguration(classLoader, config)
   }
 

framework/src/play/src/main/scala/play/api/http/StandardValues.scala

@@ -217,6 +217,7 @@ trait Status {
   val FAILED_DEPENDENCY = 424
   val UPGRADE_REQUIRED = 426
   val TOO_MANY_REQUESTS = 429
+  final val REQUEST_HEADER_FIELDS_TOO_LARGE = 431
   @deprecated("Use TOO_MANY_REQUESTS instead", "2.6.0")
   val TOO_MANY_REQUEST = TOO_MANY_REQUESTS
 

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/dev-mode/build.sbt

@@ -29,8 +29,8 @@ lazy val root = (project in file("."))
     InputKey[Unit]("makeRequestWithHeader") := {
       val args = Def.spaceDelimited("<path> <status> <headers> ...").parsed
       val path :: status :: headers = args
-      val headerValue = headers.mkString
-      DevModeBuild.verifyResourceContains(path, status.toInt, Seq.empty, 0, "Header" -> headerValue)
+      val headerName = headers.mkString
+      DevModeBuild.verifyResourceContains(path, status.toInt, Seq.empty, 0, headerName -> "Header-Value")
     },
 
     InputKey[Unit]("verifyResourceContains") := {

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/dev-mode/test

@@ -207,20 +207,20 @@ $ copy-file conf/application.original conf/application.conf
 
 # First a test without the dev-setting to ensure everything works
 > run
-> makeRequestWithHeader / 200 this-is-a-header-value-longer-than-32-chars
+> makeRequestWithHeader / 200 this-is-a-header-name-longer-than-32-chars
 > playStop
 
 # A test overriding a akka setting that should be picked by dev mode
-> set PlayKeys.devSettings ++= Seq("akka.http.parsing.max-header-value-length" -> "32 bytes")
+> set PlayKeys.devSettings ++= Seq("akka.http.parsing.max-header-name-length" -> "32 bytes")
 > run
-> makeRequestWithHeader / 431 this-is-a-header-value-longer-than-32-chars
+> makeRequestWithHeader / 431 this-is-a-header-name-longer-than-32-chars
 > playStop
 
 # Should prioritize play.akka.dev-mode
-> set PlayKeys.devSettings ++= Seq("play.akka.dev-mode.akka.http.parsing.max-header-value-length" -> "32 bytes")
+> set PlayKeys.devSettings ++= Seq("play.akka.dev-mode.akka.http.parsing.max-header-name-length" -> "32 bytes")
 
 # This should NOT be picked
-> set PlayKeys.devSettings ++= Seq("akka.http.parsing.max-header-value-length" -> "1 megabyte")
+> set PlayKeys.devSettings ++= Seq("akka.http.parsing.max-header-name-length" -> "1 megabyte")
 > run
-> makeRequestWithHeader / 431 this-is-a-header-value-longer-than-32-chars
+> makeRequestWithHeader / 431 this-is-a-header-name-longer-than-32-chars
 > playStop