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

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.