Trac file uploads/attachments failing/corrupted #1581

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

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.

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
dev-infrastructure
major
defect
n/a
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
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.
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.

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

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

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.

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.

#1577 was a duplicate of this.

#1577 was a duplicate of this.

Attachment 10Mnull (10000000 bytes) added

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

Attachment 10Mnull.2 (10000000 bytes) added

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

Attachment 10Mnull.3 (10000000 bytes) added

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

Attachment 10Mnull.4 (10000000 bytes) added

**Attachment** 10Mnull.4 (10000000 bytes) added
9.5 MiB
davidsarah commented 2012-03-12 19:03:05 +00:00
Owner

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

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

still seeing this, on a 33kB patch upload

still seeing this, on a 33kB patch upload
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).

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
critical
and removed
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
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.
Author

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

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 commented 2012-06-16 20:28:19 +00:00
Owner

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.

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.

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

Thanks for the update, PRabahy. I'll try to reproduce it using a .diff of 571 bytes length.
zooko added
normal
and removed
critical
labels 2012-11-13 23:28:27 +00:00

Possibly related: #2241

Possibly related: #2241
daira commented 2014-08-07 14:04:07 +00:00
Owner

Possibly a duplicate: #2270

Possibly a duplicate: #2270
Owner

Ticket retargeted after milestone closed

Ticket retargeted after milestone closed
meejah modified the milestone from soon (release n/a) to soon 2021-03-30 18:41:12 +00:00
Sign in to join this conversation.
No Milestone
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Reference: tahoe-lafs/trac-2024-07-25#1581
No description provided.