Make -[_WKAttachment setFileWrapper:contentType:completion:] robust when given a...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 03:59:21 +0000 (03:59 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 03:59:21 +0000 (03:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195725
<rdar://problem/48545062>

Reviewed by Tim Horton.

Source/WebKit:

Add a missing nil check before invoking the given completionHandler in the case where the attachment is invalid.
Tested by augmenting WKAttachmentTests.SetFileWrapperForPDFImageAttachment to exercise this scenario.

* UIProcess/API/APIAttachment.cpp:
(API::Attachment::invalidate):

Additionally make sure that an invalidated _WKAttachment is also considered to be disconnected.

* UIProcess/API/Cocoa/_WKAttachment.mm:
(-[_WKAttachment setFileWrapper:contentType:completion:]):

Tools:

Test that we don't crash when changing the file wrapper of an invalid attachment, if the given completion
handler is nil.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(TestWebKitAPI::TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APIAttachment.cpp
Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm

index af58072..b5114a8 100644 (file)
@@ -1,3 +1,22 @@
+2019-03-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Make -[_WKAttachment setFileWrapper:contentType:completion:] robust when given a nil completion handler
+        https://bugs.webkit.org/show_bug.cgi?id=195725
+        <rdar://problem/48545062>
+
+        Reviewed by Tim Horton.
+
+        Add a missing nil check before invoking the given completionHandler in the case where the attachment is invalid.
+        Tested by augmenting WKAttachmentTests.SetFileWrapperForPDFImageAttachment to exercise this scenario.
+
+        * UIProcess/API/APIAttachment.cpp:
+        (API::Attachment::invalidate):
+
+        Additionally make sure that an invalidated _WKAttachment is also considered to be disconnected.
+
+        * UIProcess/API/Cocoa/_WKAttachment.mm:
+        (-[_WKAttachment setFileWrapper:contentType:completion:]):
+
 2019-03-13  Timothy Hatcher  <timothy@apple.com>
 
         REGRESSION (r242908):  'NSInvalidArgumentException', reason: '+[PKPaymentMerchantSession count]: unrecognized selector sent to class 0x1c0fae060'
index a0b2ef7..f9a4cad 100644 (file)
@@ -72,6 +72,7 @@ void Attachment::invalidate()
 #if PLATFORM(COCOA)
     m_fileWrapper.clear();
 #endif
+    m_insertionState = InsertionState::NotInserted;
 }
 
 #if !PLATFORM(COCOA)
index 012d63a..4549605 100644 (file)
@@ -123,7 +123,8 @@ static const NSInteger InvalidAttachmentErrorCode = 2;
 - (void)setFileWrapper:(NSFileWrapper *)fileWrapper contentType:(NSString *)contentType completion:(void (^)(NSError *))completionHandler
 {
     if (!_attachment->isValid()) {
-        completionHandler([NSError errorWithDomain:WKErrorDomain code:InvalidAttachmentErrorCode userInfo:nil]);
+        if (completionHandler)
+            completionHandler([NSError errorWithDomain:WKErrorDomain code:InvalidAttachmentErrorCode userInfo:nil]);
         return;
     }
 
index ecf888a..ddeb5e9 100644 (file)
@@ -1,3 +1,17 @@
+2019-03-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Make -[_WKAttachment setFileWrapper:contentType:completion:] robust when given a nil completion handler
+        https://bugs.webkit.org/show_bug.cgi?id=195725
+        <rdar://problem/48545062>
+
+        Reviewed by Tim Horton.
+
+        Test that we don't crash when changing the file wrapper of an invalid attachment, if the given completion
+        handler is nil.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (TestWebKitAPI::TEST):
+
 2019-03-13  Sam Weinig  <sam@webkit.org>
 
         Add utility function to allow easy reverse range-based iteration of a container
index fe55a60..587c539 100644 (file)
@@ -1478,11 +1478,17 @@ TEST(WKAttachmentTests, SetFileWrapperForPDFImageAttachment)
     auto webView = webViewForTestingAttachments();
     [webView evaluateJavaScript:@"document.body.appendChild()" completionHandler:nil];
     NSString *identifier = [webView stringByEvaluatingJavaScript:@"const i = document.createElement('img'); document.body.appendChild(i); HTMLAttachmentElement.getAttachmentIdentifier(i)"];
-    _WKAttachment *attachment = [webView _attachmentForIdentifier:identifier];
+    auto attachment = retainPtr([webView _attachmentForIdentifier:identifier]);
 
     auto pdfFile = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testPDFData()]);
     [attachment setFileWrapper:pdfFile.get() contentType:(__bridge NSString *)kUTTypePDF completion:nil];
     [webView waitForImageElementSizeToBecome:CGSizeMake(130, 29)];
+
+    [webView synchronouslyLoadTestPageNamed:@"simple"];
+
+    auto zipFile = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testZIPData()]);
+    [attachment setFileWrapper:zipFile.get() contentType:(__bridge NSString *)kUTTypeZipArchive completion:nil];
+    EXPECT_FALSE([attachment isConnected]);
 }
 
 #pragma mark - Platform-specific tests