Deleting a character converted from pinyin after an image causes a Safari crash
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Dec 2016 04:04:08 +0000 (04:04 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Dec 2016 04:04:08 +0000 (04:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165839
Source/WebKit2:

Reviewed by Darin Adler.

The crash was caused by the payload of the IPC not being decoded correctly when the encoded attributed string
contains a NSTextAttachment but send<> would still gladly send it to the UIProcess.

Fixed it by omitting the image as done in r176412 since encoding NSFileWrapper, etc... would require
quite a bit of work, and IME doesn't really need to see the image in its attributed string.

* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::attributedSubstringForCharacterRangeAsync): Fixed the bug.

Tools:

<rdar://problem/27951933>

Reviewed by Wenson Hsieh.

Add a WebKit API test to call attributedSubstringForProposedRange on a WKWebView
while the proposed range contains an image. This should not cause a WebProcess to crash
or send an invalid message to the UIProcess.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit2/chinese-character-with-image.html: Added.
* TestWebKitAPI/Tests/WebKit2/mac/AttributedSubstringForProposedRangeWithImage.mm: Added.
(TestWebKitAPI::didFinishLoadForFrame):
(TestWebKitAPI::processDidCrash):
(TestWebKitAPI::invalidMessageFunction):
(TestWebKitAPI::WebKit2.AttributedSubstringForProposedRangeWithImage):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@209957 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit2/chinese-character-with-image.html [new file with mode: 0644]
Tools/TestWebKitAPI/Tests/WebKit2/mac/AttributedSubstringForProposedRangeWithImage.mm [new file with mode: 0644]

index 8b7d2d8..d661700 100644 (file)
@@ -1,3 +1,19 @@
+2016-12-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Deleting a character converted from pinyin after an image causes a Safari crash
+        https://bugs.webkit.org/show_bug.cgi?id=165839
+
+        Reviewed by Darin Adler.
+
+        The crash was caused by the payload of the IPC not being decoded correctly when the encoded attributed string
+        contains a NSTextAttachment but send<> would still gladly send it to the UIProcess.
+
+        Fixed it by omitting the image as done in r176412 since encoding NSFileWrapper, etc... would require
+        quite a bit of work, and IME doesn't really need to see the image in its attributed string.
+
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::attributedSubstringForCharacterRangeAsync): Fixed the bug.
+
 2016-12-16  Andy Estes  <aestes@apple.com>
 
         Add a setting to suppress keyboard input during provisional navigation
index ce66506..54bd203 100644 (file)
@@ -362,7 +362,7 @@ void WebPage::attributedSubstringForCharacterRangeAsync(const EditingRange& edit
         return;
     }
 
-    result.string = editingAttributedStringFromRange(*range);
+    result.string = editingAttributedStringFromRange(*range, IncludeImagesInAttributedString::No);
     NSAttributedString* attributedString = result.string.get();
     
     // WebCore::editingAttributedStringFromRange() insists on inserting a trailing
index 90a34ae..84af577 100644 (file)
@@ -1,3 +1,23 @@
+2016-12-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Deleting a character converted from pinyin after an image causes a Safari crash
+        https://bugs.webkit.org/show_bug.cgi?id=165839
+        <rdar://problem/27951933>
+
+        Reviewed by Wenson Hsieh.
+
+        Add a WebKit API test to call attributedSubstringForProposedRange on a WKWebView
+        while the proposed range contains an image. This should not cause a WebProcess to crash
+        or send an invalid message to the UIProcess.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit2/chinese-character-with-image.html: Added.
+        * TestWebKitAPI/Tests/WebKit2/mac/AttributedSubstringForProposedRangeWithImage.mm: Added.
+        (TestWebKitAPI::didFinishLoadForFrame):
+        (TestWebKitAPI::processDidCrash):
+        (TestWebKitAPI::invalidMessageFunction):
+        (TestWebKitAPI::WebKit2.AttributedSubstringForProposedRangeWithImage):
+
 2016-12-16  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Visual viewports: carets and selection UI are incorrectly positioned when editing fixed elements
index 016aa69..246dd2b 100644 (file)
                9B26FCCA159D16DE00CC3765 /* HTMLFormCollectionNamedItem.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B26FCB4159D15E700CC3765 /* HTMLFormCollectionNamedItem.html */; };
                9B270FEE1DDC2C0B002D53F3 /* closed-shadow-tree-test.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B270FED1DDC25FD002D53F3 /* closed-shadow-tree-test.html */; };
                9B4F8FA7159D52DD002D9F94 /* HTMLCollectionNamedItem.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9B4F8FA6159D52CA002D9F94 /* HTMLCollectionNamedItem.html */; };
+               9BD4239A1E04BD9800200395 /* AttributedSubstringForProposedRangeWithImage.mm in Sources */ = {isa = PBXBuildFile; fileRef = 9BD423991E04BD9800200395 /* AttributedSubstringForProposedRangeWithImage.mm */; };
+               9BD4239C1E04C01C00200395 /* chinese-character-with-image.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9BD4239B1E04BFD000200395 /* chinese-character-with-image.html */; };
                9C64DC321D76198A004B598E /* YouTubePluginReplacement.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9C64DC311D76198A004B598E /* YouTubePluginReplacement.cpp */; };
                A1146A8D1D2D7115000FE710 /* ContentFiltering.mm in Sources */ = {isa = PBXBuildFile; fileRef = A1146A8A1D2D704F000FE710 /* ContentFiltering.mm */; };
                A125478F1DB18B9400358564 /* LoadDataWithNilMIMEType.mm in Sources */ = {isa = PBXBuildFile; fileRef = A125478D1DB18B9400358564 /* LoadDataWithNilMIMEType.mm */; };
                        dstPath = TestWebKitAPI.resources;
                        dstSubfolderSpec = 7;
                        files = (
+                               9BD4239C1E04C01C00200395 /* chinese-character-with-image.html in Copy Resources */,
                                5110FCF91E01CD8A006F8D0B /* IndexUpgrade.blob in Copy Resources */,
                                5110FCF61E01CD83006F8D0B /* IndexUpgrade.sqlite3 in Copy Resources */,
                                5110FCF11E01CD64006F8D0B /* IDBIndexUpgradeToV2.html in Copy Resources */,
                9B4F8FA3159D52B1002D9F94 /* HTMLCollectionNamedItem.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = HTMLCollectionNamedItem.mm; sourceTree = "<group>"; };
                9B4F8FA6159D52CA002D9F94 /* HTMLCollectionNamedItem.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = HTMLCollectionNamedItem.html; sourceTree = "<group>"; };
                9B79164F1BD89D0D00D50B8F /* FirstResponderScrollingPosition.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FirstResponderScrollingPosition.mm; sourceTree = "<group>"; };
+               9BD423991E04BD9800200395 /* AttributedSubstringForProposedRangeWithImage.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = AttributedSubstringForProposedRangeWithImage.mm; sourceTree = "<group>"; };
+               9BD4239B1E04BFD000200395 /* chinese-character-with-image.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "chinese-character-with-image.html"; sourceTree = "<group>"; };
                9C64DC311D76198A004B598E /* YouTubePluginReplacement.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = YouTubePluginReplacement.cpp; sourceTree = "<group>"; };
                A1146A8A1D2D704F000FE710 /* ContentFiltering.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ContentFiltering.mm; sourceTree = "<group>"; };
                A125478D1DB18B9400358564 /* LoadDataWithNilMIMEType.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = LoadDataWithNilMIMEType.mm; sourceTree = "<group>"; };
                BC90977B125571AE00083756 /* Resources */ = {
                        isa = PBXGroup;
                        children = (
+                               9BD4239B1E04BFD000200395 /* chinese-character-with-image.html */,
                                07492B391DF8ADA400633DE1 /* enumerateMediaDevices.html */,
                                C045F9461385C2F800C0F3CD /* 18-characters.html */,
                                1C2B81851C89252300A5529F /* Ahem.ttf */,
                C0C5D3BB14598B6F00A802A6 /* mac */ = {
                        isa = PBXGroup;
                        children = (
+                               9BD423991E04BD9800200395 /* AttributedSubstringForProposedRangeWithImage.mm */,
                                8349D3C11DB96DDA004A9F65 /* ContextMenuDownload.mm */,
                                BCAA485714A044D40088FAC4 /* EditorCommands.mm */,
                                C0C5D3BC14598B6F00A802A6 /* GetBackingScaleFactor.mm */,
                        buildActionMask = 2147483647;
                        files = (
                                2E7765CD16C4D80A00BA2BB1 /* mainIOS.mm in Sources */,
+                               9BD4239A1E04BD9800200395 /* AttributedSubstringForProposedRangeWithImage.mm in Sources */,
                                7AD3FE8E1D76131200B169A4 /* TransformationMatrix.cpp in Sources */,
                                2E7765CF16C4D81100BA2BB1 /* mainMac.mm in Sources */,
                        );
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2/chinese-character-with-image.html b/Tools/TestWebKitAPI/Tests/WebKit2/chinese-character-with-image.html
new file mode 100644 (file)
index 0000000..1790857
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<meta http-equiv='content-type' content='text/html; charset=utf-8'>
+<body>
+<div contenteditable>
+<p><br></p>
+<p style="text-align: center; font-size: 15px;"><img src="icon.png">你</p>
+<p>hello, world</p>
+</div>
+<script>
+document.querySelector('div').focus();
+getSelection().setPosition(document.querySelector('img').parentNode, 1);
+</script>
+</body>
+</html>
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2/mac/AttributedSubstringForProposedRangeWithImage.mm b/Tools/TestWebKitAPI/Tests/WebKit2/mac/AttributedSubstringForProposedRangeWithImage.mm
new file mode 100644 (file)
index 0000000..05644cb
--- /dev/null
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "PlatformUtilities.h"
+#include "PlatformWebView.h"
+#include <WebKit/WKContextPrivate.h>
+#include <WebKit/WKRetainPtr.h>
+
+@interface WKView ()
+- (void)attributedSubstringForProposedRange:(NSRange)nsRange completionHandler:(void(^)(NSAttributedString *attrString, NSRange actualRange))completionHandlerPtr;
+@end
+
+namespace TestWebKitAPI {
+
+static bool didFinishLoad = false;
+static bool didFinishTest = false;
+static bool didObserveWebProcessToCrash = false;
+static bool didReceiveInvalidMessage = false;
+
+static void didFinishLoadForFrame(WKPageRef, WKFrameRef, WKTypeRef, const void*)
+{
+    didFinishLoad = true;
+}
+
+static void processDidCrash(WKPageRef, const void*)
+{
+    didObserveWebProcessToCrash = true;
+}
+
+static void invalidMessageFunction(WKStringRef messageName)
+{
+    didReceiveInvalidMessage = true;
+}
+
+TEST(WebKit2, AttributedSubstringForProposedRangeWithImage)
+{
+    WKRetainPtr<WKContextRef> context(AdoptWK, Util::createContextWithInjectedBundle());
+    WKRetainPtr<WKPageGroupRef> pageGroup(AdoptWK, WKPageGroupCreateWithIdentifier(Util::toWK("AttributedSubstringForProposedRangeWithImagePageGroup").get()));
+    PlatformWebView webView(context.get(), pageGroup.get());
+
+    WKPageLoaderClientV0 loaderClient;
+    memset(&loaderClient, 0, sizeof(loaderClient));
+    loaderClient.base.version = 0;
+    loaderClient.didFinishLoadForFrame = didFinishLoadForFrame;
+    loaderClient.processDidCrash = processDidCrash;
+    WKPageSetPageLoaderClient(webView.page(), &loaderClient.base);
+
+    WKContextSetInvalidMessageFunction(invalidMessageFunction);
+
+    WKRetainPtr<WKURLRef> url(AdoptWK, Util::createURLForResource("chinese-character-with-image", "html"));
+    WKPageLoadURL(webView.page(), url.get());
+    Util::run(&didFinishLoad);
+
+    [webView.platformView() attributedSubstringForProposedRange:NSMakeRange(0, 3) completionHandler: ^(NSAttributedString *attrString, NSRange actualRange) {
+        didFinishTest = true;
+    }];
+
+    Util::run(&didFinishTest);
+    EXPECT_EQ(didObserveWebProcessToCrash, false);
+    EXPECT_EQ(didReceiveInvalidMessage, false);
+}
+
+} // namespace TestWebKitAPI