From f908a6a9dc93a14d4e5eeb29c91f1b2e6ac0a5d4 Mon Sep 17 00:00:00 2001 From: Konrad Malawski Date: Fri, 6 Nov 2015 16:14:34 +0100 Subject: [PATCH] =str #16927 preserve header order in body parts --- .../http/impl/engine/parsing/BodyPartParser.scala | 15 +++++++++------ .../MultipartUnmarshallersSpec.scala | 13 +++++++++---- 2 files changed, 18 insertions(+), 10 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 f90c82ce9c..f03b0794a2 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 @@ -14,6 +14,8 @@ import akka.http.scaladsl.model._ import akka.http.impl.util._ import headers._ +import scala.collection.mutable.ListBuffer + /** * INTERNAL API * @@ -114,7 +116,7 @@ private[http] final class BodyPartParser(defaultContentType: ContentType, case NotEnoughDataException ⇒ continue(input.takeRight(needle.length + 2), 0)(parsePreamble) } - @tailrec def parseHeaderLines(input: ByteString, lineStart: Int, headers: List[HttpHeader] = Nil, + @tailrec def parseHeaderLines(input: ByteString, lineStart: Int, headers: ListBuffer[HttpHeader] = ListBuffer[HttpHeader](), headerCount: Int = 0, cth: Option[`Content-Type`] = None): StateResult = { def contentType = cth match { @@ -136,27 +138,28 @@ private[http] final class BodyPartParser(defaultContentType: ContentType, case null ⇒ continue(input, lineStart)(parseHeaderLinesAux(headers, headerCount, cth)) case BoundaryHeader ⇒ - emit(BodyPartStart(headers, _ ⇒ HttpEntity.empty(contentType))) + emit(BodyPartStart(headers.toList, _ ⇒ HttpEntity.empty(contentType))) val ix = lineStart + boundaryLength if (crlf(input, ix)) parseHeaderLines(input, ix + 2) else if (doubleDash(input, ix)) terminate() else fail("Illegal multipart boundary in message content") - case HttpHeaderParser.EmptyHeader ⇒ parseEntity(headers, contentType)(input, lineEnd) + case HttpHeaderParser.EmptyHeader ⇒ parseEntity(headers.toList, contentType)(input, lineEnd) case h: `Content-Type` ⇒ if (cth.isEmpty) parseHeaderLines(input, lineEnd, headers, headerCount + 1, Some(h)) else if (cth.get == h) parseHeaderLines(input, lineEnd, headers, headerCount, cth) else fail("multipart part must not contain more than one Content-Type header") - case h if headerCount < maxHeaderCount ⇒ parseHeaderLines(input, lineEnd, h :: headers, headerCount + 1, cth) + case h if headerCount < maxHeaderCount ⇒ + parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, cth) - case _ ⇒ fail(s"multipart part contains more than the configured limit of $maxHeaderCount headers") + case _ ⇒ fail(s"multipart part contains more than the configured limit of $maxHeaderCount headers") } } // work-around for compiler complaining about non-tail-recursion if we inline this method - def parseHeaderLinesAux(headers: List[HttpHeader], headerCount: Int, + def parseHeaderLinesAux(headers: ListBuffer[HttpHeader], headerCount: Int, cth: Option[`Content-Type`])(input: ByteString, lineStart: Int): StateResult = parseHeaderLines(input, lineStart, headers, headerCount, cth) diff --git a/akka-http-tests/src/test/scala/akka/http/scaladsl/unmarshalling/MultipartUnmarshallersSpec.scala b/akka-http-tests/src/test/scala/akka/http/scaladsl/unmarshalling/MultipartUnmarshallersSpec.scala index 1201f08283..1b6d295601 100644 --- a/akka-http-tests/src/test/scala/akka/http/scaladsl/unmarshalling/MultipartUnmarshallersSpec.scala +++ b/akka-http-tests/src/test/scala/akka/http/scaladsl/unmarshalling/MultipartUnmarshallersSpec.scala @@ -106,8 +106,8 @@ class MultipartUnmarshallersSpec extends FreeSpec with Matchers with BeforeAndAf |--XYZABC--""".stripMarginWithNewline("\r\n"))).to[Multipart.General] should haveParts( Multipart.General.BodyPart.Strict( HttpEntity(ContentTypes.`text/plain(UTF-8)`, "test@there.com"), - List(`Content-Disposition`(ContentDispositionTypes.`form-data`, Map("name" -> "email")), - RawHeader("date", "unknown"))))) + List(RawHeader("date", "unknown"), + `Content-Disposition`(ContentDispositionTypes.`form-data`, Map("name" -> "email")))))) "a full example (Strict)" in { Unmarshal(HttpEntity(`multipart/mixed` withBoundary "12345", """preamble and @@ -246,6 +246,8 @@ class MultipartUnmarshallersSpec extends FreeSpec with Matchers with BeforeAndAf """sition: form-data; name="userfile"; filename="test.dat" |Content-Type: application/pdf |Content-Transfer-Encoding: binary + |Content-Additional-1: anything + |Content-Additional-2: really-anything | |filecontent |--XYZABC--""".stripMarginWithNewline("\r\n") @@ -253,8 +255,11 @@ class MultipartUnmarshallersSpec extends FreeSpec with Matchers with BeforeAndAf }) }.to[Multipart.FormData].flatMap(_.toStrict(1.second)) should haveParts( Multipart.FormData.BodyPart.Strict("email", HttpEntity(ContentTypes.`application/octet-stream`, "test@there.com")), - Multipart.FormData.BodyPart.Strict("userfile", HttpEntity(MediaTypes.`application/pdf`, "filecontent"), - Map("filename" -> "test.dat"), List(RawHeader("Content-Transfer-Encoding", "binary")))) + Multipart.FormData.BodyPart.Strict("userfile", HttpEntity(MediaTypes.`application/pdf`, "filecontent"), Map("filename" -> "test.dat"), + List( + RawHeader("Content-Transfer-Encoding", "binary"), + RawHeader("Content-Additional-1", "anything"), + RawHeader("Content-Additional-2", "really-anything")))) // verifies order of headers is preserved } // TODO: reactivate after multipart/form-data unmarshalling integrity verification is implemented //