| Summary: | Sending a package file using FormData crashes WebKit on Mac | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Richard York <richard> | ||||||||||
| Component: | XML | Assignee: | Jian Li <jianli> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | abarth, ap, dimich, eric, jchaffraix, jianli, mjs, webkit.review.bot | ||||||||||
| Priority: | P1 | Keywords: | InRadar | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Mac (Intel) | ||||||||||||
| OS: | OS X 10.6 | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 53866 [details]
When I drop this file onto the drop zone, webkit crashes
Confirmed with r57789. Crashing on a secondary thread in form serialization code (WebCore::advanceCurrentStream()). The test uses XMLHttpRequest to upload FormData. The crash is caused by sending a package file using FormData. Updated the title to reflect this. Created attachment 54180 [details]
Proposed Patch
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. 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. 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.
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?
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. Sorry for the delay. I am working on this one now. Created attachment 54450 [details]
Proposed Patch
Thanks for the suggestion. Please see the revised patch.
Committed as http://trac.webkit.org/changeset/58336. http://trac.webkit.org/changeset/58336 might have broken SnowLeopard Intel Release (Tests) |
Created attachment 53865 [details] Script Including HTML5 DnD Implementation When I drag and drop the attached .pages document onto the drop zone, webkit crashes.