Attachment 369226 Details for Bug 197644 – Patch

[patch] Patch

bug-197644-20190506213756.patch (text/plain), 6.81 KB, created by Wenson Hsieh on 2019-05-06 21:37:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-05-06 21:37:57 PDT

Size: 6.81 KB

patch

obsolete

>Subversion Revision: 244993
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 83a6dcd24ee914e8d80c618ad1b806b79bf165cb..3c905503916858ccef3ccb133fd4a3f26a07236e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-05-06  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [macOS] Avoid crashing the UI process when writing empty data to the pasteboard
>+        https://bugs.webkit.org/show_bug.cgi?id=197644
>+        <rdar://problem/50526364>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: WebKit.WKWebProcessPlugInDoNotCrashWhenCopyingEmptyClientData
>+
>+        * WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
>+        (WebKit::WebPlatformStrategies::setBufferForType):
>+
>+        Make this function robust by not attempting to create a shared memory buffer in the case where the given data
>+        buffer is empty.
>+
> 2019-05-06  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Occasional crashes in layout tests when firing the shrink-to-fit-content timer
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
>index 34666244693c966cf915717f209dc919ee4ac4c0..066816753925611013418596d5641cf53154eb46 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
>@@ -202,7 +202,7 @@ long WebPlatformStrategies::setTypes(const Vector<String>& pasteboardTypes, cons
> long WebPlatformStrategies::setBufferForType(SharedBuffer* buffer, const String& pasteboardType, const String& pasteboardName)
> {
>     SharedMemory::Handle handle;
>-    if (buffer) {
>+    if (buffer && buffer->size()) {
>         RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(buffer->size());
>         // FIXME: Null check prevents crashing, but it is not great that we will have empty pasteboard content for this type,
>         // because we've already set the types.
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 480e7f4a892dfb7a6729b2d9b714bbc5cbcdbc90..e527de9298cc15d3d250021afaa6436ab167cf9e 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-05-06  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [macOS] Avoid crashing the UI process when writing empty data to the pasteboard
>+        https://bugs.webkit.org/show_bug.cgi?id=197644
>+        <rdar://problem/50526364>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a new API test to exercise a possible scenario where we may crash while writing data to the pasteboard.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm:
>+        * TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm:
>+        (-[BundleEditingDelegatePlugIn webProcessPlugIn:didCreateBrowserContextController:]):
>+        (-[BundleEditingDelegatePlugIn _webProcessPlugInBrowserContextController:pasteboardDataForRange:]):
>+
> 2019-05-04  Per Arne Vollan  <pvollan@apple.com>
> 
>         -[WKWebsiteDataStore removeDataOfTypes:forDataRecords:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm
>index eb982bc92a88826148eb08cc656ed516ee5613a4..c5fca3e7fe991251c49e9e67256b8c52d6af841d 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegate.mm
>@@ -115,4 +115,21 @@ TEST(WebKit, WKWebProcessPlugInEditingDelegate)
>     TestWebKitAPI::Util::run(&doneEvaluatingJavaScript);
> }
> 
>-#endif
>+TEST(WebKit, WKWebProcessPlugInDoNotCrashWhenCopyingEmptyClientData)
>+{
>+    auto configuration = retainPtr([WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"BundleEditingDelegatePlugIn"]);
>+    [[configuration processPool] _setObject:@YES forBundleParameter:@"EditingDelegateShouldWriteEmptyData"];
>+
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    [webView loadHTMLString:@"<body style='-webkit-user-modify: read-write-plaintext-only'>Just something to copy <script> var textNode = document.body.firstChild; document.getSelection().setBaseAndExtent(textNode, 5, textNode, 14) </script>" baseURL:nil];
>+    [webView _test_waitForDidFinishNavigation];
>+
>+    auto object = adoptNS([[BundleEditingDelegateRemoteObject alloc] init]);
>+    _WKRemoteObjectInterface *interface = [_WKRemoteObjectInterface remoteObjectInterfaceWithProtocol:@protocol(BundleEditingDelegateProtocol)];
>+    [[webView _remoteObjectRegistry] registerExportedObject:object.get() interface:interface];
>+
>+    [webView performSelector:@selector(copy:) withObject:nil];
>+    TestWebKitAPI::Util::run(&didWriteToPasteboard);
>+}
>+
>+#endif // PLATFORM(MAC)
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm
>index 9231484a7adb795c452e03dc630520e368c8b051..8f169972bfc1c168ae1166dbc44629075d3038bd 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/BundleEditingDelegatePlugIn.mm
>@@ -45,6 +45,7 @@ @implementation BundleEditingDelegatePlugIn {
>     RetainPtr<id <BundleEditingDelegateProtocol>> _remoteObject;
>     BOOL _editingDelegateShouldInsertText;
>     BOOL _shouldOverridePerformTwoStepDrop;
>+    BOOL _shouldWriteEmptyData;
> }
> 
> - (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController didCreateBrowserContextController:(WKWebProcessPlugInBrowserContextController *)browserContextController
>@@ -60,6 +61,7 @@ - (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController didCre
>     } else
>         _editingDelegateShouldInsertText = YES;
> 
>+    _shouldWriteEmptyData = [[plugInController.parameters valueForKey:@"EditingDelegateShouldWriteEmptyData"] boolValue];
>     _shouldOverridePerformTwoStepDrop = [[plugInController.parameters valueForKey:@"BundleOverridePerformTwoStepDrop"] boolValue];
> 
>     _WKRemoteObjectInterface *interface = [_WKRemoteObjectInterface remoteObjectInterfaceWithProtocol:@protocol(BundleEditingDelegateProtocol)];
>@@ -83,7 +85,7 @@ - (void)_webProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserCont
> 
> - (NSDictionary<NSString *, NSData *> *)_webProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserContextController *)controller pasteboardDataForRange:(WKWebProcessPlugInRangeHandle *)range
> {
>-    return @{ @"org.webkit.data" : [NSData dataWithBytesNoCopy:(void*)"hello" length:5 freeWhenDone:NO] };
>+    return @{ @"org.webkit.data" : _shouldWriteEmptyData ? NSData.data : [NSData dataWithBytesNoCopy:(void*)"hello" length:5 freeWhenDone:NO] };
> }
> 
> - (void)_webProcessPlugInBrowserContextControllerDidWriteToPasteboard:(WKWebProcessPlugInBrowserContextController *)controller

Actions: View | Formatted Diff | Diff

Attachments on bug 197644: 369226 | 369247