RESOLVED FIXED Bug 37882
Sending a package file using FormData crashes WebKit on Mac 
https://bugs.webkit.org/show_bug.cgi?id=37882
Summary Sending a package file using FormData crashes WebKit on Mac
Richard York
Reported 2010-04-20 12:40:51 PDT
Created attachment 53865 [details] Script Including HTML5 DnD Implementation When I drag and drop the attached .pages document onto the drop zone, webkit crashes.
Attachments
Script Including HTML5 DnD Implementation (2.78 KB, text/html)
2010-04-20 12:40 PDT, Richard York 		no flags
Details
When I drop this file onto the drop zone, webkit crashes (106.30 KB, application/zip)
2010-04-20 12:42 PDT, Richard York 		no flags
Details
Proposed Patch (1.92 KB, patch)
2010-04-23 12:14 PDT, Jian Li 		adele: review-
jianli: commit-queue-
DetailsFormatted DiffDiff
Proposed Patch (4.11 KB, patch)
2010-04-27 14:03 PDT, Jian Li 		darin: review+
jianli: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Richard York
Comment 1 2010-04-20 12:42:25 PDT
Created attachment 53866 [details] When I drop this file onto the drop zone, webkit crashes
Alexey Proskuryakov
Comment 2 2010-04-21 00:58:23 PDT
Confirmed with r57789. Crashing on a secondary thread in form serialization code (WebCore::advanceCurrentStream()). The test uses XMLHttpRequest to upload FormData.
Jian Li
Comment 3 2010-04-23 12:06:45 PDT
The crash is caused by sending a package file using FormData. Updated the title to reflect this.
Jian Li
Comment 4 2010-04-23 12:14:27 PDT
Created attachment 54180 [details] Proposed Patch
Alexey Proskuryakov
Comment 5 2010-04-23 12:29:37 PDT
XMLHttpRequest doesn't talk to Page and Chrome, because it can be used from workers. Of course, this version of send() won't be invoked from a worker, since there is no DOMFormData, but it would still be better to not even include these headers in XMLHttpRequest.cpp. Of course, FormData already does this, and it's used by XMLHttpRequest in all code paths. That's freaky.
Alexey Proskuryakov
Comment 6 2010-04-23 12:35:30 PDT
It's even more freaky because FormData is used from secondary threads on the Mac, even when there are no workers. I can't really follow which methods can be called on which thread, and whether there are race conditions. It's fine to fix this particular crasher without a huge refactoring. But I'd prefer to not add direct Page/Chrome dependencies to XMLHttpRequest.cpp, if that's possible.
Darin Adler
Comment 7 2010-04-24 09:45:47 PDT
Comment on attachment 54180 [details] Proposed Patch This does seem like roughly what we want to do, but I'm concerned about the layering here. Adele Peterson did the original work; she might have some thoughts about the best way to do this. I'll think on it myself too.
Alexey Proskuryakov
Comment 8 2010-04-24 10:57:23 PDT
<rdar://problem/7903467>
Adele Peterson
Comment 9 2010-04-26 11:43:10 PDT
Comment on attachment 54180 [details] Proposed Patch Darin suggested we could change FormData::generateFiles to take a Document* instead of a ChromeClient*. That would make this much cleaner. Jian, can you give that a try and resubmit?
Maciej Stachowiak
Comment 10 2010-04-27 13:01:02 PDT
Jian, any update on this? It would be valuable to us to have a fix soon. Apple folks can take this one over if you don't have time to revise the patch.
Jian Li
Comment 11 2010-04-27 13:42:04 PDT
Sorry for the delay. I am working on this one now.
Jian Li
Comment 12 2010-04-27 14:03:46 PDT
Created attachment 54450 [details] Proposed Patch Thanks for the suggestion. Please see the revised patch.
Jian Li
Comment 13 2010-04-27 14:33:31 PDT
Committed as http://trac.webkit.org/changeset/58336.
WebKit Review Bot
Comment 14 2010-04-27 14:57:15 PDT
http://trac.webkit.org/changeset/58336 might have broken SnowLeopard Intel Release (Tests)
Note You need to log in before you can comment on or make changes to this bug.