core: allow illegal header keys to be ignored #3133 (#3660)

Co-authored-by: Arnout Engelen <arnout@engelen.eu>
akka · Dec 8, 2020 · 5579778 · 5579778
1 parent 92dbb7e
commit 5579778
Show file tree

Hide file tree

Showing 9 changed files with 124 additions and 17 deletions.
diff --git a/...in/mima-filters/10.2.1.backwards.excludes/issue-3133-ignore-illegal-header-names.excludes b/...in/mima-filters/10.2.1.backwards.excludes/issue-3133-ignore-illegal-header-names.excludes
@@ -0,0 +1,6 @@
+ProblemFilters.exclude[Problem]("akka.http.impl.settings.ParserSettingsImpl*")
+ProblemFilters.exclude[DirectMissingMethodProblem]("akka.http.impl.model.parser.HeaderParser.Settings")
+ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.impl.engine.parsing.HttpHeaderParser#Settings.illegalResponseHeaderNameProcessingMode")
+ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.impl.model.parser.HeaderParser#Settings.illegalResponseHeaderNameProcessingMode")
+ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.javadsl.settings.ParserSettings.getIllegalResponseHeaderNameProcessingMode")
+ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.scaladsl.settings.ParserSettings.illegalResponseHeaderNameProcessingMode")
diff --git a/akka-http-core/src/main/resources/reference.conf b/akka-http-core/src/main/resources/reference.conf
@@ -680,6 +680,15 @@ akka.http {
     # `full`  : the full error details (potentially spanning several lines) are logged
     error-logging-verbosity = full
 
+    # Configures the processing mode when encountering illegal characters in
+    # header name of response.
+    #
+    # Supported mode:
+    # `error`  : default mode, reject the request
+    # `warn`   : ignore the illegal characters in response header value and log a warning message
+    # `ignore` : just ignore the illegal characters in response header value
+    illegal-response-header-name-processing-mode = error
+
     # Configures the processing mode when encountering illegal characters in
     # header value of response.
     #

diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpHeaderParser.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpHeaderParser.scala
@@ -10,7 +10,7 @@ import java.lang.{ StringBuilder => JStringBuilder }
 
 import akka.annotation.InternalApi
 import akka.event.LoggingAdapter
-import akka.http.scaladsl.settings.ParserSettings.IllegalResponseHeaderValueProcessingMode
+import akka.http.scaladsl.settings.ParserSettings.{ IllegalResponseHeaderValueProcessingMode, IllegalResponseHeaderNameProcessingMode }
 import akka.http.scaladsl.settings.ParserSettings.ErrorLoggingVerbosity
 import akka.http.scaladsl.settings.ParserSettings
 
@@ -168,6 +168,20 @@ private[engine] final class HttpHeaderParser private (
     }
   }
 
+  @tailrec private def scanHeaderNameAndReturnIndexOfColon(input: ByteString, start: Int, limit: Int)(ix: Int): Int =
+    if (ix < limit)
+      (byteChar(input, ix), settings.illegalResponseHeaderNameProcessingMode) match {
+        case (':', _) => ix
+        case (c, _) if tchar(c) => scanHeaderNameAndReturnIndexOfColon(input, start, limit)(ix + 1)
+        case (c, IllegalResponseHeaderNameProcessingMode.Error) => fail(s"Illegal character '${escape(c)}' in header name")
+        case (c, IllegalResponseHeaderNameProcessingMode.Warn) =>
+          log.warning(s"Header key contains illegal character '${escape(c)}'")
+          scanHeaderNameAndReturnIndexOfColon(input, start, limit)(ix + 1)
+        case (c, IllegalResponseHeaderNameProcessingMode.Ignore) =>
+          scanHeaderNameAndReturnIndexOfColon(input, start, limit)(ix + 1)
+      }
+    else fail(s"HTTP header name exceeds the configured limit of ${limit - start - 1} characters", StatusCodes.RequestHeaderFieldsTooLarge)
+
   @tailrec
   private def parseHeaderValue(input: ByteString, valueStart: Int, branch: ValueBranch)(cursor: Int = valueStart, nodeIx: Int = branch.branchRootNodeIx): Int = {
     def parseAndInsertHeader() = {
@@ -428,6 +442,7 @@ private[http] object HttpHeaderParser {
     def customMediaTypes: MediaTypes.FindCustom
     def illegalHeaderWarnings: Boolean
     def ignoreIllegalHeaderFor: Set[String]
+    def illegalResponseHeaderNameProcessingMode: IllegalResponseHeaderNameProcessingMode
     def illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode
     def errorLoggingVerbosity: ErrorLoggingVerbosity
     def modeledHeaderParsing: Boolean
@@ -548,15 +563,6 @@ private[http] object HttpHeaderParser {
     }
   }
 
-  @tailrec private def scanHeaderNameAndReturnIndexOfColon(input: ByteString, start: Int, limit: Int)(ix: Int): Int =
-    if (ix < limit)
-      byteChar(input, ix) match {
-        case ':'           => ix
-        case c if tchar(c) => scanHeaderNameAndReturnIndexOfColon(input, start, limit)(ix + 1)
-        case c             => fail(s"Illegal character '${escape(c)}' in header name")
-      }
-    else fail(s"HTTP header name exceeds the configured limit of ${limit - start - 1} characters", StatusCodes.RequestHeaderFieldsTooLarge)
-
   @tailrec private def scanHeaderValue(hhp: HttpHeaderParser, input: ByteString, start: Int, limit: Int, log: LoggingAdapter,
                                        mode: IllegalResponseHeaderValueProcessingMode)(sb: JStringBuilder = null, ix: Int = start): (String, Int) = {
     hhp.byteBuffer.clear()

diff --git a/akka-http-core/src/main/scala/akka/http/impl/model/parser/HeaderParser.scala b/akka-http-core/src/main/scala/akka/http/impl/model/parser/HeaderParser.scala
@@ -7,7 +7,7 @@ package akka.http.impl.model.parser
 import akka.annotation.InternalApi
 import akka.http.scaladsl.settings.ParserSettings
 import akka.http.scaladsl.settings.ParserSettings.CookieParsingMode
-import akka.http.scaladsl.settings.ParserSettings.IllegalResponseHeaderValueProcessingMode
+import akka.http.scaladsl.settings.ParserSettings.{ IllegalResponseHeaderValueProcessingMode, IllegalResponseHeaderNameProcessingMode }
 import akka.http.scaladsl.model.headers.HttpCookiePair
 import akka.util.ConstantFun
 
@@ -191,18 +191,21 @@ private[http] object HeaderParser {
     def uriParsingMode: Uri.ParsingMode
     def cookieParsingMode: ParserSettings.CookieParsingMode
     def customMediaTypes: MediaTypes.FindCustom
+    def illegalResponseHeaderNameProcessingMode: IllegalResponseHeaderNameProcessingMode
     def illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode
   }
   def Settings(
     uriParsingMode:    Uri.ParsingMode                          = Uri.ParsingMode.Relaxed,
     cookieParsingMode: ParserSettings.CookieParsingMode         = ParserSettings.CookieParsingMode.RFC6265,
     customMediaTypes:  MediaTypes.FindCustom                    = ConstantFun.scalaAnyTwoToNone,
-    mode:              IllegalResponseHeaderValueProcessingMode = ParserSettings.IllegalResponseHeaderValueProcessingMode.Error): Settings = {
+    modeValue:         IllegalResponseHeaderValueProcessingMode = ParserSettings.IllegalResponseHeaderValueProcessingMode.Error,
+    modeName:          IllegalResponseHeaderNameProcessingMode  = ParserSettings.IllegalResponseHeaderNameProcessingMode.Error): Settings = {
 
     val _uriParsingMode = uriParsingMode
     val _cookieParsingMode = cookieParsingMode
     val _customMediaTypes = customMediaTypes
-    val _illegalResponseHeaderValueProcessingMode = mode
+    val _illegalResponseHeaderValueProcessingMode = modeValue
+    val _illegalResponseHeaderNameProcessingMode = modeName
 
     new Settings {
       def uriParsingMode: Uri.ParsingMode = _uriParsingMode
@@ -211,6 +214,9 @@ private[http] object HeaderParser {
 
       def illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode =
         _illegalResponseHeaderValueProcessingMode
+
+      def illegalResponseHeaderNameProcessingMode: IllegalResponseHeaderNameProcessingMode =
+        _illegalResponseHeaderNameProcessingMode
     }
   }
   val DefaultSettings: Settings = Settings()

diff --git a/akka-http-core/src/main/scala/akka/http/impl/settings/ParserSettingsImpl.scala b/akka-http-core/src/main/scala/akka/http/impl/settings/ParserSettingsImpl.scala
@@ -5,7 +5,7 @@
 package akka.http.impl.settings
 
 import akka.annotation.InternalApi
-import akka.http.scaladsl.settings.ParserSettings.{ CookieParsingMode, ErrorLoggingVerbosity, IllegalResponseHeaderValueProcessingMode }
+import akka.http.scaladsl.settings.ParserSettings.{ CookieParsingMode, ErrorLoggingVerbosity, IllegalResponseHeaderValueProcessingMode, IllegalResponseHeaderNameProcessingMode }
 import akka.util.ConstantFun
 import com.typesafe.config.Config
 
@@ -32,6 +32,7 @@ private[akka] final case class ParserSettingsImpl(
   illegalHeaderWarnings:                    Boolean,
   ignoreIllegalHeaderFor:                   Set[String],
   errorLoggingVerbosity:                    ErrorLoggingVerbosity,
+  illegalResponseHeaderNameProcessingMode:  IllegalResponseHeaderNameProcessingMode,
   illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode,
   headerValueCacheLimits:                   Map[String, Int],
   includeTlsSessionInfoHeader:              Boolean,
@@ -95,6 +96,7 @@ object ParserSettingsImpl extends SettingsCompanionImpl[ParserSettingsImpl]("akk
       c.getBoolean("illegal-header-warnings"),
       c.getStringList("ignore-illegal-header-for").asScala.map(_.toLowerCase).toSet,
       ErrorLoggingVerbosity(c.getString("error-logging-verbosity")),
+      IllegalResponseHeaderNameProcessingMode(c.getString("illegal-response-header-name-processing-mode")),
       IllegalResponseHeaderValueProcessingMode(c.getString("illegal-response-header-value-processing-mode")),
       cacheConfig.entrySet.asScala.iterator.map(kvp => kvp.getKey -> cacheConfig.getInt(kvp.getKey)).toMap,
       c.getBoolean("tls-session-info-header"),

diff --git a/akka-http-core/src/main/scala/akka/http/javadsl/settings/ParserSettings.scala b/akka-http-core/src/main/scala/akka/http/javadsl/settings/ParserSettings.scala
@@ -40,6 +40,7 @@ abstract class ParserSettings private[akka] () extends BodyPartParser.Settings {
   def getIllegalHeaderWarnings: Boolean
   def getIgnoreIllegalHeaderFor: Set[String]
   def getErrorLoggingVerbosity: ParserSettings.ErrorLoggingVerbosity
+  def getIllegalResponseHeaderNameProcessingMode: ParserSettings.IllegalResponseHeaderNameProcessingMode
   def getIllegalResponseHeaderValueProcessingMode: ParserSettings.IllegalResponseHeaderValueProcessingMode
   def getHeaderValueCacheLimits: ju.Map[String, Int]
   def getIncludeTlsSessionInfoHeader: Boolean
@@ -95,6 +96,7 @@ abstract class ParserSettings private[akka] () extends BodyPartParser.Settings {
 object ParserSettings extends SettingsCompanion[ParserSettings] {
   trait CookieParsingMode
   trait ErrorLoggingVerbosity
+  trait IllegalResponseHeaderNameProcessingMode
   trait IllegalResponseHeaderValueProcessingMode
 
   /**

diff --git a/akka-http-core/src/main/scala/akka/http/scaladsl/settings/ParserSettings.scala b/akka-http-core/src/main/scala/akka/http/scaladsl/settings/ParserSettings.scala
@@ -40,6 +40,7 @@ abstract class ParserSettings private[akka] () extends akka.http.javadsl.setting
   def illegalHeaderWarnings: Boolean
   def ignoreIllegalHeaderFor: Set[String]
   def errorLoggingVerbosity: ParserSettings.ErrorLoggingVerbosity
+  def illegalResponseHeaderNameProcessingMode: ParserSettings.IllegalResponseHeaderNameProcessingMode
   def illegalResponseHeaderValueProcessingMode: ParserSettings.IllegalResponseHeaderValueProcessingMode
   def headerValueCacheLimits: Map[String, Int]
   def includeTlsSessionInfoHeader: Boolean
@@ -68,6 +69,7 @@ abstract class ParserSettings private[akka] () extends akka.http.javadsl.setting
   override def getMaxUriLength = maxUriLength
   override def getMaxMethodLength = maxMethodLength
   override def getErrorLoggingVerbosity: js.ParserSettings.ErrorLoggingVerbosity = errorLoggingVerbosity
+  override def getIllegalResponseHeaderNameProcessingMode = illegalResponseHeaderNameProcessingMode
   override def getIllegalResponseHeaderValueProcessingMode = illegalResponseHeaderValueProcessingMode
 
   override def getCustomMethods = new Function[String, Optional[akka.http.javadsl.model.HttpMethod]] {
@@ -117,6 +119,8 @@ abstract class ParserSettings private[akka] () extends akka.http.javadsl.setting
     val map = types.map(c => (c.mainType, c.subType) -> c).toMap
     self.copy(customMediaTypes = (main, sub) => map.get((main, sub)))
   }
+  def withIllegalResponseHeaderNameProcessingMode(newValue: ParserSettings.IllegalResponseHeaderNameProcessingMode): ParserSettings =
+    self.copy(illegalResponseHeaderNameProcessingMode = newValue)
   def withIllegalResponseHeaderValueProcessingMode(newValue: ParserSettings.IllegalResponseHeaderValueProcessingMode): ParserSettings =
     self.copy(illegalResponseHeaderValueProcessingMode = newValue)
 }
@@ -163,6 +167,21 @@ object ParserSettings extends SettingsCompanion[ParserSettings] {
       }
   }
 
+  sealed trait IllegalResponseHeaderNameProcessingMode extends akka.http.javadsl.settings.ParserSettings.IllegalResponseHeaderNameProcessingMode
+  object IllegalResponseHeaderNameProcessingMode {
+    case object Error extends IllegalResponseHeaderNameProcessingMode
+    case object Warn extends IllegalResponseHeaderNameProcessingMode
+    case object Ignore extends IllegalResponseHeaderNameProcessingMode
+
+    def apply(string: String): IllegalResponseHeaderNameProcessingMode =
+      string.toRootLowerCase match {
+        case "error"  => Error
+        case "warn"   => Warn
+        case "ignore" => Ignore
+        case x        => throw new IllegalArgumentException(s"[$x] is not a legal `illegal-response-header-name-processing-mode` setting")
+      }
+  }
+
   @deprecated("Use forServer or forClient instead", "10.2.0")
   override def apply(config: Config): ParserSettings = ParserSettingsImpl(config)
   @deprecated("Use forServer or forClient instead", "10.2.0")

diff --git a/...ttp-core/src/test/scala/akka/http/impl/engine/client/LowLevelOutgoingConnectionSpec.scala b/...ttp-core/src/test/scala/akka/http/impl/engine/client/LowLevelOutgoingConnectionSpec.scala
@@ -416,6 +416,24 @@ class LowLevelOutgoingConnectionSpec extends AkkaSpecWithMaterializer with Insid
         headerStr shouldEqual "Some-Header: value1,Other-Header: value2"
       }
 
+      val ignoreNameConfig =
+        """
+          akka.http.parsing.illegal-response-header-name-processing-mode = ignore
+        """
+      "ignore illegal response header name if setting the config to ignore" in new TestSetup(config = ignoreNameConfig) {
+        sendStandardRequest()
+        sendWireData(
+          s"""HTTP/1.1 200 OK
+              |Some Header: value1
+              |Other-Header: value2
+              |
+              |""")
+
+        val HttpResponse(_, headers, _, _) = expectResponse()
+        val headerStr = headers.map(h => s"${h.name}: ${h.value}").mkString(",")
+        headerStr shouldEqual "Some Header: value1,Other-Header: value2"
+      }
+
       val warnConfig =
         """
           akka.http.parsing.illegal-response-header-value-processing-mode = warn
@@ -435,6 +453,24 @@ class LowLevelOutgoingConnectionSpec extends AkkaSpecWithMaterializer with Insid
       }
     }
 
+    val warnNameConfig =
+      """
+          akka.http.parsing.illegal-response-header-name-processing-mode = warn
+        """
+    "ignore illegal response header name and log a warning message if setting the config to warn" in new TestSetup(config = warnNameConfig) {
+      sendStandardRequest()
+      sendWireData(
+        s"""HTTP/1.1 200 OK
+              |Some Header: value1
+              |Other-Header: value2
+              |
+              |""")
+
+      val HttpResponse(_, headers, _, _) = expectResponse()
+      val headerStr = headers.map(h => s"${h.name}: ${h.value}").mkString(",")
+      headerStr shouldEqual "Some Header: value1,Other-Header: value2"
+    }
+
     "produce proper errors" which {
 
       "catch the request entity stream being shorter than the Content-Length" in new TestSetup {

diff --git a/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/HttpHeaderParserSpec.scala b/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/HttpHeaderParserSpec.scala
@@ -17,7 +17,7 @@ import akka.http.scaladsl.model.{ ErrorInfo, HttpHeader }
 import akka.http.scaladsl.model.headers._
 import akka.http.impl.model.parser.CharacterClasses
 import akka.http.impl.util._
-import akka.http.scaladsl.settings.ParserSettings.IllegalResponseHeaderValueProcessingMode
+import akka.http.scaladsl.settings.ParserSettings.{ IllegalResponseHeaderNameProcessingMode, IllegalResponseHeaderValueProcessingMode }
 import akka.testkit.EventFilter
 
 abstract class HttpHeaderParserSpec(mode: String, newLine: String) extends AkkaSpecWithMaterializer(
@@ -192,6 +192,26 @@ abstract class HttpHeaderParserSpec(mode: String, newLine: String) extends AkkaS
       the[ParsingException] thrownBy parseLine(s"Connec/tion: close${newLine}x") should have message "Illegal character '/' in header name"
     }
 
+    "ignore illegal headers when configured to ignore them" in new TestSetup(
+      parserSettings = createParserSettings(
+        actorSystem = system,
+        illegalResponseHeaderNameProcessingMode = IllegalResponseHeaderNameProcessingMode.Ignore)
+    ) {
+      parseLine(s" Connection: close${newLine}x")
+      parseLine(s"Connection : close${newLine}x")
+      parseLine(s"Connec/tion: close${newLine}x")
+    }
+
+    "ignore illegal headers when configured to ignore and warn about them" in new TestSetup(
+      parserSettings = createParserSettings(
+        actorSystem = system,
+        illegalResponseHeaderNameProcessingMode = IllegalResponseHeaderNameProcessingMode.Warn)
+    ) {
+      parseLine(s" Connection: close${newLine}x")
+      parseLine(s"Connection : close${newLine}x")
+      parseLine(s"Connec/tion: close${newLine}x")
+    }
+
     "produce an error message for lines with a too-long header name" in new TestSetup() {
       noException should be thrownBy parseLine(s"123456789012345678901234567890123456789012345678901234567890: foo${newLine}x")
       the[ParsingException] thrownBy parseLine(s"1234567890123456789012345678901234567890123456789012345678901: foo${newLine}x") should have message
@@ -328,10 +348,11 @@ abstract class HttpHeaderParserSpec(mode: String, newLine: String) extends AkkaS
   }
 
   def createParserSettings(
-    actorSystem:                              ActorSystem,
-    illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode = IllegalResponseHeaderValueProcessingMode.Error): ParserSettings =
+    actorSystem:                             ActorSystem,
+    illegalResponseHeaderNameProcessingMode: IllegalResponseHeaderNameProcessingMode = IllegalResponseHeaderNameProcessingMode.Error, illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode = IllegalResponseHeaderValueProcessingMode.Error): ParserSettings =
     ParserSettings(actorSystem)
       .withIllegalResponseHeaderValueProcessingMode(illegalResponseHeaderValueProcessingMode)
+      .withIllegalResponseHeaderNameProcessingMode(illegalResponseHeaderNameProcessingMode)
 
   abstract class TestSetup(testSetupMode: TestSetupMode = TestSetupMode.Primed, parserSettings: ParserSettings = createParserSettings(system)) {