Re: [2.0-scala] maxLength body parser and file uploads from browser

[David Illsley]

Hi,
As it happens I've been trying to do similar things over that last few weeks and similarly haven't had much luck (play-scala 2.0.3). The difference is that I've been trying to set a max size on each of the file parts and when doing that I can't seem to actually stop the input by producing a Done e.g.:

  def handleFilePartAsDescriptiveString: parse.Multipart.PartHandler[MultipartFormData.FilePart[String]] = {
    parse.Multipart.handleFilePart {
      case parse.Multipart.FileInfo(partName, filename, contentType) =>
        Logger.info("About to return Done!")
        Done("Done!",Input.EOF)
    }
  }

  def multipartFormData: BodyParser[MultipartFormData[String]] = parse.multipartFormData(handleFilePartAsDescriptiveString)

  def post = Action(multipartFormData) {
    request =>
      Logger.error("Post")
      println("Post")
      request.body.files.foreach { part =>
        println(part.filename)
      }
      Ok(views.html.index("Your new application is ready."))
  }

... actually consumes the whole request despite Done being the immediate response before finally printing out the filename. (It also does it much more slowly than actually streaming the file straight to disk, but that's less of a concern.)

Producing an Error instead of Done as above is actually even worse as it never calls the controller, nor responds in any way to the browser (much as you describe).

From that I wonder if the problem is to do with the maxLength causing the inner parser to produce an Error which hangs everything.

I don't really have the time at the moment to spend much longer investigating, and I'm far from an expert, but it feels like there's a bug in Play in this area.

David

On Wednesday, September 26, 2012 12:55:25 AM UTC+1, jimr wrote:

Hi,

I've been trying to implement an upper bound on the request body length using the maxLength body parser combined with the multipartFormData body parser (or the raw body parser) so that huge requests can be blocked without having to load all of the data into memory.

Unfortunately this doesn't seem to be working when uploading large files from the browser. When I choose a large file (e.g. 5MB) and submit the <form>, the request hangs in the browser and never completes.  On the server-side, the request completes, but the browser never seems to see the response.

Simple example:

    def upload = Action(parse.maxLength(1048576, parse.multipartFormData)) { request =>

      request.body match {

        case Left(err) => EntityTooLarge("Request too big")

        case Right(body) => Ok("Not too big")

      }

    }

I've been able to reproduce it in Chrome, Firefox, and IE.

Has anyone seen this before and/or have any idea how to work around it?

Thanks,

Jim

[David M.]

Hello,

I also had an hard time trying to implement a file upload process that would directly stream the data to another backend system without caching the uploaded data into a temporary file.

The main problem is that the HTTP protocol is made in a way that the server has to wait until the client has finished sending its whole request body before sending a response. If you send an error code it won't be taken into account before the request has been completely consumed, if you close the connection by sending an Exception the browser will seem to hang so there seems to be no-way to properly cancel a request (from the server point of view) that has been started.

The solution I decided to use for managing my uploads is the following (it requires to write some javascript)

1. Before doing the actual upload I call a specific controller (isUploadValid) that will tell me if the upload will be valid (based on target file size, currently logged-in user account settings, ...)
2. If it's not possible then I simply don't do the upload
3. Else I proceed with the real upload

In the "real" upload Action I have written a specific part handler that handle uploaded data (in my case I just want to stream it somewhere else) that always read the full user-provided data. In case of problem I keep reading the content (but don't use it any more) and when everything has been red I send an error-code.

This is not a perfect solution because in case of problems network resources are still wasted uploading unused data but if your isUploadValid controller is well written you should normally only upload valid data. My post with code sample is on https://groups.google.com/forum/?fromgroups=#!starred/play-framework/n7yF6wNBL_s

Of course if someone is trying to "attack" your site and directly calls the upload controller without going through the check first then this solution won't work, but I guess this is another topic.

Since I have implemented this solution I heard about the expect handshake. HTTP protocol defines an expect 100 continue handshake: in this mode the client starts by only sending the headers of the request (i.e. including content-length) and waits for the server to return either an error code or a 100-continue response. If the later is received then the full request with post content is made. I think that it would be the more elegant way to handle your problem but I don't know if it's supported by the play! framework yet.

Good luck!

[David Illsley]

That's an interesting approach. Unfortunately what I'm trying to do is protect against file upload attacks (both to do with size and files containing viruses), so I really do need a mechanism to cut off the upload once we decide it's a problem.

Cheers,
David

[jimr]

I've done some more investigation and I think the hanging behavior might have something to do with HTTP keep-alive.  When Play finishes a request, if keep-alive was enabled, it leaves the connection open (which is correct).  Unfortunately in the file upload case, not closing the connection after returning a 413 error causes the browser to keep sending data that is never read.

I'm pretty sure this would be fixable if there was a way to tell Play to close the connection even though keep-alive is on.  Unfortunately I can't find a way to tell Play to do it.

Jim

[James Roper]

I would say there are two separate issues here:

1) If Play encounters a problem while consuming the request body, it doesn't make any attempt to finish reading it.  Play (or Netty) should ensure that the body of each request is completely consumed, passing an ignore Iteratee to the body enumerator if the body parser returns Done or Error before consuming it.

2) Play should provide a way to force a connection close.

The first is a bug, the second is a feature request.  Implementing either will solve the problem.  Although the second might be nice, the first is probably the higher priority, and I think also that's how other frameworks work.

On Saturday, 13 October 2012 06:10:18 UTC+11, Benjamin Sellers wrote:

Hello,

I seem to have stumbled across the same issue that you are having. Have you made any progress or is this actually a bug that we should report? It seems like Play should disconnect after sending the 413 or at least finish reading the stream.

[Benjamin Sellers]

I couldn't recreate the bug in 2.1. I actually had created a bug report, but all seems to be well with master. So your problem should be fixed. https://play.lighthouseapp.com/projects/82401/tickets/802-play-doesnt-finish-consuming-request-if-maxlength-defined-causing-browser-hang

[tsuyoshi yoshizawa]

Thank you for replying so quickly.

I was not looking for issue system.

I'm expecting to play 2.1.

2012年11月28日水曜日 12時31分02秒 UTC+8 Benjamin Sellers: