Bug 37882

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:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.6

Attachments:

Description	Flags
Script Including HTML5 DnD Implementation	none
When I drop this file onto the drop zone, webkit crashes	none
Proposed Patch	adele: review-, jianli: commit-queue-
Proposed Patch	darin: review+, jianli: commit-queue-

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-	Details Formatted Diff Diff
Proposed Patch (4.11 KB, patch) 2010-04-27 14:03 PDT, Jian Li	darin: review+ jianli: commit-queue-	Details Formatted Diff Diff
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.