Fix request body line parsing: Issue akka#2581

* Improved the heuristic for detecting end of line character(s) * Added tests for UndefinedEndOfLineConfiguration
cyburgee · Apr 16, 2020 · ee8a5e4 · ee8a5e4
1 parent c58a0e5
commit ee8a5e4
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 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
@@ -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
@@ -5,19 +5,24 @@
 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._
 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"
+    }
+  }
+}