Skip to content

Commit

Permalink
core: disallow parsing of Transfer-Encoding other than chunked (#3754)
Browse files Browse the repository at this point in the history
  • Loading branch information
@johanandren
johanandren committed Feb 15, 2021
1 parent 26cfb32 commit e3a4935
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 140 deletions.
2 changes: 2 additions & 0 deletions ...mima-filters/10.2.3.backwards.excludes/stricter-transfer-encoding-header-parsing.excludes
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# parsing internals only
ProblemFilters.exclude[Problem]("akka.http.impl.engine.parsing.*")
64 changes: 35 additions & 29 deletions akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpMessageParser.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] {
protected def onBadProtocol(): Nothing
protected def parseMessage(input: ByteString, offset: Int): HttpMessageParser.StateResult
protected def parseEntity(headers: List[HttpHeader], protocol: HttpProtocol, input: ByteString, bodyStart: Int,
clh: Option[`Content-Length`], cth: Option[`Content-Type`], teh: Option[`Transfer-Encoding`],
clh: Option[`Content-Length`], cth: Option[`Content-Type`], isChunked: Boolean,
expect100continue: Boolean, hostHeaderPresent: Boolean, closeAfterResponseCompletion: Boolean,
sslSession: SSLSession): HttpMessageParser.StateResult

Expand Down Expand Up @@ -137,7 +137,7 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] {
@tailrec protected final def parseHeaderLines(input: ByteString, lineStart: Int, headers: ListBuffer[HttpHeader] = initialHeaderBuffer,
headerCount: Int = 0, ch: Option[Connection] = None,
clh: Option[`Content-Length`] = None, cth: Option[`Content-Type`] = None,
teh: Option[`Transfer-Encoding`] = None, e100c: Boolean = false,
isChunked: Boolean = false, e100c: Boolean = false,
hh: Boolean = false): StateResult =
if (headerCount < settings.maxHeaderCount) {
var lineEnd = 0
Expand All @@ -149,57 +149,69 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] {
case NotEnoughDataException => null
}
resultHeader match {
case null => continue(input, lineStart)(parseHeaderLinesAux(headers, headerCount, ch, clh, cth, teh, e100c, hh))
case null => continue(input, lineStart)(parseHeaderLinesAux(headers, headerCount, ch, clh, cth, isChunked, e100c, hh))

case EmptyHeader =>
val close = HttpMessage.connectionCloseExpected(protocol, ch)
setCompletionHandling(CompletionIsEntityStreamError)
parseEntity(headers.toList, protocol, input, lineEnd, clh, cth, teh, e100c, hh, close, lastSession)
parseEntity(headers.toList, protocol, input, lineEnd, clh, cth, isChunked, e100c, hh, close, lastSession)

case h: `Content-Length` => clh match {
case None => parseHeaderLines(input, lineEnd, headers, headerCount + 1, ch, Some(h), cth, teh, e100c, hh)
case Some(`h`) => parseHeaderLines(input, lineEnd, headers, headerCount, ch, clh, cth, teh, e100c, hh)
case None => parseHeaderLines(input, lineEnd, headers, headerCount + 1, ch, Some(h), cth, isChunked, e100c, hh)
case Some(`h`) => parseHeaderLines(input, lineEnd, headers, headerCount, ch, clh, cth, isChunked, e100c, hh)
case _ => failMessageStart("HTTP message must not contain more than one Content-Length header")
}
case h: `Content-Type` => cth match {
case None =>
parseHeaderLines(input, lineEnd, headers, headerCount + 1, ch, clh, Some(h), teh, e100c, hh)
parseHeaderLines(input, lineEnd, headers, headerCount + 1, ch, clh, Some(h), isChunked, e100c, hh)
case Some(`h`) =>
parseHeaderLines(input, lineEnd, headers, headerCount, ch, clh, cth, teh, e100c, hh)
parseHeaderLines(input, lineEnd, headers, headerCount, ch, clh, cth, isChunked, e100c, hh)
case Some(`Content-Type`(ContentTypes.`NoContentType`)) => // never encountered except when parsing conflicting headers (see below)
parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, teh, e100c, hh)
parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, isChunked, e100c, hh)
case Some(x) =>
import ConflictingContentTypeHeaderProcessingMode._
settings.conflictingContentTypeHeaderProcessingMode match {
case Error => failMessageStart("HTTP message must not contain more than one Content-Type header")
case First => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, teh, e100c, hh)
case Last => parseHeaderLines(input, lineEnd, headers += x, headerCount + 1, ch, clh, Some(h), teh, e100c, hh)
case NoContentType => parseHeaderLines(input, lineEnd, headers += x += h, headerCount + 1, ch, clh, Some(`Content-Type`(ContentTypes.`NoContentType`)), teh, e100c, hh)
case First => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, isChunked, e100c, hh)
case Last => parseHeaderLines(input, lineEnd, headers += x, headerCount + 1, ch, clh, Some(h), isChunked, e100c, hh)
case NoContentType => parseHeaderLines(input, lineEnd, headers += x += h, headerCount + 1, ch, clh, Some(`Content-Type`(ContentTypes.`NoContentType`)), isChunked, e100c, hh)
}
}
case h: `Transfer-Encoding` => teh match {
case None => parseHeaderLines(input, lineEnd, headers, headerCount + 1, ch, clh, cth, Some(h), e100c, hh)
case Some(x) => parseHeaderLines(input, lineEnd, headers, headerCount, ch, clh, cth, Some(x append h.encodings), e100c, hh)
}
case h: `Transfer-Encoding` =>
if (!isChunked) {
h.encodings match {
case Seq(TransferEncodings.chunked) =>
// A single chunked is the only one we support
parseHeaderLines(input, lineEnd, headers, headerCount + 1, ch, clh, cth, isChunked = true, e100c, hh)
case Seq(unknown) =>
failMessageStart(s"Unsupported Transfer-Encoding '${unknown.name}'")
case _ =>
failMessageStart("Multiple Transfer-Encoding entries not supported")

}
} else {
// only allow one 'chunked'
failMessageStart("Multiple Transfer-Encoding entries not supported")
}
case h: Connection => ch match {
case None => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, Some(h), clh, cth, teh, e100c, hh)
case Some(x) => parseHeaderLines(input, lineEnd, headers, headerCount, Some(x append h.tokens), clh, cth, teh, e100c, hh)
case None => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, Some(h), clh, cth, isChunked, e100c, hh)
case Some(x) => parseHeaderLines(input, lineEnd, headers, headerCount, Some(x append h.tokens), clh, cth, isChunked, e100c, hh)
}
case h: Host =>
if (!hh || isResponseParser) parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, teh, e100c, hh = true)
if (!hh || isResponseParser) parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, isChunked, e100c, hh = true)
else failMessageStart("HTTP message must not contain more than one Host header")

case h: Expect => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, teh, e100c = true, hh)
case h: Expect => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, isChunked, e100c = true, hh)

case h => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, teh, e100c, hh)
case h => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, ch, clh, cth, isChunked, e100c, hh)
}
} else failMessageStart(s"HTTP message contains more than the configured limit of ${settings.maxHeaderCount} headers")

// work-around for compiler complaining about non-tail-recursion if we inline this method
private def parseHeaderLinesAux(headers: ListBuffer[HttpHeader], headerCount: Int, ch: Option[Connection],
clh: Option[`Content-Length`], cth: Option[`Content-Type`], teh: Option[`Transfer-Encoding`],
clh: Option[`Content-Length`], cth: Option[`Content-Type`], isChunked: Boolean,
e100c: Boolean, hh: Boolean)(input: ByteString, lineStart: Int): StateResult =
parseHeaderLines(input, lineStart, headers, headerCount, ch, clh, cth, teh, e100c, hh)
parseHeaderLines(input, lineStart, headers, headerCount, ch, clh, cth, isChunked, e100c, hh)

protected final def parseFixedLengthBody(
remainingBodyBytes: Long,
Expand Down Expand Up @@ -372,12 +384,6 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] {
HttpEntity.Chunked(contentType(cth), chunks)
}

protected final def addTransferEncodingWithChunkedPeeled(headers: List[HttpHeader], teh: `Transfer-Encoding`): List[HttpHeader] =
teh.withChunkedPeeled match {
case Some(x) => x :: headers
case None => headers
}

protected final def setCompletionHandling(completionHandling: CompletionHandling): Unit =
this.completionHandling = completionHandling

Expand Down
63 changes: 29 additions & 34 deletions akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpRequestParser.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ private[http] final class HttpRequestParser(

// http://tools.ietf.org/html/rfc7230#section-3.3
override def parseEntity(headers: List[HttpHeader], protocol: HttpProtocol, input: ByteString, bodyStart: Int,
clh: Option[`Content-Length`], cth: Option[`Content-Type`], teh: Option[`Transfer-Encoding`],
clh: Option[`Content-Length`], cth: Option[`Content-Type`], isChunked: Boolean,
expect100continue: Boolean, hostHeaderPresent: Boolean, closeAfterResponseCompletion: Boolean,
sslSession: SSLSession): StateResult =
if (hostHeaderPresent || protocol == HttpProtocols.`HTTP/1.0`) {
Expand Down Expand Up @@ -199,40 +199,35 @@ private[http] final class HttpRequestParser(
emit(requestStart)
}

teh match {
case None =>
val contentLength = clh match {
case Some(`Content-Length`(len)) => len
case None => 0
}
if (contentLength == 0) {
emitRequestStart(emptyEntity(cth))
setCompletionHandling(HttpMessageParser.CompletionOk)
startNewMessage(input, bodyStart)
} else if (!method.isEntityAccepted) {
failMessageStart(UnprocessableEntity, s"${method.name} requests must not have an entity")
} else if (contentLength <= input.size - bodyStart) {
val cl = contentLength.toInt
emitRequestStart(strictEntity(cth, input, bodyStart, cl))
setCompletionHandling(HttpMessageParser.CompletionOk)
startNewMessage(input, bodyStart + cl)
} else {
emitRequestStart(defaultEntity(cth, contentLength))
parseFixedLengthBody(contentLength, closeAfterResponseCompletion)(input, bodyStart)
}

case Some(_) if !method.isEntityAccepted =>
if (!isChunked) {
val contentLength = clh match {
case Some(`Content-Length`(len)) => len
case None => 0
}
if (contentLength == 0) {
emitRequestStart(emptyEntity(cth))
setCompletionHandling(HttpMessageParser.CompletionOk)
startNewMessage(input, bodyStart)
} else if (!method.isEntityAccepted) {
failMessageStart(UnprocessableEntity, s"${method.name} requests must not have an entity")

case Some(te) =>
val completedHeaders = addTransferEncodingWithChunkedPeeled(headers, te)
if (te.isChunked) {
if (clh.isEmpty) {
emitRequestStart(chunkedEntity(cth), completedHeaders)
parseChunk(input, bodyStart, closeAfterResponseCompletion, totalBytesRead = 0L)
} else failMessageStart("A chunked request must not contain a Content-Length header.")
} else parseEntity(completedHeaders, protocol, input, bodyStart, clh, cth, teh = None,
expect100continue, hostHeaderPresent, closeAfterResponseCompletion, sslSession)
} else if (contentLength <= input.size - bodyStart) {
val cl = contentLength.toInt
emitRequestStart(strictEntity(cth, input, bodyStart, cl))
setCompletionHandling(HttpMessageParser.CompletionOk)
startNewMessage(input, bodyStart + cl)
} else {
emitRequestStart(defaultEntity(cth, contentLength))
parseFixedLengthBody(contentLength, closeAfterResponseCompletion)(input, bodyStart)
}
} else {
if (!method.isEntityAccepted) {
failMessageStart(UnprocessableEntity, s"${method.name} requests must not have an entity")
} else {
if (clh.isEmpty) {
emitRequestStart(chunkedEntity(cth), headers)
parseChunk(input, bodyStart, closeAfterResponseCompletion, totalBytesRead = 0L)
} else failMessageStart("A chunked request must not contain a Content-Length header")
}
}
} else failMessageStart("Request is missing required `Host` header")

Expand Down
64 changes: 30 additions & 34 deletions akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpResponseParser.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private[http] class HttpResponseParser(protected val settings: ParserSettings, p

// http://tools.ietf.org/html/rfc7230#section-3.3
protected final def parseEntity(headers: List[HttpHeader], protocol: HttpProtocol, input: ByteString, bodyStart: Int,
clh: Option[`Content-Length`], cth: Option[`Content-Type`], teh: Option[`Transfer-Encoding`],
clh: Option[`Content-Length`], cth: Option[`Content-Type`], isChunked: Boolean,
expect100continue: Boolean, hostHeaderPresent: Boolean, closeAfterResponseCompletion: Boolean,
sslSession: SSLSession): StateResult = {

Expand Down Expand Up @@ -180,41 +180,37 @@ private[http] class HttpResponseParser(protected val settings: ParserSettings, p
}
case HttpMethods.CONNECT =>
finishEmptyResponse()
case _ => teh match {
case None => clh match {
case Some(`Content-Length`(contentLength)) =>
if (contentLength == 0) finishEmptyResponse()
else if (contentLength <= input.size - bodyStart) {
val cl = contentLength.toInt
emitResponseStart(strictEntity(cth, input, bodyStart, cl))
setCompletionHandling(HttpMessageParser.CompletionOk)
emit(MessageEnd)
startNewMessage(input, bodyStart + cl)
} else {
emitResponseStart(defaultEntity(cth, contentLength))
parseFixedLengthBody(contentLength, closeAfterResponseCompletion)(input, bodyStart)
}
case None =>
emitResponseStart {
StreamedEntityCreator { entityParts =>
val data = entityParts.collect { case EntityPart(bytes) => bytes }
HttpEntity.CloseDelimited(contentType(cth), data)
case _ =>
if (!isChunked) {
clh match {
case Some(`Content-Length`(contentLength)) =>
if (contentLength == 0) finishEmptyResponse()
else if (contentLength <= input.size - bodyStart) {
val cl = contentLength.toInt
emitResponseStart(strictEntity(cth, input, bodyStart, cl))
setCompletionHandling(HttpMessageParser.CompletionOk)
emit(MessageEnd)
startNewMessage(input, bodyStart + cl)
} else {
emitResponseStart(defaultEntity(cth, contentLength))
parseFixedLengthBody(contentLength, closeAfterResponseCompletion)(input, bodyStart)
}
}
setCompletionHandling(HttpMessageParser.CompletionOk)
parseToCloseBody(input, bodyStart, totalBytesRead = 0)
case None =>
emitResponseStart {
StreamedEntityCreator { entityParts =>
val data = entityParts.collect { case EntityPart(bytes) => bytes }
HttpEntity.CloseDelimited(contentType(cth), data)
}
}
setCompletionHandling(HttpMessageParser.CompletionOk)
parseToCloseBody(input, bodyStart, totalBytesRead = 0)
}
} else {
if (clh.isEmpty) {
emitResponseStart(chunkedEntity(cth), headers)
parseChunk(input, bodyStart, closeAfterResponseCompletion, totalBytesRead = 0L)
} else failMessageStart("A chunked response must not contain a Content-Length header")
}

case Some(te) =>
val completedHeaders = addTransferEncodingWithChunkedPeeled(headers, te)
if (te.isChunked) {
if (clh.isEmpty) {
emitResponseStart(chunkedEntity(cth), completedHeaders)
parseChunk(input, bodyStart, closeAfterResponseCompletion, totalBytesRead = 0L)
} else failMessageStart("A chunked response must not contain a Content-Length header.")
} else parseEntity(completedHeaders, protocol, input, bodyStart, clh, cth, teh = None,
expect100continue, hostHeaderPresent, closeAfterResponseCompletion, sslSession)
}
}
} else finishEmptyResponse()
}
Expand Down
Loading

0 comments on commit e3a4935

Please sign in to comment.