Skip to content

Commit

Permalink
add setting ignore-illegal-header-for #687 (#1942)
Browse files Browse the repository at this point in the history
* add setting ignore-illegal-header-for #687

* fix binary incompatibility #687

* move binary compatibility warning configuration to 10.1.0 #687

* add setting's comment #687

* rename method #687

* reorder method #687

* check ignore header name insensitive #687

* Update 10.1.0.backwards.excludes

* put case class back for ErrorInfo. and add annotation @internalapi #687

* fix test #687

* delete InternalApi annnotation from ErrorInfo #687

* create 10.1.2.backwards.excludes #687

* Chainge ErrorInfo from caseclass to class for bin incomp  #687

* Update reference.conf

* add tests #687

* use parseLine #687

* Update reference.conf
  • Loading branch information
shnmorimoto authored and ktoso committed May 7, 2018
1 parent 561f02d commit 78e5b1f
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Don't monitor changes to internal API
ProblemFilters.exclude[Problem]("akka.http.impl.*")

# #1942 New ignoreIllegalHeaderFor setting
ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.javadsl.settings.ParserSettings.getIgnoreIllegalHeaderFor")
ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.scaladsl.settings.ParserSettings.ignoreIllegalHeaderFor")
6 changes: 6 additions & 0 deletions akka-http-core/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,12 @@ akka.http {
# `error-logging-verbosity`.
illegal-header-warnings = on

# Sets the list of headers for which illegal values will *not* cause warning logs to be emitted;
#
# Adding a header name to this setting list disables the logging of warning messages in case an incoming message
# contains an HTTP header which cannot be parsed into its high-level model class due to incompatible syntax.
ignore-illegal-header-for = []

# Parse headers into typed model classes in the Akka Http core layer.
#
# If set to `off`, only essential headers will be parsed into their model classes. All other ones will be provided
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ private[http] object HttpHeaderParser {
def headerValueCacheLimit(headerName: String): Int
def customMediaTypes: MediaTypes.FindCustom
def illegalHeaderWarnings: Boolean
def ignoreIllegalHeaderFor: Set[String]
def illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode
def errorLoggingVerbosity: ErrorLoggingVerbosity
def modeledHeaderParsing: Boolean
Expand Down Expand Up @@ -461,7 +462,7 @@ private[http] object HttpHeaderParser {

def defaultIllegalHeaderHandler(settings: HttpHeaderParser.Settings, log: LoggingAdapter): ErrorInfo Unit =
if (settings.illegalHeaderWarnings)
info logParsingError(info withSummaryPrepended "Illegal header", log, settings.errorLoggingVerbosity)
info logParsingError(info withSummaryPrepended "Illegal header", log, settings.errorLoggingVerbosity, settings.ignoreIllegalHeaderFor)
else
(_: ErrorInfo) _ // Does exactly what the label says - nothing

Expand Down Expand Up @@ -529,7 +530,7 @@ private[http] object HttpHeaderParser {
val header = parser(trimmedHeaderValue) match {
case HeaderParser.Success(h) h
case HeaderParser.Failure(error)
onIllegalHeader(error.withSummaryPrepended(s"Illegal '$headerName' header"))
onIllegalHeader(error.withSummaryPrepended(s"Illegal '$headerName' header").withErrorHeaderName(headerName))
RawHeader(headerName, trimmedHeaderValue)
case HeaderParser.RuleNotFound
throw new IllegalStateException(s"Unexpected RuleNotFound exception for modeled header [$headerName]")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,16 @@ package object parsing {
}

private[http] def logParsingError(info: ErrorInfo, log: LoggingAdapter,
setting: ParserSettings.ErrorLoggingVerbosity): Unit =
setting match {
case ParserSettings.ErrorLoggingVerbosity.Off // nothing to do
case ParserSettings.ErrorLoggingVerbosity.Simple log.warning(info.summary)
case ParserSettings.ErrorLoggingVerbosity.Full log.warning(info.formatPretty)
settings: ParserSettings.ErrorLoggingVerbosity,
ignoreHeaderNames: Set[String] = Set.empty): Unit =
settings match {
case ParserSettings.ErrorLoggingVerbosity.Off // nothing to do
case ParserSettings.ErrorLoggingVerbosity.Simple
if (!ignoreHeaderNames.contains(info.errorHeaderName))
log.warning(info.summary)
case ParserSettings.ErrorLoggingVerbosity.Full
if (!ignoreHeaderNames.contains(info.errorHeaderName))
log.warning(info.formatPretty)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ private[akka] final case class ParserSettingsImpl(
uriParsingMode: Uri.ParsingMode,
cookieParsingMode: CookieParsingMode,
illegalHeaderWarnings: Boolean,
ignoreIllegalHeaderFor: Set[String],
errorLoggingVerbosity: ErrorLoggingVerbosity,
illegalResponseHeaderValueProcessingMode: IllegalResponseHeaderValueProcessingMode,
headerValueCacheLimits: Map[String, Int],
Expand Down Expand Up @@ -82,6 +83,7 @@ object ParserSettingsImpl extends SettingsCompanion[ParserSettingsImpl]("akka.ht
Uri.ParsingMode(c getString "uri-parsing-mode"),
CookieParsingMode(c getString "cookie-parsing-mode"),
c getBoolean "illegal-header-warnings",
(c getStringList "ignore-illegal-header-for").asScala.map(_.toLowerCase).toSet,
ErrorLoggingVerbosity(c getString "error-logging-verbosity"),
IllegalResponseHeaderValueProcessingMode(c getString "illegal-response-header-value-processing-mode"),
cacheConfig.entrySet.asScala.map(kvp kvp.getKey cacheConfig.getInt(kvp.getKey))(collection.breakOut),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ abstract class ParserSettings private[akka] () extends BodyPartParser.Settings {
def getUriParsingMode: Uri.ParsingMode
def getCookieParsingMode: ParserSettings.CookieParsingMode
def getIllegalHeaderWarnings: Boolean
def getIgnoreIllegalHeaderFor: Set[String]
def getErrorLoggingVerbosity: ParserSettings.ErrorLoggingVerbosity
def getIllegalResponseHeaderValueProcessingMode: ParserSettings.IllegalResponseHeaderValueProcessingMode
def getHeaderValueCacheLimits: ju.Map[String, Int]
Expand Down Expand Up @@ -64,6 +65,7 @@ abstract class ParserSettings private[akka] () extends BodyPartParser.Settings {
def withHeaderValueCacheLimits(newValue: ju.Map[String, Int]): ParserSettings = self.copy(headerValueCacheLimits = newValue.asScala.toMap)
def withIncludeTlsSessionInfoHeader(newValue: Boolean): ParserSettings = self.copy(includeTlsSessionInfoHeader = newValue)
def withModeledHeaderParsing(newValue: Boolean): ParserSettings = self.copy(modeledHeaderParsing = newValue)
def withIgnoreIllegalHeaderFor(newValue: List[String]): ParserSettings = self.copy(ignoreIllegalHeaderFor = newValue.map(_.toLowerCase).toSet)

// special ---

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,56 @@
package akka.http.scaladsl.model

import StatusCodes.ClientError
import akka.annotation.InternalApi

/**
* Two-level model of error information.
* The summary should explain what is wrong with the request or response *without* directly
* repeating anything present in the message itself (in order to not open holes for XSS attacks),
* while the detail can contain additional information from any source (even the request itself).
*/
final case class ErrorInfo(summary: String = "", detail: String = "") {
final class ErrorInfo(
val summary: String = "",
val detail: String = "",
val errorHeaderName: String = ""
) extends scala.Product with scala.Serializable with scala.Equals with java.io.Serializable {
def withSummary(newSummary: String) = copy(summary = newSummary)
def withSummaryPrepended(prefix: String) = withSummary(if (summary.isEmpty) prefix else prefix + ": " + summary)
def withErrorHeaderName(headerName: String) = new ErrorInfo(summary, detail, headerName.toLowerCase)
def withFallbackSummary(fallbackSummary: String) = if (summary.isEmpty) withSummary(fallbackSummary) else this
def formatPretty = if (summary.isEmpty) detail else if (detail.isEmpty) summary else summary + ": " + detail
def format(withDetail: Boolean): String = if (withDetail) formatPretty else summary

/** INTERNAL API */
@InternalApi private[akka] def copy(summary: String = summary, detail: String = detail): ErrorInfo = {
new ErrorInfo(summary, detail, errorHeaderName)
}

override def canEqual(that: Any): Boolean = that.isInstanceOf[ErrorInfo]

override def equals(that: Any): Boolean = that match {
case that: ErrorInfo that.canEqual(this) && that.summary == this.summary && that.detail == this.detail && that.errorHeaderName == this.errorHeaderName
case _ false
}

override def productElement(n: Int): Any = n match {
case 0 summary
case 1 detail
case 2 errorHeaderName
}

override def productArity: Int = 3

/** INTERNAL API */
@InternalApi private[akka] def this(summary: String, detail: String) = this(summary, detail, "")
}

object ErrorInfo {
/** INTERNAL API */
@InternalApi private[akka] def apply(summary: String = "", detail: String = ""): ErrorInfo = new ErrorInfo(summary, detail, "")

def unapply(arg: ErrorInfo): Option[(String, String)] = Some((arg.summary, arg.detail))

/**
* Allows constructing an `ErrorInfo` from a single string.
* Used for example when catching exceptions generated by the header value parser, which doesn't provide
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ abstract class ParserSettings private[akka] () extends akka.http.javadsl.setting
def uriParsingMode: Uri.ParsingMode
def cookieParsingMode: ParserSettings.CookieParsingMode
def illegalHeaderWarnings: Boolean
def ignoreIllegalHeaderFor: Set[String]
def errorLoggingVerbosity: ParserSettings.ErrorLoggingVerbosity
def illegalResponseHeaderValueProcessingMode: ParserSettings.IllegalResponseHeaderValueProcessingMode
def headerValueCacheLimits: Map[String, Int]
Expand All @@ -55,6 +56,7 @@ abstract class ParserSettings private[akka] () extends akka.http.javadsl.setting
override def getMaxHeaderValueLength = maxHeaderValueLength
override def getIncludeTlsSessionInfoHeader = includeTlsSessionInfoHeader
override def getIllegalHeaderWarnings = illegalHeaderWarnings
override def getIgnoreIllegalHeaderFor = ignoreIllegalHeaderFor
override def getMaxHeaderNameLength = maxHeaderNameLength
override def getMaxChunkSize = maxChunkSize
override def getMaxResponseReasonLength = maxResponseReasonLength
Expand Down Expand Up @@ -88,6 +90,7 @@ abstract class ParserSettings private[akka] () extends akka.http.javadsl.setting
override def withIllegalHeaderWarnings(newValue: Boolean): ParserSettings = self.copy(illegalHeaderWarnings = newValue)
override def withIncludeTlsSessionInfoHeader(newValue: Boolean): ParserSettings = self.copy(includeTlsSessionInfoHeader = newValue)
override def withModeledHeaderParsing(newValue: Boolean): ParserSettings = self.copy(modeledHeaderParsing = newValue)
override def withIgnoreIllegalHeaderFor(newValue: List[String]): ParserSettings = self.copy(ignoreIllegalHeaderFor = newValue.map(_.toLowerCase).toSet)

// overloads for idiomatic Scala use
def withUriParsingMode(newValue: Uri.ParsingMode): ParserSettings = self.copy(uriParsingMode = newValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ 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.testkit.TestKit
import akka.testkit.{ EventFilter, TestKit }

abstract class HttpHeaderParserSpec(mode: String, newLine: String) extends WordSpec with Matchers with BeforeAndAfterAll {

val testConf: Config = ConfigFactory.parseString("""
akka.event-handlers = ["akka.testkit.TestEventListener"]
akka.loglevel = ERROR
akka.loglevel = WARNING
akka.http.parsing.max-header-name-length = 60
akka.http.parsing.max-header-value-length = 1000
akka.http.parsing.header-cache.Host = 300""")
val system = ActorSystem(getClass.getSimpleName, testConf)
implicit val system = ActorSystem(getClass.getSimpleName, testConf)

s"The HttpHeaderParser (mode: $mode)" should {
"insert the 1st value" in new TestSetup(testSetupMode = TestSetupMode.Unprimed) {
Expand Down Expand Up @@ -254,6 +254,19 @@ abstract class HttpHeaderParserSpec(mode: String, newLine: String) extends WordS
parseAndCache(s"User-Agent: hmpf${newLine}x")(s"USER-AGENT: hmpf${newLine}x") shouldEqual RawHeader("User-Agent", "hmpf")
parseAndCache(s"X-Forwarded-Host: localhost:8888${newLine}x")(s"X-FORWARDED-Host: localhost:8888${newLine}x") shouldEqual RawHeader("X-Forwarded-Host", "localhost:8888")
}
"disables the logging of warning message when set the whitelist for illegal headers" in new TestSetup(
testSetupMode = TestSetupMode.Default,
parserSettings = createParserSettings(system).withIgnoreIllegalHeaderFor(List("Content-Type"))) {
//Illegal header is `Retry-After`. So logged warning message
EventFilter.warning(occurrences = 1).intercept {
parseLine(s"Retry-After: -10${newLine}x")
}

//Illegal header is `Content-Type` and it is in the whitelist. So not logged warning message
EventFilter.warning(occurrences = 0).intercept {
parseLine(s"Content-Type: abc:123${newLine}x")
}
}
}

override def afterAll() = TestKit.shutdownActorSystem(system)
Expand Down Expand Up @@ -284,13 +297,13 @@ abstract class HttpHeaderParserSpec(mode: String, newLine: String) extends WordS
case TestSetupMode.Default HttpHeaderParser(parserSettings, system.log)
}

private def defaultIllegalHeaderHandler = (info: ErrorInfo) system.log.warning(info.formatPretty)
private def defaultIllegalHeaderHandler = (info: ErrorInfo) system.log.debug(info.formatPretty)

def insert(line: String, value: AnyRef): Unit =
if (parser.isEmpty) HttpHeaderParser.insertRemainingCharsAsNewNodes(parser, ByteString(line), value)
else HttpHeaderParser.insert(parser, ByteString(line), value)

def parseLine(line: String) = parser.parseHeaderLine(ByteString(line))() { system.log.warning(parser.resultHeader.getClass.getSimpleName); parser.resultHeader }
def parseLine(line: String) = parser.parseHeaderLine(ByteString(line))() { system.log.debug(parser.resultHeader.getClass.getSimpleName); parser.resultHeader }

def parseAndCache(lineA: String)(lineB: String = lineA): HttpHeader = {
val (ixA, headerA) = parseLine(lineA)
Expand Down

0 comments on commit 78e5b1f

Please sign in to comment.