tahoe-lafs/trac

Trac file uploads/attachments failing/corrupted #1581

New issue

Open

opened 2011-11-12 19:28:01 +00:00 by warner · 20 comments

warner commented

2011-11-12 19:28:01 +00:00

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.

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](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 added the

labels 2011-11-12 19:28:01 +00:00

warner added this to the soon (release n/a) milestone 2011-11-12 19:28:01 +00:00

zooko was assigned by warner

2011-11-12 19:28:01 +00:00

warner commented

2011-11-12 19:47:56 +00:00

Author

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.

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 commented

2011-11-12 19:57:34 +00:00

Author

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.

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 commented

2011-11-19 23:38:06 +00:00

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

zooko commented

2011-11-19 23:57:54 +00:00

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 commented

2012-01-02 09:31:22 +00:00

#1577 was a duplicate of this.

zooko commented

2012-01-02 09:41:18 +00:00

Attachment 10Mnull (10000000 bytes) added

**Attachment** 10Mnull (10000000 bytes) added

10Mnull

9.5 MiB

zooko commented

2012-01-02 09:46:50 +00:00

Attachment 10Mnull.2 (10000000 bytes) added

**Attachment** 10Mnull.2 (10000000 bytes) added

10Mnull.2

9.5 MiB

zooko commented

2012-01-03 06:31:36 +00:00

Attachment 10Mnull.3 (10000000 bytes) added

**Attachment** 10Mnull.3 (10000000 bytes) added

10Mnull.3

9.5 MiB

zooko commented

2012-01-03 12:58:41 +00:00

Attachment 10Mnull.4 (10000000 bytes) added

**Attachment** 10Mnull.4 (10000000 bytes) added

10Mnull.4

9.5 MiB

daira commented

2012-03-12 19:03:05 +00:00

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

warner commented

2012-04-23 06:15:12 +00:00

Author

still seeing this, on a 33kB patch upload

warner commented

2012-05-13 03:05:09 +00:00

Author

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 added

p/critical

and removed

p/major

labels 2012-05-13 03:05:09 +00:00

warner changed title from ~~Trac file uploads/attachments failing~~ to Trac file uploads/attachments failing/corrupted

2012-05-13 03:05:09 +00:00

warner commented

2012-05-31 21:40:47 +00:00

Author

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.

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 commented

2012-05-31 21:58:02 +00:00

Author

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).

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).

daira commented

2012-06-16 20:28:19 +00:00

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

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 commented

2012-11-06 16:09:08 +00:00

Owner

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.