[webkit-changes] [244573] trunk

Tue, 23 Apr 2019 17:29:42 -0700

Title: [244573] trunk
Revision
244573
Author
[email protected]
Date
2019-04-23 17:24:27 -0700 (Tue, 23 Apr 2019)

Log Message

[iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations
https://bugs.webkit.org/show_bug.cgi?id=196749
<rdar://problem/35773454>

Reviewed by Daniel Bates.

Source/WebCore:

QuickLook previews are in a non-local origin defined by a unique x-apple-ql-id: URL, which
isolates the origin that hosted the document from the document preview itself. When a
QuickLook document is loaded as a file: URL, SecurityOrigin's protections against loading
local resources from non-local origins prevented navigations like location.reload() and
fragment navigations.

To allow reloads and same-document navigations in QuickLook documents loaded from file: URLs,
we should grant the QuickLook document's SecurityOrigin access to the file path that loaded
the preview.

Added a new API test.

* dom/Document.cpp:
(WebCore::Document::applyQuickLookSandbox):
* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::createNonLocalWithAllowedFilePath):
(WebCore::SecurityOrigin::canDisplay const):
* page/SecurityOrigin.h:

Tools:

Added a new QuickLook API test and added new expectations to existing QuickLook tests.

* TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
(-[QuickLookDelegate webView:didStartProvisionalNavigation:]):
(-[QuickLookDelegate webView:didFinishNavigation:]):
(-[QuickLookDelegate _webView:didFailNavigation:withError:userInfo:]):
(-[QuickLookDelegate webView:didFailProvisionalNavigation:withError:]):
(runTest):
(runTestDecideBeforeLoading):
(runTestDecideAfterLoading):
(TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (244572 => 244573)


--- trunk/Source/WebCore/ChangeLog	2019-04-24 00:13:00 UTC (rev 244572)
+++ trunk/Source/WebCore/ChangeLog	2019-04-24 00:24:27 UTC (rev 244573)
@@ -1,3 +1,30 @@
+2019-04-23  Andy Estes  <[email protected]>
+
+        [iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations
+        https://bugs.webkit.org/show_bug.cgi?id=196749
+        <rdar://problem/35773454>
+
+        Reviewed by Daniel Bates.
+
+        QuickLook previews are in a non-local origin defined by a unique x-apple-ql-id: URL, which
+        isolates the origin that hosted the document from the document preview itself. When a
+        QuickLook document is loaded as a file: URL, SecurityOrigin's protections against loading
+        local resources from non-local origins prevented navigations like location.reload() and
+        fragment navigations.
+
+        To allow reloads and same-document navigations in QuickLook documents loaded from file: URLs,
+        we should grant the QuickLook document's SecurityOrigin access to the file path that loaded
+        the preview.
+
+        Added a new API test.
+
+        * dom/Document.cpp:
+        (WebCore::Document::applyQuickLookSandbox):
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::createNonLocalWithAllowedFilePath):
+        (WebCore::SecurityOrigin::canDisplay const):
+        * page/SecurityOrigin.h:
+
 2019-04-23  Devin Rousso  <[email protected]>
 
         Web Inspector: Debugger: remove ASSERT_NOT_REACHED where it's possible to reach

Modified: trunk/Source/WebCore/dom/Document.cpp (244572 => 244573)


--- trunk/Source/WebCore/dom/Document.cpp	2019-04-24 00:13:00 UTC (rev 244572)
+++ trunk/Source/WebCore/dom/Document.cpp	2019-04-24 00:24:27 UTC (rev 244573)
@@ -7225,10 +7225,13 @@
 
 void Document::applyQuickLookSandbox()
 {
-    const URL& responseURL = m_frame->loader().activeDocumentLoader()->responseURL();
+    auto& documentLoader = *m_frame->loader().activeDocumentLoader();
+    auto documentURL = documentLoader.documentURL();
+    auto& responseURL = documentLoader.responseURL();
+    ASSERT(!documentURL.protocolIs(QLPreviewProtocol));
     ASSERT(responseURL.protocolIs(QLPreviewProtocol));
 
-    auto securityOrigin = SecurityOrigin::create(responseURL);
+    auto securityOrigin = SecurityOrigin::createNonLocalWithAllowedFilePath(responseURL, documentURL.fileSystemPath());
     securityOrigin->setStorageBlockingPolicy(SecurityOrigin::BlockAllStorage);
     setSecurityOriginPolicy(SecurityOriginPolicy::create(WTFMove(securityOrigin)));

Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (244572 => 244573)


--- trunk/Source/WebCore/page/SecurityOrigin.cpp	2019-04-24 00:13:00 UTC (rev 244572)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp	2019-04-24 00:24:27 UTC (rev 244573)
@@ -205,6 +205,14 @@
     return origin;
 }
 
+Ref<SecurityOrigin> SecurityOrigin::createNonLocalWithAllowedFilePath(const URL& url, const String& filePath)
+{
+    ASSERT(!url.isLocalFile());
+    auto securityOrigin = SecurityOrigin::create(url);
+    securityOrigin->m_filePath = filePath;
+    return securityOrigin;
+}
+
 Ref<SecurityOrigin> SecurityOrigin::isolatedCopy() const
 {
     return adoptRef(*new SecurityOrigin(this));
@@ -362,7 +370,13 @@
     if (SchemeRegistry::shouldTreatURLSchemeAsDisplayIsolated(protocol))
         return equalIgnoringASCIICase(m_data.protocol, protocol) || SecurityPolicy::isAccessToURLWhiteListed(this, url);
 
-    if (SecurityPolicy::restrictAccessToLocal() && SchemeRegistry::shouldTreatURLSchemeAsLocal(protocol))
+    if (!SecurityPolicy::restrictAccessToLocal())
+        return true;
+
+    if (url.isLocalFile() && url.fileSystemPath() == m_filePath)
+        return true;
+
+    if (SchemeRegistry::shouldTreatURLSchemeAsLocal(protocol))
         return canLoadLocalResources() || SecurityPolicy::isAccessToURLWhiteListed(this, url);
 
     return true;

Modified: trunk/Source/WebCore/page/SecurityOrigin.h (244572 => 244573)


--- trunk/Source/WebCore/page/SecurityOrigin.h	2019-04-24 00:13:00 UTC (rev 244572)
+++ trunk/Source/WebCore/page/SecurityOrigin.h	2019-04-24 00:24:27 UTC (rev 244573)
@@ -54,6 +54,12 @@
     WEBCORE_EXPORT static Ref<SecurityOrigin> createFromString(const String&);
     WEBCORE_EXPORT static Ref<SecurityOrigin> create(const String& protocol, const String& host, Optional<uint16_t> port);
 
+    // QuickLook documents are in non-local origins even when loaded from file: URLs. They need to
+    // be allowed to display their own file: URLs in order to perform reloads and same-document
+    // navigations. This lets those documents specify the file path that should be allowed to be
+    // displayed from their non-local origin.
+    static Ref<SecurityOrigin> createNonLocalWithAllowedFilePath(const URL&, const String& filePath);
+
     // Some URL schemes use nested URLs for their security context. For example,
     // filesystem URLs look like the following:
     //

Modified: trunk/Tools/ChangeLog (244572 => 244573)


--- trunk/Tools/ChangeLog	2019-04-24 00:13:00 UTC (rev 244572)
+++ trunk/Tools/ChangeLog	2019-04-24 00:24:27 UTC (rev 244573)
@@ -1,3 +1,23 @@
+2019-04-23  Andy Estes  <[email protected]>
+
+        [iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations
+        https://bugs.webkit.org/show_bug.cgi?id=196749
+        <rdar://problem/35773454>
+
+        Reviewed by Daniel Bates.
+
+        Added a new QuickLook API test and added new expectations to existing QuickLook tests.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
+        (-[QuickLookDelegate webView:didStartProvisionalNavigation:]):
+        (-[QuickLookDelegate webView:didFinishNavigation:]):
+        (-[QuickLookDelegate _webView:didFailNavigation:withError:userInfo:]):
+        (-[QuickLookDelegate webView:didFailProvisionalNavigation:withError:]):
+        (runTest):
+        (runTestDecideBeforeLoading):
+        (runTestDecideAfterLoading):
+        (TEST):
+
 2019-04-23  John Wilander  <[email protected]>
 
         Remove Ad Click Attribution data when removing website data

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm (244572 => 244573)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm	2019-04-24 00:13:00 UTC (rev 244572)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm	2019-04-24 00:24:27 UTC (rev 244573)
@@ -57,8 +57,10 @@
 - (instancetype)initWithExpectedFileURL:(NSURL *)fileURL previewMIMEType:(NSString *)mimeType responsePolicy:(WKNavigationResponsePolicy)responsePolicy;
 - (void)verifyDownload;
 
-@property (nonatomic) BOOL didStartQuickLookLoad;
-@property (nonatomic) BOOL didFinishQuickLookLoad;
+@property (nonatomic, readonly) BOOL didFailNavigation;
+@property (nonatomic, readonly) BOOL didFinishNavigation;
+@property (nonatomic, readonly) BOOL didFinishQuickLookLoad;
+@property (nonatomic, readonly) BOOL didStartQuickLookLoad;
 
 @end
 
@@ -107,6 +109,14 @@
     return self;
 }
 
+- (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
+{
+    _didFailNavigation = NO;
+    _didFinishNavigation = NO;
+    _didFinishQuickLookLoad = NO;
+    _didStartQuickLookLoad = NO;
+}
+
 - (void)webView:(WKWebView *)webView decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
 {
     EXPECT_TRUE(navigationResponse.canShowMIMEType);
@@ -134,11 +144,25 @@
 
 - (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
 {
+    EXPECT_FALSE(_didFailNavigation);
+    EXPECT_FALSE(_didFinishNavigation);
+    _didFinishNavigation = YES;
     isDone = true;
 }
 
+- (void)_webView:(WKWebView *)webView didFailNavigation:(WKNavigation *)navigation withError:(NSError *)error userInfo:(id<NSSecureCoding>)userInfo
+{
+    EXPECT_FALSE(_didFailNavigation);
+    EXPECT_FALSE(_didFinishNavigation);
+    _didFailNavigation = YES;
+    isDone = true;
+}
+
 - (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(WKNavigation *)navigation withError:(NSError *)error
 {
+    EXPECT_FALSE(_didFailNavigation);
+    EXPECT_FALSE(_didFinishNavigation);
+    _didFailNavigation = YES;
     isDone = true;
 }
 
@@ -206,7 +230,7 @@
 
 @end
 
-static void runTest(QuickLookDelegate *delegate, NSURLRequest *request, BOOL shouldDecidePolicyBeforeLoading)
+static RetainPtr<WKWebView> runTest(QuickLookDelegate *delegate, NSURLRequest *request, BOOL shouldDecidePolicyBeforeLoading)
 {
     auto processPool = adoptNS([[WKProcessPool alloc] init]);
     [processPool _setDownloadDelegate:delegate];
@@ -221,16 +245,18 @@
 
     isDone = false;
     Util::run(&isDone);
+
+    return webView;
 }
 
-static void runTestDecideBeforeLoading(QuickLookDelegate *delegate, NSURLRequest *request)
+static RetainPtr<WKWebView> runTestDecideBeforeLoading(QuickLookDelegate *delegate, NSURLRequest *request)
 {
-    runTest(delegate, request, YES);
+    return runTest(delegate, request, YES);
 }
 
-static void runTestDecideAfterLoading(QuickLookDelegate *delegate, NSURLRequest *request)
+static RetainPtr<WKWebView> runTestDecideAfterLoading(QuickLookDelegate *delegate, NSURLRequest *request)
 {
-    runTest(delegate, request, NO);
+    return runTest(delegate, request, NO);
 }
 
 TEST(QuickLook, AllowResponseBeforeLoadingPreview)
@@ -237,8 +263,10 @@
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL responsePolicy:WKNavigationResponsePolicyAllow]);
     runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didStartQuickLookLoad]);
-    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
 }
 
 TEST(QuickLook, AllowResponseAfterLoadingPreview)
@@ -245,8 +273,10 @@
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL previewMIMEType:pagesDocumentPreviewMIMEType responsePolicy:WKNavigationResponsePolicyAllow]);
     runTestDecideAfterLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didStartQuickLookLoad]);
-    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
 }
 
 @interface QuickLookAsyncDelegate : QuickLookDelegate
@@ -270,8 +300,10 @@
 {
     auto delegate = adoptNS([[QuickLookAsyncDelegate alloc] initWithExpectedFileURL:pagesDocumentURL responsePolicy:WKNavigationResponsePolicyAllow]);
     runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didStartQuickLookLoad]);
-    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
 }
 
 TEST(QuickLook, AsyncAllowResponseAfterLoadingPreview)
@@ -278,8 +310,10 @@
 {
     auto delegate = adoptNS([[QuickLookAsyncDelegate alloc] initWithExpectedFileURL:pagesDocumentURL previewMIMEType:pagesDocumentPreviewMIMEType responsePolicy:WKNavigationResponsePolicyAllow]);
     runTestDecideAfterLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didStartQuickLookLoad]);
-    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
 }
 
 TEST(QuickLook, CancelResponseBeforeLoadingPreview)
@@ -286,8 +320,10 @@
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL responsePolicy:WKNavigationResponsePolicyCancel]);
     runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFinishNavigation]);
+    EXPECT_FALSE([delegate didFinishQuickLookLoad]);
     EXPECT_FALSE([delegate didStartQuickLookLoad]);
-    EXPECT_FALSE([delegate didFinishQuickLookLoad]);
+    EXPECT_TRUE([delegate didFailNavigation]);
 }
 
 TEST(QuickLook, CancelResponseAfterLoadingPreview)
@@ -294,8 +330,10 @@
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL previewMIMEType:pagesDocumentPreviewMIMEType responsePolicy:WKNavigationResponsePolicyCancel]);
     runTestDecideAfterLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didStartQuickLookLoad]);
-    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
 }
 
 TEST(QuickLook, DownloadResponseBeforeLoadingPreview)
@@ -302,8 +340,10 @@
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL responsePolicy:_WKNavigationResponsePolicyBecomeDownload]);
     runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFinishNavigation]);
+    EXPECT_FALSE([delegate didFinishQuickLookLoad]);
     EXPECT_FALSE([delegate didStartQuickLookLoad]);
-    EXPECT_FALSE([delegate didFinishQuickLookLoad]);
+    EXPECT_TRUE([delegate didFailNavigation]);
 
     Util::run(&downloadIsDone);
     [delegate verifyDownload];
@@ -313,8 +353,10 @@
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL previewMIMEType:pagesDocumentPreviewMIMEType responsePolicy:_WKNavigationResponsePolicyBecomeDownload]);
     runTestDecideAfterLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didStartQuickLookLoad]);
-    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
 }
 
 @interface QuickLookPasswordDelegate : QuickLookDelegate
@@ -336,7 +378,11 @@
     NSURL *passwordProtectedDocumentURL = [NSBundle.mainBundle URLForResource:@"password-protected" withExtension:@"pages" subdirectory:@"TestWebKitAPI.resources"];
     auto delegate = adoptNS([[QuickLookPasswordDelegate alloc] initWithExpectedFileURL:passwordProtectedDocumentURL responsePolicy:WKNavigationResponsePolicyAllow]);
     runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:passwordProtectedDocumentURL]);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_FALSE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didRequestPassword]);
+    EXPECT_TRUE([delegate didStartQuickLookLoad]);
 }
 
 TEST(QuickLook, RequestPasswordAfterLoadingPreview)
@@ -344,9 +390,39 @@
     NSURL *passwordProtectedDocumentURL = [NSBundle.mainBundle URLForResource:@"password-protected" withExtension:@"pages" subdirectory:@"TestWebKitAPI.resources"];
     auto delegate = adoptNS([[QuickLookPasswordDelegate alloc] initWithExpectedFileURL:passwordProtectedDocumentURL previewMIMEType:pagesDocumentPreviewMIMEType responsePolicy:WKNavigationResponsePolicyAllow]);
     runTestDecideAfterLoading(delegate.get(), [NSURLRequest requestWithURL:passwordProtectedDocumentURL]);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_FALSE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
     EXPECT_TRUE([delegate didRequestPassword]);
+    EXPECT_TRUE([delegate didStartQuickLookLoad]);
 }
 
+TEST(QuickLook, ReloadAndSameDocumentNavigation)
+{
+    auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL responsePolicy:WKNavigationResponsePolicyAllow]);
+    auto webView = runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
+    EXPECT_TRUE([delegate didStartQuickLookLoad]);
+
+    isDone = false;
+    [webView evaluateJavaScript:@"window.location.reload()" completionHandler:nil];
+    Util::run(&isDone);
+    EXPECT_FALSE([delegate didFailNavigation]);
+    EXPECT_TRUE([delegate didFinishNavigation]);
+    EXPECT_TRUE([delegate didFinishQuickLookLoad]);
+    EXPECT_TRUE([delegate didStartQuickLookLoad]);
+
+    isDone = false;
+    [webView evaluateJavaScript:@"window.location = '#test'; window.location.hash" completionHandler:^(id _Nullable value, NSError * _Nullable error) {
+        EXPECT_NULL(error);
+        EXPECT_WK_STREQ(@"#test", value);
+        isDone = true;
+    }];
+    Util::run(&isDone);
+}
+
 @interface QuickLookFrameLoadDelegate : NSObject <WebFrameLoadDelegate>
 @end
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to