Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle multipart form upload with empty file as BadPart #8882

Merged
merged 9 commits into from
Dec 18, 2018

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Dec 14, 2018

#8880 was for empty filenames, this is for empty files.

@mkurz mkurz added this to the Play 2.7.0 milestone Dec 14, 2018
Copy link
Member

@marcospereira marcospereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments.

@mkurz
Copy link
Member Author

mkurz commented Dec 17, 2018

@marcospereira Please have a look again. Now an empty file will not even be created anymore. I added a checkEmptyBody method which finds out if the body of a multipart element is empty and calls corresponding methods.

Copy link
Member

@marcospereira marcospereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Thanks, @mkurz.

@@ -340,11 +340,11 @@ object Multipart {
def parseHeader(input: ByteString, headerStart: Int, memoryBufferSize: Int): StateResult = {
input.indexOfSlice(crlfcrlf, headerStart) match {
case -1 if input.length - headerStart >= maxHeaderSize =>
bufferExceeded("Header length exceeded buffer size of " + memoryBufferSize)
bufferExceeded("Header length exceeded maximum header size of " + maxHeaderSize)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@mkurz mkurz merged commit f031b20 into playframework:master Dec 18, 2018
@mkurz mkurz deleted the multipartFixZeroFilesize branch December 18, 2018 18:35
@mkurz
Copy link
Member Author

mkurz commented Jan 9, 2019

This still needs to be backported to 2.7.x, seems like it was overlooked.

octonato pushed a commit that referenced this pull request Jan 10, 2019
* Handle multipart form upload with empty file as BadPart

* Fix failing test because empty file isn't available anymore

* Better error log

* Revert original fix

* Handle multipart form upload with empty file as BadPart

* More test cases

* Correct error message

* Migration notes + mention behaviour in upload docs

* One more test (file with newline only)
@octonato
Copy link
Contributor

Backported to 2.7.x (cc9783d)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants