Bug 159288

Summary: Prevent crash when attempting to copy an image
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit2Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, bfulgham, commit-queue, enrica, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: iPhone / iPad   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=159366
https://bugs.webkit.org/show_bug.cgi?id=159367
Attachments:
Description Flags
Patch
none
Manual test case none

Brent Fulgham
Reported 2016-06-29 18:35:31 PDT
The WebProcess crashes under certain circumstances when you attempt to copy and image in iOS. 1. Select an accessory like lightning cable. http://www.apple.com/shop/product/MD819/lightning-to-usb-cable-2-m?fnode=37 2. Tap and hold the image on the right > Copy 3. WebKit crashes. Image is not copied.
Attachments
Patch (2.17 KB, patch)
2016-06-29 18:41 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Manual test case (6.14 KB, application/zip)
2016-07-01 09:37 PDT, Brent Fulgham 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2016-06-29 18:37:42 PDT
This is due to a null-pointer dereference in WebPage::performActionOnElement. In this test case, the HTMLImageElement's renderer does not have a valid cached image. We ask the nullptr for its URL, and the process crashes. This could be avoided by a nullptr check, but the method we pass the URL to doesn't even use it! So we can avoid the whole issue by simply passing a default URL that can be just as easily ignored as a fully-formed URL (at less cost!).
Brent Fulgham
Comment 2 2016-06-29 18:37:57 PDT
<rdar://problem/23507828>
Brent Fulgham
Comment 3 2016-06-29 18:41:04 PDT
Created attachment 282402 [details] Patch
Brady Eidson
Comment 4 2016-06-29 21:38:04 PDT
Comment on attachment 282402 [details] Patch Test?
Brent Fulgham
Comment 5 2016-06-30 14:48:11 PDT
Note: This crash only reproduces if you LONG PRESS to display the Action Sheet, and then use the "Copy" action sheet option.
Brent Fulgham
Comment 6 2016-07-01 09:37:48 PDT
Created attachment 282556 [details] Manual test case I cannot create an API test or LayoutTest for this, because our iOS testing infrastructure does not support controlling action sheets or simulating touch events. Test: 1. Unzip the attached "Image_Copy_Crash.zip" someplace you can server from a web server. 2. Navigate to the "image_copy_crash.html" file in iOS. 3. Long-press on the colored region of the screen. 4. Select 'Copy'. If you do not crash, the test passes.
Brent Fulgham
Comment 7 2016-07-01 13:27:34 PDT
Neither WebKitTestRunner nor TestWebKitAPI support driving the UI interaction needed to trigger this bug.
Brent Fulgham
Comment 8 2016-07-01 13:33:02 PDT
I have filed Bug 159366 to expand our testing infrastructure to allow automated tests that exercise the UI interactions required to reproduce this crash.
Brent Fulgham
Comment 9 2016-07-01 13:47:57 PDT
Note that this crash revealed that specific markup can cause the cachedImage value to be null. I filled Bug 159367 to investigate this underlying bug.
WebKit Commit Bot
Comment 10 2016-07-01 13:52:57 PDT
Comment on attachment 282402 [details] Patch Clearing flags on attachment: 282402 Committed r202754: <http://trac.webkit.org/changeset/202754>
WebKit Commit Bot
Comment 11 2016-07-01 13:53:01 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.