Handle 413 Response when trying to upload a very large image

[miketaylr]

We set the limit at 4MB, and if you try to upload something larger the error is currently swallowed. We should figure out how to send back a JSON error message and consume that in our regular form validation message stuff.

[karlcow]

I was trying to fix that but I ran into another issue for upload, probably not related.
https://gist.github.com/anonymous/12587e60463b0f628396

[karlcow]

Yeah not related. It's happening on the current master.

[miketaylr]

That's an interesting error, @karlcow. Steps to reproduce?

[karlcow]

Sure https://gist.github.com/anonymous/12587e60463b0f628396#gistcomment-1574599

[karlcow]

@miketaylr ok solved my issue. Do you want to open a bug and make a pull request for it.
Can you reproduce my issue?
Solution https://gist.github.com/anonymous/12587e60463b0f628396#gistcomment-1574599

I can go back to this bug. :)

[karlcow]

Back to the bug. Testing.

ok tested the 413 error handler.

→ python run.py
 * Running on http://localhost:5000/ (Press CTRL+C to quit)
 * Restarting with stat
127.0.0.1 - - [16/Sep/2015 14:53:21] "GET / HTTP/1.1" 200 -
127.0.0.1 - - [16/Sep/2015 14:53:24] "GET /api/issues/category/new HTTP/1.1" 200 -
127.0.0.1 - - [16/Sep/2015 14:56:49] "POST / HTTP/1.1" 413 -
127.0.0.1 - - [16/Sep/2015 14:57:39] "POST / HTTP/1.1" 413 -
127.0.0.1 - - [16/Sep/2015 14:57:39] "POST / HTTP/1.1" 413 -

We receive a 413 but I guess it's just for now the behavior of the Upload extension.
The interesting part for me was that it failed with "The connection was reset" and no return to anywhere.
This will have to be solved in my PR.

[karlcow]

Ah ok http://flask.pocoo.org/docs/0.10/patterns/fileuploads/#improving-uploads

The request we send:

POST / HTTP/1.1
Host: localhost:5000
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:43.0) Gecko/20100101 Firefox/43.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Encoding: gzip, deflate
Referer: http://localhost:5000/
Cookie: session=*****
Connection: keep-alive

[miketaylr]

Any progress on this @karlcow?

[karlcow]

Let me check again:

The interesting part for me was that it failed with "The connection was reset" and no return to anywhere.
This will have to be solved in my PR.

So we do not have a handler for 413 in Flask but that would not matter anyway, because there is an issue before hand. Our super testing method. 🐒

@uploads.route('/', methods=['POST'])
def upload():
    '''Endpoint to upload an image.

    If the image asset passes validation, it's saved as:
        UPLOADS_DEFAULT_DEST + /year/month/random-uuid.ext

    Returns a JSON string that contains the filename and url.
    '''
    print "we are in uploads.route"
    if 'image' in request.files and request.files['image'].filename:
        print "first"
        imagedata = request.files['image']
        print "second"
    elif 'screenshot' in request.form:
        imagedata = request.form['screenshot']
    else:
        # We don't know what you're trying to do, but it ain't gonna work.
        print "abort 501"
        abort(501)

    try:
        print "we are in the try"
        upload = Upload(imagedata)
        print "after upload Class"
        upload.save()
        print "after upload save"
        data = {
            'filename': upload.get_filename(),
            'url': upload.get_url()
        }
        return (json.dumps(data), 201, {'content-type': JSON_MIME})
    except (TypeError, IOError):
        abort(415)
    except RequestEntityTooLarge:
        print "do we reach 413?"
        abort(413)

Let's run with a small image.

127.0.0.1 - - [02/Mar/2016 11:55:21] "GET /img/loader.gif HTTP/1.1" 304 -
we are in uploads.route
first
second
we are in the try
after upload Class
after upload save
127.0.0.1 - - [02/Mar/2016 11:55:22] "POST /issues/new HTTP/1.1" 302 -
127.0.0.1 - - [02/Mar/2016 11:55:22] "GET /thanks/330 HTTP/1.1" 200 -

All good! 🐵

Now let's try with a big image. We get a

And the following log:

127.0.0.1 - - [02/Mar/2016 11:57:41] "GET / HTTP/1.1" 200 -
127.0.0.1 - - [02/Mar/2016 11:57:43] "GET /api/issues/labels?per_page=100 HTTP/1.1" 304 -
127.0.0.1 - - [02/Mar/2016 11:57:44] "GET /api/issues/category/new HTTP/1.1" 200 -
127.0.0.1 - - [02/Mar/2016 11:58:19] "POST /issues/new HTTP/1.1" 413 -

no print statements at all. 🙈

[karlcow]

http://flask.pocoo.org/snippets/47/

You might notice that if you start not accessing .form or .files on incoming POST requests, some browsers will honor this with a connection reset message. This can happen if you start rejecting uploads that are larger than a given size.

Some WSGI servers solve that problem for you, others do not. For instance the builtin Flask webserver is pretty dumb and will not attempt to fix this problem.

[karlcow]

Ah…

Thankfully it's easy to fix this in a WSGI middleware. But first we have to figure out why it's actually happening. The reason this happens is that the browser will have queued up quite some bytes sent and will not start reading from the socket unless it transmitted everything to the server. Because the server however is not accepting the data, it's not responding.

Logical. There is a piece of code to handle this, but it kind of creates a layer which is a bit weird. Maybe it's better to not address it? I wonder how Blink, Webkit and Edge browsers are handling this.

[miketaylr]

Hmm. It would be nice to prevent a CONNECTION RESET page -- that makes it feel like the browser or site is busted, IMO (rather than reflect that the user is trying something a little crazy).

What does the middleware look like?

[karlcow]

@miketaylr follow the link to the snippets the code is there.

[miketaylr]

Ah, sorry, I only read the quoted text, didn't realize that was a link. 🙈

[miketaylr]

Karl told me to close this.

[miketaylr]

This was fixed by #1049.