Skip to content

Commit

Permalink
Merge pull request #10236 from mkurz/non_empty_file
Browse files Browse the repository at this point in the history 
Allow file uploads with empty body or empty filenames
  • Loading branch information
@mergify
mergify[bot] committed May 19, 2020
2 parents 29fdb3e + 3ea4cef commit 2fa17aa
Show file tree
Hide file tree
Showing 11 changed files with 317 additions and 32 deletions.
134 changes: 132 additions & 2 deletions ...play-integration-test/src/it/scala/play/it/http/parsing/MultipartFormDataParserSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
|
|text field with unquoted name and colon
|--aabbccddee
|Content-Disposition: form-data; name="empty_text"
|
|
|--aabbccddee
|Content-Disposition: form-data; name=""
|
|empty name should work
|--aabbccddee
|Content-Disposition: form-data; name="arr[]"
|
|array value 0
Expand Down Expand Up @@ -111,6 +119,11 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
|the fifth file (with empty filename)
|
|--aabbccddee
|Content-Disposition: form-data; name="empty_file_empty_filename"; filename=""
|Content-Type: application/octet-stream
|
|
|--aabbccddee
|Content-Disposition: form-data; name="empty_file_bottom"; filename="empty_file_not_followed_by_any_other_part.txt"
|Content-Type: text/plain
|
Expand All @@ -123,11 +136,13 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
def checkResult(result: Either[Result, MultipartFormData[TemporaryFile]]) = {
result must beRight.like {
case parts =>
parts.dataParts must haveLength(7)
parts.dataParts must haveLength(9)
parts.dataParts.get("text1") must beSome(Seq("the first text field"))
parts.dataParts.get("text2:colon") must beSome(Seq("the second text field"))
parts.dataParts.get("noQuotesText1") must beSome(Seq("text field with unquoted name"))
parts.dataParts.get("noQuotesText1:colon") must beSome(Seq("text field with unquoted name and colon"))
parts.dataParts.get("empty_text") must beSome(Seq(""))
parts.dataParts.get("") must beSome(Seq("empty name should work"))
parts.dataParts.get("arr[]").get must contain(("array value 0"))
parts.dataParts.get("arr[]").get must contain(("array value 1"))
parts.dataParts.get("orderedarr[0]") must beSome(Seq("ordered array value 0"))
Expand Down Expand Up @@ -157,7 +172,7 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
parts.file("file_with_newline_only") must beSome.like {
case filePart => PlayIO.readFileAsString(filePart.ref) must_== "\r\n"
}
parts.badParts must haveLength(4)
parts.badParts must haveLength(5)
parts.badParts must contain(
(BadPart(
Map(
Expand All @@ -182,6 +197,14 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
)
))
)
parts.badParts must contain(
(BadPart(
Map(
"content-disposition" -> """form-data; name="empty_file_empty_filename"; filename=""""",
"content-type" -> "application/octet-stream"
)
))
)
parts.badParts must contain(
(BadPart(
Map(
Expand All @@ -193,6 +216,84 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
}
}

def checkResultEmptyFileAllowed(result: Either[Result, MultipartFormData[TemporaryFile]]) = {
result must beRight.like {
case parts =>
parts.dataParts must haveLength(9)
parts.dataParts.get("text1") must beSome(Seq("the first text field"))
parts.dataParts.get("text2:colon") must beSome(Seq("the second text field"))
parts.dataParts.get("noQuotesText1") must beSome(Seq("text field with unquoted name"))
parts.dataParts.get("noQuotesText1:colon") must beSome(Seq("text field with unquoted name and colon"))
parts.dataParts.get("empty_text") must beSome(Seq(""))
parts.dataParts.get("") must beSome(Seq("empty name should work"))
parts.dataParts.get("arr[]").get must contain(("array value 0"))
parts.dataParts.get("arr[]").get must contain(("array value 1"))
parts.dataParts.get("orderedarr[0]") must beSome(Seq("ordered array value 0"))
parts.dataParts.get("orderedarr[1]") must beSome(Seq("ordered array value 1"))
parts.files must haveLength(10)
parts.file("file1") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== "the first file\r\n"
filePart.fileSize must_== 16
}
}
parts.file("file2") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== "the second file\r\n"
filePart.fileSize must_== 17
}
}
parts.file("file3") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== "the third file (with 'Content-Disposition: file' instead of 'form-data' as used in webhook callbacks of some scanners, see issue #8527)\r\n"
filePart.fileSize must_== 137
}
}
parts.file("file_with_space_only") must beSome.like {
case filePart => PlayIO.readFileAsString(filePart.ref) must_== " "
}
parts.file("file_with_newline_only") must beSome.like {
case filePart => PlayIO.readFileAsString(filePart.ref) must_== "\r\n"
}
parts.file("empty_file_middle") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== ""
filePart.fileSize must_== 0
filePart.filename must_== "empty_file_followed_by_other_part.txt"
}
}
parts.file("file4") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== "the fourth file (with empty filename)\r\n"
filePart.fileSize must_== 39
filePart.filename must_== ""
}
}
parts.file("file5") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== "the fifth file (with empty filename)\r\n"
filePart.fileSize must_== 38
filePart.filename must_== ""
}
}
parts.file("empty_file_empty_filename") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== ""
filePart.fileSize must_== 0
filePart.filename must_== ""
}
}
parts.file("empty_file_bottom") must beSome.like {
case filePart => {
PlayIO.readFileAsString(filePart.ref) must_== ""
filePart.fileSize must_== 0
filePart.filename must_== "empty_file_not_followed_by_any_other_part.txt"
}
}
parts.badParts must haveLength(0)
}
}

def withClientAndServer[T](totalSpace: Long)(block: WSClient => T) = {
Server.withApplicationFromContext() { context =>
new BuiltInComponentsFromContext(context) with NoHttpFiltersComponents {
Expand Down Expand Up @@ -224,6 +325,20 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
checkResult(result)
}

"parse some content with empty file allowed" in new WithApplication() {
val parser = parse
.multipartFormData(allowEmptyFiles = true)
.apply(
FakeRequest().withHeaders(
CONTENT_TYPE -> "multipart/form-data; boundary=aabbccddee"
)
)

val result = await(parser.run(Source.single(ByteString(body))))

checkResultEmptyFileAllowed(result)
}

"parse some content that arrives one byte at a time" in new WithApplication() {
val parser = parse.multipartFormData.apply(
FakeRequest().withHeaders(
Expand All @@ -237,6 +352,21 @@ class MultipartFormDataParserSpec extends PlaySpecification with WsTestClient {
checkResult(result)
}

"parse some content that arrives one byte at a time with empty file allowed" in new WithApplication() {
val parser = parse
.multipartFormData(allowEmptyFiles = true)
.apply(
FakeRequest().withHeaders(
CONTENT_TYPE -> "multipart/form-data; boundary=aabbccddee"
)
)

val bytes = body.getBytes.map(byte => ByteString(byte)).toVector
val result = await(parser.run(Source(bytes)))

checkResultEmptyFileAllowed(result)
}

"return bad request for invalid body" in new WithApplication() {
val parser = parse.multipartFormData.apply(
FakeRequest().withHeaders(
Expand Down
34 changes: 30 additions & 4 deletions core/play/src/main/java/play/mvc/BodyParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,9 +523,19 @@ public MultipartFormData(PlayBodyParsers parsers) {
super(parsers.multipartFormData(), JavaParsers::toJavaMultipartFormData);
}

public MultipartFormData(PlayBodyParsers parsers, boolean allowEmptyFiles) {
super(parsers.multipartFormData(allowEmptyFiles), JavaParsers::toJavaMultipartFormData);
}

public MultipartFormData(PlayBodyParsers parsers, long maxLength) {
super(parsers.multipartFormData(maxLength), JavaParsers::toJavaMultipartFormData);
}

public MultipartFormData(PlayBodyParsers parsers, long maxLength, boolean allowEmptyFiles) {
super(
parsers.multipartFormData(maxLength, allowEmptyFiles),
JavaParsers::toJavaMultipartFormData);
}
}

/** Don't parse the body. */
Expand Down Expand Up @@ -706,19 +716,34 @@ public DelegatingMultipartFormDataBodyParser(
this.materializer = materializer;
this.errorHandler = errorHandler;
this.maxMemoryBufferSize = 102400; // 100k, default for play.http.parser.maxMemoryBuffer
delegate = multipartParser();
delegate = multipartParser(false);
}

/**
* @deprecated Deprecated as of 2.9.0. Use {@link
* #DelegatingMultipartFormDataBodyParser(Materializer, long, long, boolean,
* HttpErrorHandler)} instead.
*/
@Deprecated
public DelegatingMultipartFormDataBodyParser(
Materializer materializer,
long maxMemoryBufferSize,
long maxLength,
HttpErrorHandler errorHandler) {
this(materializer, maxMemoryBufferSize, maxLength, false, errorHandler);
}

public DelegatingMultipartFormDataBodyParser(
Materializer materializer,
long maxMemoryBufferSize,
long maxLength,
boolean allowEmptyFiles,
HttpErrorHandler errorHandler) {
super(maxLength, errorHandler);
this.materializer = materializer;
this.maxMemoryBufferSize = maxMemoryBufferSize;
this.errorHandler = new JavaHttpErrorHandlerAdapter(errorHandler);
delegate = multipartParser();
delegate = multipartParser(allowEmptyFiles);
}

/**
Expand All @@ -732,10 +757,11 @@ public DelegatingMultipartFormDataBodyParser(
createFilePartHandler();

/** Calls out to the Scala API to create a multipart parser. */
private play.api.mvc.BodyParser<play.api.mvc.MultipartFormData<A>> multipartParser() {
private play.api.mvc.BodyParser<play.api.mvc.MultipartFormData<A>> multipartParser(
boolean allowEmptyFiles) {
ScalaFilePartHandler filePartHandler = new ScalaFilePartHandler();
return Multipart.multipartParser(
maxMemoryBufferSize, filePartHandler, errorHandler, materializer);
maxMemoryBufferSize, allowEmptyFiles, filePartHandler, errorHandler, materializer);
}

private class ScalaFilePartHandler
Expand Down
3 changes: 3 additions & 0 deletions core/play/src/main/resources/reference.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ play {

# The maximum amount of a request body that should be buffered into disk
maxDiskBuffer = 10m

# If empty multipart/form-data file uploads are allowed (no matter if filename or file is empty)
allowEmptyFiles = false
}

# Action composition configuration
Expand Down
10 changes: 8 additions & 2 deletions core/play/src/main/scala/play/api/http/HttpConfiguration.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,13 @@ case class FlashConfiguration(
*
* @param maxMemoryBuffer The maximum size that a request body that should be buffered in memory.
* @param maxDiskBuffer The maximum size that a request body should be buffered on disk.
* @param allowEmptyFiles If empty file uploads are allowed (no matter if filename or file is empty)
*/
case class ParserConfiguration(maxMemoryBuffer: Long = 102400, maxDiskBuffer: Long = 10485760)
case class ParserConfiguration(
maxMemoryBuffer: Long = 102400,
maxDiskBuffer: Long = 10485760,
allowEmptyFiles: Boolean = false
)

/**
* Configuration for action composition.
Expand Down Expand Up @@ -240,7 +245,8 @@ object HttpConfiguration {
parser = ParserConfiguration(
maxMemoryBuffer =
config.getDeprecated[ConfigMemorySize]("play.http.parser.maxMemoryBuffer", "parsers.text.maxLength").toBytes,
maxDiskBuffer = config.get[ConfigMemorySize]("play.http.parser.maxDiskBuffer").toBytes
maxDiskBuffer = config.get[ConfigMemorySize]("play.http.parser.maxDiskBuffer").toBytes,
allowEmptyFiles = config.get[Boolean]("play.http.parser.allowEmptyFiles")
),
actionComposition = ActionCompositionConfiguration(
controllerAnnotationsFirst = config.get[Boolean]("play.http.actionComposition.controllerAnnotationsFirst"),
Expand Down
58 changes: 54 additions & 4 deletions core/play/src/main/scala/play/api/mvc/BodyParsers.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,17 @@ trait PlayBodyParsers extends BodyParserUtils {
*/
def DefaultMaxDiskLength: Long = config.maxDiskBuffer

/**
* If empty file uploads are allowed (no matter if filename or file is empty)
*
* You can configure it in application.conf:
*
* {{{
* play.http.parser.allowEmptyFiles = true
* }}}
*/
def DefaultAllowEmptyFileUploads: Boolean = config.allowEmptyFiles

// -- Text parser

/**
Expand Down Expand Up @@ -922,8 +933,11 @@ trait PlayBodyParsers extends BodyParserUtils {

case Some("multipart/form-data") =>
logger.trace("Parsing AnyContent as multipartFormData")
multipartFormData(Multipart.handleFilePartAsTemporaryFile(temporaryFileCreator), maxLengthOrDefaultLarge)
.apply(request)
multipartFormData(
Multipart.handleFilePartAsTemporaryFile(temporaryFileCreator),
maxLengthOrDefaultLarge,
DefaultAllowEmptyFileUploads
).apply(request)
.map(_.right.map(m => AnyContentAsMultipartFormData(m)))

case _ =>
Expand All @@ -948,22 +962,58 @@ trait PlayBodyParsers extends BodyParserUtils {
def multipartFormData(maxLength: Long): BodyParser[MultipartFormData[TemporaryFile]] =
multipartFormData(Multipart.handleFilePartAsTemporaryFile(temporaryFileCreator), maxLength)

/**
* Parse the content as multipart/form-data
*
* @param allowEmptyFiles If empty file uploads are allowed (no matter if filename or file is empty)
*/
def multipartFormData(allowEmptyFiles: Boolean): BodyParser[MultipartFormData[TemporaryFile]] =
multipartFormData(Multipart.handleFilePartAsTemporaryFile(temporaryFileCreator), allowEmptyFiles = allowEmptyFiles)

/**
* Parse the content as multipart/form-data
*
* @param maxLength Max length (in bytes) allowed or returns EntityTooLarge HTTP response.
* @param allowEmptyFiles If empty file uploads are allowed (no matter if filename or file is empty)
*/
def multipartFormData(maxLength: Long, allowEmptyFiles: Boolean): BodyParser[MultipartFormData[TemporaryFile]] =
multipartFormData(Multipart.handleFilePartAsTemporaryFile(temporaryFileCreator), maxLength, allowEmptyFiles)

/**
* Parse the content as multipart/form-data
*
* @param filePartHandler Handles file parts.
* @param maxLength Max length (in bytes) allowed or returns EntityTooLarge HTTP response.
*
* @see [[DefaultMaxDiskLength]]
* @see [[Results.EntityTooLarge]]
*
* @deprecated Since 2.9.0. Use the overloaded multipartFormData method that takes the allowEmptyFiles flag.
*/
@deprecated("Use the overloaded multipartFormData method that takes the allowEmptyFiles flag", "2.9.0")
def multipartFormData[A](
filePartHandler: Multipart.FilePartHandler[A],
maxLength: Long
): BodyParser[MultipartFormData[A]] = multipartFormData(filePartHandler, maxLength, false)

/**
* Parse the content as multipart/form-data
*
* @param filePartHandler Handles file parts.
* @param maxLength Max length (in bytes) allowed or returns EntityTooLarge HTTP response.
* @param allowEmptyFiles If empty file uploads are allowed (no matter if filename or file is empty)
*
* @see [[DefaultMaxDiskLength]]
* @see [[Results.EntityTooLarge]]
*/
def multipartFormData[A](
filePartHandler: Multipart.FilePartHandler[A],
maxLength: Long = DefaultMaxDiskLength
maxLength: Long = DefaultMaxDiskLength,
allowEmptyFiles: Boolean = DefaultAllowEmptyFileUploads
): BodyParser[MultipartFormData[A]] = {
BodyParser("multipartFormData") { request =>
val bodyAccumulator =
Multipart.multipartParser(DefaultMaxTextLength, filePartHandler, errorHandler).apply(request)
Multipart.multipartParser(DefaultMaxTextLength, allowEmptyFiles, filePartHandler, errorHandler).apply(request)
enforceMaxLength(request, maxLength, bodyAccumulator)
}
}
Expand Down

0 comments on commit 2fa17aa

Please sign in to comment.