Changeset 273181 in webkit

Timestamp:

Feb 19, 2021 4:26:29 PM (3 years ago)

Author:

commit-queue@webkit.org

Message:

NSError returned by WebKit API should actually conform to NSSecureCoding
​https://bugs.webkit.org/show_bug.cgi?id=222204
<rdar://problem/63893583>

Patch by Alex Christensen <​achristensen@webkit.org> on 2021-02-19
Reviewed by Chris Dumez.

Source/WebKit:

When an NSError is encoded, its userInfo is encoded, which means everything we put in userInfo should be encodable.
WKReloadFrameErrorRecoveryAttempter was not, so let's make it! When encoded, it will just encode nothing.
When decoded, it will produce a WKReloadFrameErrorRecoveryAttempter that does nothing, which is valid behavior,
and more desirable than crashing.

• UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.h:
• UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.mm:

(-[WKReloadFrameErrorRecoveryAttempter encodeWithCoder:]):
(-[WKReloadFrameErrorRecoveryAttempter initWithCoder:]):
(+[WKReloadFrameErrorRecoveryAttempter supportsSecureCoding]):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:

(TestWebKitAPI::TEST):

• TestWebKitAPI/cocoa/TestNavigationDelegate.h:
• TestWebKitAPI/cocoa/TestNavigationDelegate.mm:

(-[TestNavigationDelegate waitForDidFailProvisionalNavigation]):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.h (modified) (2 diffs)
• Source/WebKit/UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.mm (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm (modified) (1 diff)
• Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.h (modified) (1 diff)
• Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.mm (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-02-19  Alex Christensen  <achristensen@webkit.org>
+
+        NSError returned by WebKit API should actually conform to NSSecureCoding
+        https://bugs.webkit.org/show_bug.cgi?id=222204
+        <rdar://problem/63893583>
+
+        Reviewed by Chris Dumez.
+
+        When an NSError is encoded, its userInfo is encoded, which means everything we put in userInfo should be encodable.
+        WKReloadFrameErrorRecoveryAttempter was not, so let's make it!  When encoded, it will just encode nothing.
+        When decoded, it will produce a WKReloadFrameErrorRecoveryAttempter that does nothing, which is valid behavior,
+        and more desirable than crashing.
+
+        * UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.h:
+        * UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.mm:
+        (-[WKReloadFrameErrorRecoveryAttempter encodeWithCoder:]):
+        (-[WKReloadFrameErrorRecoveryAttempter initWithCoder:]):
+        (+[WKReloadFrameErrorRecoveryAttempter supportsSecureCoding]):
+
 2021-02-19  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.h

@@ -24 +24 @@
  */
 
+#import "_WKErrorRecoveryAttempting.h"
 #import <WebKit/WKFoundation.h>
-
 #import <wtf/Forward.h>
 
@@ -31 +31 @@
 @class _WKFrameHandle;
 
-@interface WKReloadFrameErrorRecoveryAttempter : NSObject
+@interface WKReloadFrameErrorRecoveryAttempter : NSObject<_WKErrorRecoveryAttempting, NSSecureCoding>
 
 - (id)initWithWebView:(WKWebView *)webView frameHandle:(_WKFrameHandle *)frameHandle urlString:(const String&)urlString;

trunk/Source/WebKit/UIProcess/Cocoa/WKReloadFrameErrorRecoveryAttempter.mm

@@ -37 +37 @@
 #import <wtf/WeakObjCPtr.h>
 
-@interface WKReloadFrameErrorRecoveryAttempter () <_WKErrorRecoveryAttempting>
+@interface WKReloadFrameErrorRecoveryAttempter ()
 @end
 
@@ -72 +72 @@
 }
 
+- (void)encodeWithCoder:(NSCoder *)coder
+{
+}
+
+- (instancetype)initWithCoder:(NSCoder *)coder
+{
+    return [super init];
+}
+
++ (BOOL)supportsSecureCoding
+{
+    return YES;
+}
+
 @end

trunk/Tools/ChangeLog

@@ +1 @@
+2021-02-19  Alex Christensen  <achristensen@webkit.org>
+
+        NSError returned by WebKit API should actually conform to NSSecureCoding
+        https://bugs.webkit.org/show_bug.cgi?id=222204
+        <rdar://problem/63893583>
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/cocoa/TestNavigationDelegate.h:
+        * TestWebKitAPI/cocoa/TestNavigationDelegate.mm:
+        (-[TestNavigationDelegate waitForDidFailProvisionalNavigation]):
+
 2021-02-16  Jiewen Tan  <jiewen_tan@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm

@@ -574 +574 @@
 }
 
+TEST(WebKit, ErrorSecureCoding)
+{
+    HTTPServer server({{ "/", { HTTPResponse::TerminateConnection::Yes }}});
+    auto webView = [[WKWebView new] autorelease];
+    auto delegate = [[TestNavigationDelegate new] autorelease];
+    webView.navigationDelegate = delegate;
+    [webView loadRequest:server.request()];
+    NSError *error = [delegate waitForDidFailProvisionalNavigation];
+
+    EXPECT_WK_STREQ(NSStringFromClass([error.userInfo[_WKRecoveryAttempterErrorKey] class]), @"WKReloadFrameErrorRecoveryAttempter");
+    auto archiver = adoptNS([[NSKeyedArchiver alloc] initRequiringSecureCoding:YES]);
+    [archiver encodeObject:error forKey:NSKeyedArchiveRootObjectKey];
+    [archiver finishEncoding];
+    auto unarchiver = adoptNS([[NSKeyedUnarchiver alloc] initForReadingFromData:archiver.get().encodedData error:nullptr]);
+    NSError *decodedError = [unarchiver decodeObjectOfClasses:[NSSet setWithObjects:[NSDictionary class], [NSString class], [NSError class], NSClassFromString(@"WKReloadFrameErrorRecoveryAttempter"), nil] forKey:NSKeyedArchiveRootObjectKey];
+    EXPECT_EQ(decodedError.code, NSURLErrorNetworkConnectionLost);
+    EXPECT_WK_STREQ(decodedError.domain, NSURLErrorDomain);
+    EXPECT_WK_STREQ(NSStringFromClass([decodedError.userInfo[_WKRecoveryAttempterErrorKey] class]), @"WKReloadFrameErrorRecoveryAttempter");
+}
+
 // FIXME: Find out why these tests time out on Mojave.
 #if !PLATFORM(MAC) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500

trunk/Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.h

@@ -43 +43 @@
 - (void)waitForDidFinishNavigation;
 - (void)waitForDidFinishNavigationWithPreferences:(WKWebpagePreferences *)preferences;
-- (void)waitForDidFailProvisionalNavigation;
+- (NSError *)waitForDidFailProvisionalNavigation;
 
 @end

trunk/Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.mm

@@ -31 +31 @@
 #import <wtf/RetainPtr.h>
 
-@implementation TestNavigationDelegate
+@implementation TestNavigationDelegate {
+    RetainPtr<NSError> _navigationError;
+}
 
 - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction preferences:(WKWebpagePreferences *)preferences decisionHandler:(void (^)(WKNavigationActionPolicy, WKWebpagePreferences *))decisionHandler
@@ -152 +154 @@
 }
 
-- (void)waitForDidFailProvisionalNavigation
+- (NSError *)waitForDidFailProvisionalNavigation
 {
     EXPECT_FALSE(self.didFailProvisionalNavigation);
 
     __block bool finished = false;
-    self.didFailProvisionalNavigation = ^(WKWebView *, WKNavigation *, NSError *) {
+    self.didFailProvisionalNavigation = ^(WKWebView *, WKNavigation *, NSError *error) {
+        _navigationError = error;
         finished = true;
     };
@@ -164 +167 @@
 
     self.didFailProvisionalNavigation = nil;
+    return _navigationError.autorelease();
 }