There are several ways that the upload can hang in Safari, all of them related to an AHAH call that either never returns, or returns nothing.

When validation fails on file size, the module returns full drupal_not_found(), which causes the browser problems because it's expecting JSON.

When validation fails on the extension, the module goes through the logic of uploading it anyways, even though the file hasn't been moved, which sometimes causes the browser to hang when it returns nothing at all to the browser.

This patch addresses these issues. I've uploaded 200 files through Safari, and got three infinite loads. All three corresponded to a "broken pipe" message in my Apache log. I didn't get any from Firefox or Camino. When this happens, the form action gets assigned to /comment_upload/js in the AHAH process and doesn't get reassigned properly because the AHAH process never ends.

CommentFileSizeAuthor
comment_upload_validation.patch2.86 KBnetaustin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

netaustin’s picture

Status: Active » Needs review
dww’s picture

Status: Needs review » Needs work

Hate to do this, but I tried testing the patch and I'm getting the same behavior I reported originally. :( The description makes sense. The patch looks reasonable, but I still get the same hanging uploads (and occasional redirects to comment-upload/js) in Safari. I even restarted Safari on a whim (it had been open for quite a while) -- same badness. I double checked that I've correctly patched the right version of comment_upload, etc, etc. I've definitely got the latest from DRUPAL-6--1 (// $Id: comment_upload.module,v 1.7.2.23 2009/04/21 16:41:04 netaustin Exp $) plus this patch, and it's still broken. :( Ugh...

netaustin’s picture

No worries, if it needs work, it needs work. What I'm most concerned about at this point is just getting to replicate it. Can you do these things?

1. Test it on my public server: http://drupalorg.netaustin.net. I'll send you admin and auth user details off-issue. (If any spectators want to test, just register; I'm not going to pass around user 1 details).
2. What's the story in Firefox on your local machine?
3. Can you describe how to replicate it reliably on your machine?
4. Would you check the list of my installed modules against yours and report differences?
5. When it happens locally, what shows up in the apache error log?

Thanks again for helping me debug this. I'll get bluebeach installed for total parity as soon as I fix my svn.d.o access issue (waiting for rest from infra.d.o).

greggles’s picture

I tried a couple of uploads on drupalorg.netaustin and was unable to reproduce this. My Safari settings are pretty basic, afaik.

Scenarios:
1. Choose the file, click attach, click save.
2. Choose the file, click save.
3. Choose the file, click preview, click save.
4. Choose the file, clicke attach, choose another file, click save.

All of those worked fine. So...I don't see a clear cause of this. FWIW, we will probably be upgrading our project installation in the next month (we use comment_upload in it as well) so that should provide some additional testing ground for this. If you'd like, we could probably prioritize that a bit more.

dww’s picture

@netaustin -- thanks, that's all good stuff. Here are my results:

1) Safari seems to work just fine on your test site. I can't reproduce any problems there. Hrmph.

2) Firefox seems to work perfectly on my local machine, too...

3) To "reliably" reproduce on my local site, all I have to do is open an issue node, and try attaching files with Safari. Every time I attach a non-allowed extension (e.g. "foo.patch") as a non-admin, the attach hangs, and I never see the validation error message. Sometimes even when attaching an allowed extension, it also hangs. In both cases, it hangs indefinitely. If I click Safari's "X" button to stop the page loading, then try to preview or save the comment, I'm sent to [sites]/comment-upload/js with the JSON output, similar to what I pastebin'ed for you earlier... As uid 1, I get the hangs intermittently when attaching any files at all. As a non-admin, I always get them on non-allowed extensions, and semi-regularly (but not all the time) on allowed extensions.

4) The most obvious difference is your site seems to be missing views. ;) Not sure how you managed that, since it's supposed to be a required dependency. Here's a list of the modules only installed on each site:

netaustin:
- php filter

dww:
- devel (I really thought this might have been the problem -- since it routinely can break AJAX callbacks if they don't set the proper http headers, but I just disabled it, tried again, and still got the hanging uploads)
- drupalorg
- install profile API
- comment_alter_taxonomy
- views
- views_ui

I just tried disabling drupalorg, install profile API, and comment_alter_taxonomy (and devel), so that the only difference was views, and still seeing the problems. :(

5) When it fails locally, I see nothing in the error_log, and access_log just has lines like this:

fe80::223:12ff:fe52:ad71 - - [23/Apr/2009:13:49:51 -0700] "POST /drupal-6-project/comment-upload/js HTTP/1.1" 200 639
fe80::223:12ff:fe52:ad71 - - [23/Apr/2009:13:52:31 -0700] "POST /drupal-6-project/comment-upload/js HTTP/1.1" 200 2400

...

No idea if that helps. Probably not. :( I should try installing the latest code on d6.drupal.org and seeing if I (or anyone else) can break it there. If not, I'm willing to chalk this up to something really stupid with my local install and forget about it. So far, that seems like the most likely, if no one else can reproduce this on any other site... I'm just terrified of deploying this on d.o without a better understanding of what's going on, since I'm *sure* people there will find ways to break it if it's not rock solid. :(

netaustin’s picture

OK, thanks. I've installed views as well as the drupalorg project customization module. Still can't replicate. I would appreciate an install on d6.d.o and some testing. There's just one issue remaining to boost stability on d.o, but it's relatively minor, and going to be a PI patch. #445484: With comment_upload HEAD, changing the project strips the uploaded files

Anonymous’s picture

Just on a whim, what happens if you use a slightly older version of the enabled modules? If you can find a version that does work, even though we can't reproduce it, we could make a diff would at least give us an idea of where to look.

dww’s picture

Status: Needs work » Needs review

I put this on http://d6.drupal.org (DRUPAL-6--1-0-ALPHA4 and this patch) and have NOT been able to replicate my problems at all. Other folks are encouraged to beat on d6.d.o and see if they find any problems, but I think this is probably RTBC...

Sorry for the delay!
-Derek

netaustin’s picture

Status: Needs review » Fixed

Fixed in 221560.

dww’s picture

Issue tags: +needs drupal.org deployment

We still need to push this to the live site... I'm on it.

dww’s picture

Issue tags: -needs drupal.org deployment

6.x-1.0-alpha5 is now deployed on d.o.

Yay!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

dww’s picture

Title: hanging upload in safari » upload periodically hanging in Safari, Opera and perhaps others
Status: Closed (fixed) » Active

This seems to be back on d.o. Folks are reporting it in Opera, too. :(