Trac file uploads/attachments failing/corrupted

[warner]

Since the migration (and switching from Apache/mod_python to nginx/uwsgi), uploads of moderate-sized files are failing in weird ways. Small patches sometimes work, but larger patches fail in weird ways:

• sirvaliance tried to upload a 600kB .png to #1185 and observed reported success, but the image was not present on the subsequent attachment list
• warner tried to upload 900kB-1.1MB .jpg files to wiki:Summit2Day1 . Sometimes it failed with a UnicodeDecodeError, sometimes with an nginx 413 Request Entity Too Large.

Somebody needs to study the way that nginx invokes uwsgi with large request bodies. If it writes them to a file before handing them to uwsgi (seems unlikely), then it's conceivable that a file-permission/ownership problem is involved.

I'm going to assign this one to Zooko, since he configured nginx/uwsgi on this box, and thus far is the only one who even begins to understand them.

For now, I'm manually scp'ing files to tahoe-lafs.org and using 'trac-admin' to attach them to tickets.

[warner]

The UnicodeDecodeError looks like it's coming from a part of Trac that handles form POST arguments,

File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.2-py2.6.egg/trac/web/api.py",
  line 572, in _parse_arg_list:

        args = []
        for value in fs.list or ():
            name = value.name
            if not value.filename:
                try:
                    value = unicode(value.value, 'utf-8')
                except UnicodeDecodeError, e:
                    e.args = tuple(e.args + (name, repr(value.value),))
                    raise
            args.append((name, value))
        return args

What I think is happening here is that file uploads use MIME-Multipart, so each body section can have a couple of attributes in addition to the actual value. "name" is one such attribute, as is "filename". I'm guessing that the Trac code assumes that any attachment that doesn't have a filename must be a UTF8-encoded unicode string. If that's the case, then it implies that Trac is not seeing a filename= attribute on the main file's MIME-Multipart body. This could be a result of nginx/uwsgi removing it, or something odd happening in my browser such that it's not sending it.

[warner]

Also, .jpg attachments (manually added with trac-admin) larger than 1MB don't render as images when you look at them: the message says HTML preview not available, since the file size exceeds 1000000 bytes. Try downloading the file instead.. I increased the preview limit to 1.2MB to accomodate this, which removes the error, but now it shows me the first inch or so of the image and then nothing else: it looks like the server stops serving image data after some limited-size buffer. This feels like another nginx/uwsgi issue.

[zooko]

I was able to attach and download a file of size 1 MB just now. Now trying 10 MB...

[zooko]

Okay, I attempted to upload a 10 MB file and got an error from nginx. Then I changed nginx's config to allow client request bodies up to 2 GB and tried again. Now I got the error that Brian said Kevan had -- the upload appeared to succeed but then the file wasn't visible in the attachments list. Will investigate more later.

[zooko]

#1577 was a duplicate of this.

[davidsarah]

I still see this occasionally, and not always with large files.

[warner]

still seeing this, on a 33kB patch upload

[warner]

it's worse, I just uploaded a 150kB image to #1265 and it managed to corrupt the file, by inserting a single newline in front of the whole file (not a text-mode-FTP -style problem, just a single-byte 0x0a prefix).

[warner]

I don't know if it's related, but I just tried to attach a 43kB patch to #166 and got an "internal trac error" with the following exception in the logs:

2012-05-31 21:33:47,862 Trac[main] ERROR: Internal Server Error: 
Traceback (most recent call last):
  File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.3-py2.6.egg/trac/web/main.py", line 522, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.3-py2.6.egg/trac/web/main.py", line 243, in dispatch
    resp = chosen_handler.process_request(req)
  File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.3-py2.6.egg/trac/attachment.py", line 466, in process_request
    self._do_save(req, attachment)
  File "/usr/local/lib/python2.6/dist-packages/Trac-0.12.3-py2.6.egg/trac/attachment.py", line 638, in _do_save
    upload = req.args['attachment']
KeyError: 'attachment'

My vague hunch is that the accept-an-attachment handler got called but somehow the attachment itself got lost on the way in.

[warner]

One other data point, I re-tried attaching that same 43kB patch to #166, and got the previously-buggy "lalala I can't hear you" behavior, where I land on the expected "list of attachments" page but the file I just tried to upload wasn't listed. I repeated that several times with the same results.

Then, about 10 minutes later, I tried again, and it worked. Two things had happened in the meantime: Zooko restarted nginx "after changing its ssl cache size from 1m to 100m entries", and I changed the attachment comments (removed a few words).

[davidsarah]

At least one problem (maybe the only remaining one) does seem to be related to the description length. Attaching ​https://tahoe-lafs.org/trac/tahoe-lafs/attachment/ticket/1636/fix-1636-for-1.9.2.darcs.patch to #1636 with the following description failed:

[rebased for 1.9.2] After a server disconnects, make the IServer retain the dead RemoteReference, and continue to return it to anyone who calls get_rref(). This removes the need for callers to guard against receiving a None (as long as the server was connected at least once, which is always the case for servers returned by get_servers_for_psi(), which is how all upload/download code gets servers). Includes test, which is now the same as on trunk. fixes #1636 for 1.9.2.

as did the same description without the last sentence. The next attempt with the same file and this description worked:

[rebased for 1.9.2] After a server disconnects, make the IServer retain the dead RemoteReference, and continue to return it to anyone who calls get_rref(). This removes the need for callers to guard against receiving a None (as long as the server was connected at least once, which is always the case for servers returned by get_servers_for_psi(), which is how all upload/download code gets servers). Includes test

[PRabahy]

Looks like this issue still exists. I tried to upload a .diff (571 bytes) to #1842, it looked like it succeeded, but when I refreshed the ticket there was no attachment. I waited about an hour and tried again with the same result.

[zooko]

Thanks for the update, PRabahy. I'll try to reproduce it using a .diff of 571 bytes length.

[zooko]

Possibly related: #2241

[daira]

Possibly a duplicate: #2270

[meejah]

Ticket retargeted after milestone closed