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.
Comment | File | Size | Author |
---|---|---|---|
comment_upload_validation.patch | 2.86 KB | netaustin |
Comments
Comment #1
netaustin CreditAttribution: netaustin commentedComment #2
dwwHate 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...Comment #3
netaustin CreditAttribution: netaustin commentedNo 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).
Comment #4
gregglesI 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.
Comment #5
dww@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:
...
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. :(
Comment #6
netaustin CreditAttribution: netaustin commentedOK, 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
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedJust 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.
Comment #8
dwwI 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
Comment #9
netaustin CreditAttribution: netaustin commentedFixed in 221560.
Comment #10
dwwWe still need to push this to the live site... I'm on it.
Comment #11
dww6.x-1.0-alpha5 is now deployed on d.o.
Yay!
Comment #13
dwwThis seems to be back on d.o. Folks are reporting it in Opera, too. :(