Behavior when uploading a multipart file which exceeds max content length config


#1

When I use nginx to enforce a max file size on file uploads and I upload a file that’s too large, nginx early-outs immediately (without uploading the entire file) - it must be looking at the Content-Length header.

I want to have Play enforce the max file size. However, when I disable the checking in nginx and allow Play to handle this limit, the limit does get enforced, but the entire file is uploaded to my server. This is not desirable.

My nginx proxy buffering is off.

I have confirmed that in the happy path, my controller method does receive the ‘Content-Length’ header (it’s not being eaten by nginx or anything like that - it’s there). However, in the case that the file is too big, my controller method does not even get called and I see the following in the output:

2018-04-02 20:45:29,705 [ERROR] from akka.stream.impl.io.FileSubscriber in application-akka.actor.default-dispatcher-10 - Tearing down FileSink(/tmp/playtemp8312801549160061678/multipartBody6640999551180422187asTemporaryFile) due to upstream error
play.api.mvc.BodyParsers$MaxLengthLimitAttained: null

When I enable log-config-on-start = true, I see the following:

# This setting is set in `akka.http.server.parsing.max-content-length`
# Play uses the concept of a `BodyParser` to enforce this limit, so we override it to infinite.
"max-content-length" : "infinite",

So Akka is not checking the limit (maybe it would use the Content-Length and early out without allowing the entire file to be uploaded). I tried to set play.http.server.max-content-length = 10m in my application.conf, but it looks like Play overrides it. I was not able to change this value.

Looking at https://github.com/playframework/playframework/blob/b5a88b3af2b806d519cf4d7afad704f847f4899d/framework/src/play/src/main/scala/play/api/mvc/BodyParsers.scala and how DefaultMaxDiskLength is used,

val takeUpToFlow = Flow.fromGraph(new BodyParsers.TakeUpTo(maxLength))

I don’t see anything making use of Content-Length in any of these (body parser) files.

Is it possible to use the Content-Length on the request and early-out with a 413 without allowing the entire file to be uploaded?


(Igmar Palsenberg) #2

If you want to error out early, I would move the check to a filter, and enforce it there.


(Rich Dougherty) #3

It makes sense to me to check the Content-Length if it’s available. Would you consider raising an issue over on the Github repo for Play? Here’s the link: https://github.com/playframework/playframework/issues/new


#4

@igmar: It seems desirable for checking Content-Length, if it’s there, to be the default behavior, as it is with nginx. Why allow an attacker to upload a gigabyte-sized file to your server only to have it be rejected for being over 10 megs?

@richdougherty: Thanks - https://github.com/playframework/playframework/issues/8336


(Rich Dougherty) #5

Just to clarify, only 10 megs should be uploaded before it is rejected, not the full gigabyte. Still, we may as well reject the file before any bytes are uploaded at all if they send a Content-Length.


#6

You’re right - I was testing with a 40MB file and I assumed that the whole thing was being uploaded due to how long it would take to get the 413 (vs it feeling instant with nginx), but I just tested and 10MB is being uploaded. Thanks for the correction.


(Rich Dougherty) #7

Thanks for checking :+1: