From ee8a5e4a74206f37935d87b322b68df6073f3b6d Mon Sep 17 00:00:00 2001 From: Collin Burger Date: Thu, 27 Jun 2019 16:10:29 -0400 Subject: [PATCH] Fix request body line parsing: Issue #2581 * Improved the heuristic for detecting end of line character(s) * Added tests for UndefinedEndOfLineConfiguration --- .../impl/engine/parsing/BodyPartParser.scala | 9 ++- .../engine/parsing/RequestParserSpec.scala | 63 ++++++++++++++++++- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/BodyPartParser.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/BodyPartParser.scala index 360f4b8a90b..9817b601f96 100644 --- a/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/BodyPartParser.scala +++ b/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/BodyPartParser.scala @@ -339,11 +339,10 @@ private[http] object BodyPartParser { override def eol: String = "\r\n" override def defineOnce(byteString: ByteString): EndOfLineConfiguration = { - // Hypothesis: There is either CRLF or LF as EOL, no mix possible - val cr = '\r'.toByte - val lf = '\n'.toByte - if (byteString.contains(cr)) DefinedEndOfLineConfiguration("\r\n", boundary) - else if (byteString.contains(lf)) DefinedEndOfLineConfiguration("\n", boundary) + val crNeedle = ByteString(s"$boundary\r\n") + val lfNeedle = ByteString(s"$boundary\n") + if (byteString.containsSlice(crNeedle)) DefinedEndOfLineConfiguration("\r\n", boundary) + else if (byteString.containsSlice(lfNeedle)) DefinedEndOfLineConfiguration("\n", boundary) else this } } diff --git a/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/RequestParserSpec.scala b/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/RequestParserSpec.scala index db90751470a..9adf43ac34d 100644 --- a/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/RequestParserSpec.scala +++ b/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/RequestParserSpec.scala @@ -5,12 +5,16 @@ package akka.http.impl.engine.parsing import akka.NotUsed +import java.math.BigInteger +import java.nio.file.{ Files, Paths } +import java.util.UUID import scala.concurrent.Future import scala.concurrent.duration._ import com.typesafe.config.{ Config, ConfigFactory } import akka.util.ByteString import akka.actor.ActorSystem +import akka.http.impl.engine.parsing.BodyPartParser.UndefinedEndOfLineConfiguration import akka.stream.ActorMaterializer import akka.stream.scaladsl._ import akka.stream.TLSProtocol._ @@ -18,6 +22,7 @@ import org.scalatest.matchers.Matcher import org.scalatest.{ BeforeAndAfterAll, FreeSpec, Matchers } import akka.http.scaladsl.settings.ParserSettings import akka.http.impl.engine.parsing.ParserOutput._ +import akka.http.impl.util.ExampleHttpContexts.getClass import akka.http.impl.util._ import akka.http.scaladsl.model.HttpEntity._ import akka.http.scaladsl.model.HttpMethods._ @@ -528,13 +533,13 @@ abstract class RequestParserSpec(mode: String, newLine: String) extends FreeSpec "with a too-long header name" in new Test { """|GET / HTTP/1.1 - |UserxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxAgent: curl/7.19.7""" should parseToError( + |UserxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxAgent: curl/7.19.7""" should parseToError( RequestHeaderFieldsTooLarge, ErrorInfo("HTTP header name exceeds the configured limit of 64 characters")) } "with a too-long header-value" in new Test { """|GET / HTTP/1.1 - |Fancy: 123456789012345678901234567890123""" should parseToError( + |Fancy: 123456789012345678901234567890123""" should parseToError( RequestHeaderFieldsTooLarge, ErrorInfo("HTTP header value exceeds the configured limit of 32 characters")) } @@ -674,3 +679,57 @@ abstract class RequestParserSpec(mode: String, newLine: String) extends FreeSpec class RequestParserCRLFSpec extends RequestParserSpec("CRLF", "\r\n") class RequestParserLFSpec extends RequestParserSpec("LF", "\n") + +class RequestParserUndefinedSpec extends FreeSpec with Matchers with BeforeAndAfterAll { + val cr = '\r' + val boundary = UUID.randomUUID().toString + + "Multipart request parsing should" - { + "correctly define the EOL when data contains CR characters and is CRLF-formatted" in { + val requestBody = + s""" + |POST /file HTTP/1.1${cr} + |Host: example.com${cr} + |Content-Type: multipart/form-data; boundary=${boundary}${cr} + |${cr} + |--${boundary}${cr} + |Content-Disposition: form-data; name="foo"${cr} + |${cr} + |bar${cr} + |--${boundary}${cr} + |Content-Disposition: form-data; name="file"; filename="file.bin"${cr} + |Content-Type: application/octet-stream${cr} + |${cr} + |data${cr} + |with + |carriage${cr} + |returns${cr} + |--${boundary}-- + """.stripMargin + UndefinedEndOfLineConfiguration(boundary).defineOnce(ByteString.fromString(requestBody)).eol shouldBe "\r\n" + } + "correctly define the EOL when data contains CR characters and is LF-formatted" in { + val requestBody = + s""" + |POST /file HTTP/1.1 + |Host: example.com + |Content-Type: multipart/form-data; boundary=${boundary} + | + |--${boundary} + |Content-Disposition: form-data; name="foo" + | + |bar + |--${boundary} + |Content-Disposition: form-data; name="file"; filename="file.bin" + |Content-Type: application/octet-stream + | + |data${cr} + |with + |carriage${cr} + |returns + |--${boundary}-- + """.stripMargin + UndefinedEndOfLineConfiguration(boundary).defineOnce(ByteString.fromString(requestBody)).eol shouldBe "\n" + } + } +}