Bug 159288 - Prevent crash when attempting to copy an image
Summary: Prevent crash when attempting to copy an image
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: iPhone / iPad All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-29 18:35 PDT by Brent Fulgham
Modified: 2016-07-01 13:53 PDT (History)
6 users (show)

See Also:

Attachments
Patch (2.17 KB, patch)
2016-06-29 18:41 PDT, Brent Fulgham 		no flags Details | Formatted Diff | Diff
Manual test case (6.14 KB, application/zip)
2016-07-01 09:37 PDT, Brent Fulgham 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 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.
Comment 1 Brent Fulgham 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!).
Comment 2 Brent Fulgham 2016-06-29 18:37:57 PDT 
<rdar://problem/23507828>
Comment 3 Brent Fulgham 2016-06-29 18:41:04 PDT 
Created attachment 282402 [details]
Patch
Comment 4 Brady Eidson 2016-06-29 21:38:04 PDT 
Comment on attachment 282402 [details]
Patch

Test?
Comment 5 Brent Fulgham 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.
Comment 6 Brent Fulgham 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.
Comment 7 Brent Fulgham 2016-07-01 13:27:34 PDT 
Neither WebKitTestRunner nor TestWebKitAPI support driving the UI interaction needed to trigger this bug.
Comment 8 Brent Fulgham 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.
Comment 9 Brent Fulgham 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.
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2016-07-01 13:53:01 PDT 
All reviewed patches have been landed.  Closing bug.