Permalink
Browse files

! http: make multipart form-data more flexible but have it adhere to …

…the RFC more strictly
  • Loading branch information...
dlouwers authored and jrudolph committed Sep 17, 2013
1 parent a995ec6 commit ad593d17f2747df2e4236f1c34b893f2f0ab816e
@@ -11,11 +11,10 @@ import HttpHeaders._
import parser.HttpParser
import HttpHeaders.RawHeader
import spray.io.CommandWrapper
-import spray.util.SprayActorLogging
import scala.annotation.tailrec
-class FileUploadHandler(client: ActorRef, start: ChunkedRequestStart) extends Actor with SprayActorLogging {
+class FileUploadHandler(client: ActorRef, start: ChunkedRequestStart) extends Actor with ActorLogging {
import start.request._
client ! CommandWrapper(SetRequestTimeout(Duration.Inf)) // cancel timeout
@@ -16,29 +16,38 @@
package spray.http
+import spray.http.HttpHeaders._
+
sealed trait HttpForm {
type FieldType
- def fields: Map[String, FieldType]
+ def fields: Seq[FieldType]
}
/**
* Model for `application/x-www-form-urlencoded` form data.
*/
-case class FormData(fields: Map[String, String]) extends HttpForm {
- type FieldType = String
+case class FormData(fields: Seq[(String, String)]) extends HttpForm {
+ type FieldType = (String, String)
}
object FormData {
- val Empty = FormData(Map.empty)
+ val Empty = FormData(Seq.empty)
+ def apply(fields: Map[String, String]): FormData = this(fields.toSeq)
}
/**
* Model for `multipart/form-data` content as defined in RFC 2388.
+ * All parts must contain a Content-Disposition header with a type form-data
+ * and a name parameter that is unique
*/
-case class MultipartFormData(fields: Map[String, BodyPart]) extends HttpForm {
+case class MultipartFormData(fields: Seq[BodyPart]) extends HttpForm {
type FieldType = BodyPart
+ def get(partName: String): Option[BodyPart] = fields.find(_.getName == Some(partName))
}
object MultipartFormData {
- val Empty = MultipartFormData(Map.empty)
+ val Empty = MultipartFormData(Seq.empty)
+ def apply(fields: Map[String, BodyPart]): MultipartFormData = this(fields.map {
+ case (key, value) value.copy(headers = `Content-Disposition`("form-data", Map("name" -> key)) +: value.headers)
+ }.toSeq)
}
@@ -30,4 +30,10 @@ object MultipartContent {
/**
* Model for one part of a multipart message.
*/
-case class BodyPart(entity: HttpEntity, headers: List[HttpHeader] = Nil)
+case class BodyPart(entity: HttpEntity, headers: Seq[HttpHeader] = Nil) {
+ def getNameOrEmpty: String = getName.getOrElse("")
+ def getName: Option[String] = headers.collectFirst {
+ case HttpHeaders.`Content-Disposition`("form-data", parameters) if parameters.contains("name")
+ parameters("name")
+ }
+}
@@ -60,13 +60,7 @@ trait MultipartMarshallers {
ctx.tryAccept(`multipart/form-data` :: Nil) match {
case None ctx.rejectMarshalling(Seq(`multipart/form-data`))
case _ mcm(
- value = MultipartContent {
- value.fields.map {
- case (name, part) if !part.headers.exists(_.is("content-disposition")) part.copy(
- headers = `Content-Disposition`("form-data", Map("name" -> name)) :: part.headers)
- case (name, part) part
- }(collection.breakOut)
- },
+ value = MultipartContent(value.fields),
ctx = new DelegatingMarshallingContext(ctx) {
var boundary = ""
override def tryAccept(contentTypes: Seq[ContentType]) = {
@@ -27,11 +27,11 @@ object FormFieldExtractor {
def apply(form: HttpForm): FormFieldExtractor = form match {
case FormData(fields) new FormFieldExtractor {
type Field = UrlEncodedFormField
- def field(name: String) = new UrlEncodedFormField(name, fields.get(name))
+ def field(name: String) = new UrlEncodedFormField(name, fields.find(_._1 == name).map(_._2))
}
case MultipartFormData(fields) new FormFieldExtractor {
type Field = MultipartFormField
- def field(name: String) = new MultipartFormField(name, fields.get(name))
+ def field(name: String) = new MultipartFormField(name, fields.find(_.getName == Some(name)))
}
}
}
@@ -71,18 +71,27 @@ trait MultipartUnmarshallers {
def unmarshal(entity: HttpEntity) =
MultipartContentUnmarshaller(entity).right.flatMap { mpContent ⇒
- try Right(MultipartFormData(mpContent.parts.map(part ⇒ nameOf(part) -> part)(collection.breakOut)))
- catch {
+ try {
+ checkValid(mpContent.parts)
+ Right(MultipartFormData(mpContent.parts))
+ } catch {
case NonFatal(ex) ⇒
Left(MalformedContent("Illegal multipart/form-data content: " + ex.getMessage.nullAsEmpty, ex))
}
}
- def nameOf(part: BodyPart): String =
- part.headers.mapFind {
+ def checkValid(parts: Seq[BodyPart]): Unit = {
+ parts.foreach { part ⇒
+ if (part.headers.map(_.lowercaseName).toSet.size != part.headers.size)
+ sys.error("duplicate header names")
+ }
+ val contentDispositionNames = parts.map(_.headers.mapFind {
case `Content-Disposition`("form-data", parms) ⇒ parms.get("name")
case _ ⇒ None
- } getOrElse sys.error("unnamed body part (no Content-Disposition header or no 'name' parameter)")
+ } getOrElse sys.error("unnamed body part (no Content-Disposition header or no 'name' parameter)"))
+ if (contentDispositionNames.size != contentDispositionNames.toSet.size)
+ sys.error("duplicate 'name' parameter values in Content-Disposition headers")
+ }
}
}
@@ -26,7 +26,11 @@ import HttpCharsets._
import HttpHeaders._
class MultipartMarshallersSpec extends Specification with MultipartMarshallers {
- override protected val multipartBoundaryRandom = new Random(0) // fix for stable value
+
+ protected class FixedRandom extends Random {
+ override def nextBytes(array: Array[Byte]): Unit = "my-stable-boundary".getBytes("UTF-8").copyToArray(array)
+ }
+ override protected val multipartBoundaryRandom = new FixedRandom // fix for stable value
sequential // required for stable random sequence
@@ -35,9 +39,9 @@ class MultipartMarshallersSpec extends Specification with MultipartMarshallers {
"correctly marshal to multipart content with one empty part" in {
marshal(MultipartContent(Seq(BodyPart("")))) === Right {
HttpEntity(
- contentType = ContentType(`multipart/mixed` withBoundary "YLQguzhR2dR6y5M9vnA5m-bJ"),
- string = """|--YLQguzhR2dR6y5M9vnA5m-bJ
- |--YLQguzhR2dR6y5M9vnA5m-bJ--""".stripMargin.replace(EOL, "\r\n"))
+ contentType = ContentType(`multipart/mixed` withBoundary randomBoundary),
+ string = s"""|--${randomBoundary}
+ |--${randomBoundary}--""".stripMargin.replace(EOL, "\r\n"))
}
}
@@ -50,13 +54,13 @@ class MultipartMarshallersSpec extends Specification with MultipartMarshallers {
headers = `Content-Disposition`("form-data", Map("name" -> "email")) :: Nil))
}
} === Right {
- HttpEntity(ContentType(`multipart/mixed` withBoundary "OvAdT7dw6YwDJfQdPrr4mG2n"),
- """|--OvAdT7dw6YwDJfQdPrr4mG2n
+ HttpEntity(ContentType(`multipart/mixed` withBoundary randomBoundary),
+ s"""|--${randomBoundary}
|Content-Disposition: form-data; name=email
|Content-Type: text/plain; charset=UTF-8
|
|test@there.com
- |--OvAdT7dw6YwDJfQdPrr4mG2n--""".stripMargin.replace(EOL, "\r\n"))
+ |--${randomBoundary}--""".stripMargin.replace(EOL, "\r\n"))
}
}
@@ -70,18 +74,18 @@ class MultipartMarshallersSpec extends Specification with MultipartMarshallers {
RawHeader("Content-Transfer-Encoding", "binary") :: Nil))
}
} === Right {
- HttpEntity(ContentType(`multipart/mixed` withBoundary "K81NVUvwtUAjwptiTenvnC+T"),
- """|--K81NVUvwtUAjwptiTenvnC+T
+ HttpEntity(ContentType(`multipart/mixed` withBoundary randomBoundary),
+ s"""|--${randomBoundary}
|Content-Type: text/plain; charset=US-ASCII
|
|first part, with a trailing linebreak
|
- |--K81NVUvwtUAjwptiTenvnC+T
+ |--${randomBoundary}
|Content-Transfer-Encoding: binary
|Content-Type: application/octet-stream
|
|filecontent
- |--K81NVUvwtUAjwptiTenvnC+T--""".stripMargin.replace(EOL, "\r\n"))
+ |--${randomBoundary}--""".stripMargin.replace(EOL, "\r\n"))
}
}
}
@@ -92,50 +96,50 @@ class MultipartMarshallersSpec extends Specification with MultipartMarshallers {
marshal(MultipartFormData(ListMap("surname" -> BodyPart("Mike"), "age" -> BodyPart(marshal(<int>42</int>).get)))) ===
Right {
HttpEntity(
- contentType = ContentType(`multipart/form-data` withBoundary "WA+a+wgbEuEHsegF8rT18PHQ"),
- string = """|--WA+a+wgbEuEHsegF8rT18PHQ
+ contentType = ContentType(`multipart/form-data` withBoundary randomBoundary),
+ string = s"""|--${randomBoundary}
|Content-Disposition: form-data; name=surname
|Content-Type: text/plain
|
|Mike
- |--WA+a+wgbEuEHsegF8rT18PHQ
+ |--${randomBoundary}
|Content-Disposition: form-data; name=age
|Content-Type: text/xml
|
|<int>42</int>
- |--WA+a+wgbEuEHsegF8rT18PHQ--""".stripMargin.replace(EOL, "\r\n"))
+ |--${randomBoundary}--""".stripMargin.replace(EOL, "\r\n"))
}
}
"correctly marshal 'multipart/form-data' with two fields having a custom Content-Disposition" in {
- marshal(MultipartFormData(ListMap(
- "first-attachment" -> BodyPart(
+ marshal(MultipartFormData(Seq(
+ BodyPart(
HttpEntity(`text/csv`, "name,age\r\n\"John Doe\",20\r\n"),
- List(`Content-Disposition`("form-data", Map("name" -> "attachment", "filename" -> "attachment.csv")))),
- "second-attachment" -> BodyPart(
+ List(`Content-Disposition`("form-data", Map("name" -> "attachment[0]", "filename" -> "attachment.csv")))),
+ BodyPart(
HttpEntity("name,age\r\n\"John Doe\",20\r\n".getBytes),
List(
- `Content-Disposition`("form-data", Map("name" -> "attachment", "filename" -> "attachment.csv")),
+ `Content-Disposition`("form-data", Map("name" -> "attachment[1]", "filename" -> "attachment.csv")),
RawHeader("Content-Transfer-Encoding", "binary")))))) ===
Right {
HttpEntity(
- contentType = ContentType(`multipart/form-data` withBoundary "D2JjRnCSHFBYZ-8g9qgzXpiv"),
- string = """|--D2JjRnCSHFBYZ-8g9qgzXpiv
- |Content-Disposition: form-data; name=attachment; filename=attachment.csv
+ contentType = ContentType(`multipart/form-data` withBoundary randomBoundary),
+ string = s"""|--${randomBoundary}
+ |Content-Disposition: form-data; name="attachment[0]"; filename=attachment.csv
|Content-Type: text/csv
|
|name,age
|"John Doe",20
|
- |--D2JjRnCSHFBYZ-8g9qgzXpiv
- |Content-Disposition: form-data; name=attachment; filename=attachment.csv
+ |--${randomBoundary}
+ |Content-Disposition: form-data; name="attachment[1]"; filename=attachment.csv
|Content-Transfer-Encoding: binary
|Content-Type: application/octet-stream
|
|name,age
|"John Doe",20
|
- |--D2JjRnCSHFBYZ-8g9qgzXpiv--""".stripMargin.replace(EOL, "\r\n"))
+ |--${randomBoundary}--""".stripMargin.replace(EOL, "\r\n"))
}
}
@@ -88,8 +88,7 @@ class MultipartUnmarshallersSpec extends Specification {
|--XYZABC--""".stripMargin).as[MultipartFormData] === Right {
MultipartFormData(
Map("email" -> BodyPart(
- HttpEntity(ContentType(`text/plain`, `US-ASCII`), "test@there.com"),
- List(`Content-Disposition`("form-data", Map("name" -> "email"))))))
+ HttpEntity(ContentType(`text/plain`, `US-ASCII`), "test@there.com"))))
})
"correctly unmarshal 'multipart/form-data' content mixed with a file" in {
HttpEntity(`multipart/form-data` withBoundary "XYZABC",
@@ -104,7 +103,7 @@ class MultipartUnmarshallersSpec extends Specification {
|
|filecontent
|--XYZABC--""".stripMargin).as[MultipartFormData].get.fields.map {
- case (name, BodyPart(entity, _)) name + ": " + entity.as[String].get
+ case part @ BodyPart(entity, _) part.getName.get + ": " + entity.as[String].get
}.mkString("|") === "email: test@there.com|userfile: filecontent"
}
"reject illegal multipart content" in {
@@ -63,7 +63,7 @@ class AnyParamDirectivesSpec extends RoutingSpec {
}
"extract None if no form" in {
- Post("/test", FormData(Map())) ~> route ~> check {
+ Post("/test", FormData(Seq())) ~> route ~> check {
entityAs[String] === "None"
}
}

0 comments on commit ad593d1

Please sign in to comment.