WebKit Bugzilla
Attachment 372556 Details for
Bug 198902
: WebPageProxy should use the right path for sandbox extension
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-198902-20190620080455.patch (text/plain), 19.16 KB, created by
youenn fablet
on 2019-06-20 08:04:56 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2019-06-20 08:04:56 PDT
Size:
19.16 KB
patch
obsolete
>Subversion Revision: 246549 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index f4d8755efb0797419557e265e45e96a870b31523..9caa0d5c558e4ad05a1cab002d3531a9b3875e25 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,43 @@ >+2019-06-19 Youenn Fablet <youenn@apple.com> >+ >+ WebPageProxy should use the right path for sandbox extension >+ https://bugs.webkit.org/show_bug.cgi?id=198902 >+ <rdar://problem/50772810> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Store the sandbox path, if any, in UIProcess for each page in its PageLoadState. >+ This allows proper sandbox handling for reload after crash and process swap cases. >+ Store the sandbox path, if any, in the b/w list so that the sandbox path can be properly computed >+ during b/f navigation works. >+ >+ Add SPI for test purposes to check what is the current sandbox path. >+ >+ * Shared/WebBackForwardListItem.h: >+ (WebKit::WebBackForwardListItem::resourceDirectoryPath const): >+ (WebKit::WebBackForwardListItem::setResourceDirectoryPath): >+ * UIProcess/API/Cocoa/WKWebView.mm: >+ (-[WKWebView _resourceDirectoryPath]): >+ * UIProcess/API/Cocoa/WKWebViewPrivate.h: >+ * UIProcess/PageLoadState.cpp: >+ (WebKit::PageLoadState::activeResourceDirectoryPath const): >+ (WebKit::PageLoadState::pendingAPIResourceDirectoryPath const): >+ (WebKit::PageLoadState::setPendingAPIRequestURL): >+ (WebKit::PageLoadState::clearPendingAPIRequestURL): >+ * UIProcess/PageLoadState.h: >+ (WebKit::PageLoadState::setPendingAPIRequestURL): >+ * UIProcess/WebPageProxy.cpp: >+ (WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle): >+ (WebKit::WebPageProxy::loadRequestWithNavigationShared): >+ (WebKit::WebPageProxy::loadFile): >+ (WebKit::WebPageProxy::reload): >+ (WebKit::WebPageProxy::setIsNeverRichlyEditableForTouchBar): >+ (WebKit::WebPageProxy::requestDOMPasteAccess): >+ (WebKit::WebPageProxy::backForwardGoToItem): >+ (WebKit::WebPageProxy::didChangeProcessIsResponsive): >+ (WebKit::WebPageProxy::currentURL const): >+ * UIProcess/WebPageProxy.h: >+ > 2019-06-18 Adrian Perez de Castro <aperez@igalia.com> > > Unreviewed. Update OptionsWPE.cmake and NEWS for the 2.25.1 release >diff --git a/Source/WebKit/Shared/WebBackForwardListItem.h b/Source/WebKit/Shared/WebBackForwardListItem.h >index 5b2d2e033afd81684beef42c486ba87a8bb842f1..a9140337e730b60178e9a0f5fcb92a8888232f08 100644 >--- a/Source/WebKit/Shared/WebBackForwardListItem.h >+++ b/Source/WebKit/Shared/WebBackForwardListItem.h >@@ -67,6 +67,9 @@ public: > const String& url() const { return m_itemState.pageState.mainFrameState.urlString; } > const String& title() const { return m_itemState.pageState.title; } > >+ const String& resourceDirectoryPath() const { return m_resourceDirectoryPath; } >+ void setResourceDirectoryPath(String&& path) { m_resourceDirectoryPath = WTFMove(path); } >+ > bool itemIsInSameDocument(const WebBackForwardListItem&) const; > bool itemIsClone(const WebBackForwardListItem&); > >@@ -87,6 +90,7 @@ private: > void removeSuspendedPageFromProcessPool(); > > BackForwardListItemState m_itemState; >+ String m_resourceDirectoryPath; > WebCore::PageIdentifier m_pageID; > WebCore::ProcessIdentifier m_lastProcessIdentifier; > WeakPtr<SuspendedPageProxy> m_suspendedPage; >diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >index d11b57a76386e595f3aa6a1d2aafd6e4859d500f..4b232801b0a181625c6fea8ada38d4452f52d3e7 100644 >--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >@@ -982,6 +982,11 @@ - (NSURL *)URL > return [NSURL _web_URLWithWTFString:_page->pageLoadState().activeURL()]; > } > >+- (NSString *)_resourceDirectoryPath >+{ >+ return _page->currentResourceDirectoryPath(); >+} >+ > - (BOOL)isLoading > { > return _page->pageLoadState().isLoading(); >diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h >index c675bfff266b5748a566d2740bd6d731ede64909..52968cb176eaa9a8e0f36f053015e8e761735971 100644 >--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h >+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h >@@ -176,6 +176,8 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) { > > @property (nonatomic, readonly, getter=_isShowingNavigationGestureSnapshot) BOOL _showingNavigationGestureSnapshot; > >+@property (nonatomic, readonly) NSString *_resourceDirectoryPath WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); >+ > - (void)_close; > > - (void)_updateWebsitePolicies:(_WKWebsitePolicies *)websitePolicies WK_API_AVAILABLE(macos(10.13), ios(11.0)); >diff --git a/Source/WebKit/UIProcess/PageLoadState.cpp b/Source/WebKit/UIProcess/PageLoadState.cpp >index 10185a467764902d004f80c83767a1069510e3a5..c62b4e074597693a1c20bd0a55c4ebedba710266 100644 >--- a/Source/WebKit/UIProcess/PageLoadState.cpp >+++ b/Source/WebKit/UIProcess/PageLoadState.cpp >@@ -177,6 +177,11 @@ bool PageLoadState::hasUncommittedLoad() const > return isLoading(m_uncommittedState); > } > >+const String& PageLoadState::activeResourceDirectoryPath() const >+{ >+ return m_committedState.pendingAPIResourceDirectoryPath; >+} >+ > String PageLoadState::activeURL(const Data& data) > { > // If there is a currently pending URL, it is the active URL, >@@ -239,10 +244,16 @@ const String& PageLoadState::pendingAPIRequestURL() const > return m_committedState.pendingAPIRequestURL; > } > >-void PageLoadState::setPendingAPIRequestURL(const Transaction::Token& token, const String& pendingAPIRequestURL) >+const String& PageLoadState::pendingAPIResourceDirectoryPath() const >+{ >+ return m_committedState.pendingAPIResourceDirectoryPath; >+} >+ >+void PageLoadState::setPendingAPIRequestURL(const Transaction::Token& token, const String& pendingAPIRequestURL, const String& pendingAPIResourceDirectoryPath) > { > ASSERT_UNUSED(token, &token.m_pageLoadState == this); > m_uncommittedState.pendingAPIRequestURL = pendingAPIRequestURL; >+ m_uncommittedState.pendingAPIResourceDirectoryPath = pendingAPIResourceDirectoryPath; > } > > void PageLoadState::clearPendingAPIRequestURL(const Transaction::Token& token) >diff --git a/Source/WebKit/UIProcess/PageLoadState.h b/Source/WebKit/UIProcess/PageLoadState.h >index dbeaa9a1744a2cd90b58c457f816ae5989d47f7f..7ad36581ef739c38209b2d1eb839351cf8430a20 100644 >--- a/Source/WebKit/UIProcess/PageLoadState.h >+++ b/Source/WebKit/UIProcess/PageLoadState.h >@@ -132,6 +132,7 @@ public: > const String& unreachableURL() const { return m_committedState.unreachableURL; } > > String activeURL() const; >+ const String& activeResourceDirectoryPath() const; > > bool hasOnlySecureContent() const; > >@@ -141,7 +142,9 @@ public: > WebCertificateInfo* certificateInfo() const { return m_committedState.certificateInfo.get(); } > > const String& pendingAPIRequestURL() const; >- void setPendingAPIRequestURL(const Transaction::Token&, const String&); >+ const String& pendingAPIResourceDirectoryPath() const; >+ void setPendingAPIRequestURL(const Transaction::Token&, const String& pendingAPIRequestURL, const String& pendingAPIResourceDirectoryPath = { }); >+ void setPendingAPIResourceDirectoryPath(const Transaction::Token&, String&&); > void clearPendingAPIRequestURL(const Transaction::Token&); > > void didStartProvisionalLoad(const Transaction::Token&, const String& url, const String& unreachableURL); >@@ -205,6 +208,7 @@ private: > bool hasInsecureContent; > > String pendingAPIRequestURL; >+ String pendingAPIResourceDirectoryPath; > > String provisionalURL; > String url; >diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp >index 1302a84e107fa6d380730da1b86939ff29bfc2dc..2d3a399a7b1741c333ed97fb244ac541d27216a3 100644 >--- a/Source/WebKit/UIProcess/WebPageProxy.cpp >+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp >@@ -1048,19 +1048,27 @@ bool WebPageProxy::tryClose() > return false; > } > >-bool WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, SandboxExtension::Handle& sandboxExtensionHandle) >+void WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, const String& resourceDirectoryPath, SandboxExtension::Handle& sandboxExtensionHandle) > { > if (!url.isLocalFile()) >- return false; >+ return; >+ >+ if (!resourceDirectoryPath.isEmpty()) { >+ if (process.hasAssumedReadAccessToURL(URL { URL { }, resourceDirectoryPath })) >+ return; >+ >+ if (SandboxExtension::createHandle(resourceDirectoryPath, SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) >+ return; >+ } > > if (process.hasAssumedReadAccessToURL(url)) >- return false; >+ return; > > // Inspector resources are in a directory with assumed access. > ASSERT_WITH_SECURITY_IMPLICATION(!WebKit::isInspectorPage(*this)); > >- SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle); >- return true; >+ if (SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) >+ willAcquireUniversalFileReadSandboxExtension(process); > } > > #if !PLATFORM(COCOA) >@@ -1114,9 +1122,8 @@ void WebPageProxy::loadRequestWithNavigationShared(Ref<WebProcessProxy>&& proces > loadParameters.lockHistory = navigation.lockHistory(); > loadParameters.lockBackForwardList = navigation.lockBackForwardList(); > loadParameters.clientRedirectSourceForHistory = navigation.clientRedirectSourceForHistory(); >- bool createdExtension = maybeInitializeSandboxExtensionHandle(process, url, loadParameters.sandboxExtensionHandle); >- if (createdExtension) >- willAcquireUniversalFileReadSandboxExtension(process); >+ maybeInitializeSandboxExtensionHandle(process, url, m_pageLoadState.pendingAPIResourceDirectoryPath(), loadParameters.sandboxExtensionHandle); >+ > addPlatformLoadParameters(loadParameters); > > process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID); >@@ -1156,10 +1163,10 @@ RefPtr<API::Navigation> WebPageProxy::loadFile(const String& fileURLString, cons > > auto transaction = m_pageLoadState.transaction(); > >- m_pageLoadState.setPendingAPIRequestURL(transaction, fileURLString); >- > String resourceDirectoryPath = resourceDirectoryURL.fileSystemPath(); > >+ m_pageLoadState.setPendingAPIRequestURL(transaction, fileURLString, resourceDirectoryPath); >+ > LoadParameters loadParameters; > loadParameters.navigationID = navigation->navigationID(); > loadParameters.request = fileURL; >@@ -1336,9 +1343,7 @@ RefPtr<API::Navigation> WebPageProxy::reload(OptionSet<WebCore::ReloadOption> op > m_pageLoadState.setPendingAPIRequestURL(transaction, url); > > // We may not have an extension yet if back/forward list was reinstated after a WebProcess crash or a browser relaunch >- bool createdExtension = maybeInitializeSandboxExtensionHandle(m_process, URL(URL(), url), sandboxExtensionHandle); >- if (createdExtension) >- willAcquireUniversalFileReadSandboxExtension(m_process); >+ maybeInitializeSandboxExtensionHandle(m_process, URL(URL(), url), currentResourceDirectoryPath(), sandboxExtensionHandle); > } > > if (!hasRunningProcess()) >@@ -5714,6 +5719,10 @@ void WebPageProxy::requestDOMPasteAccess(const WebCore::IntRect& elementRect, co > void WebPageProxy::backForwardAddItem(BackForwardListItemState&& itemState) > { > auto item = WebBackForwardListItem::create(WTFMove(itemState), pageID()); >+ auto path = currentResourceDirectoryPath(); >+ if (!path.isEmpty()) >+ item->setResourceDirectoryPath(WTFMove(path)); >+ > m_backForwardList->addItem(WTFMove(item)); > } > >@@ -5736,9 +5745,7 @@ void WebPageProxy::backForwardGoToItemShared(Ref<WebProcessProxy>&& process, con > return completionHandler({ }); > > SandboxExtension::Handle sandboxExtensionHandle; >- bool createdExtension = maybeInitializeSandboxExtensionHandle(process, URL(URL(), item->url()), sandboxExtensionHandle); >- if (createdExtension) >- willAcquireUniversalFileReadSandboxExtension(process); >+ maybeInitializeSandboxExtensionHandle(process, URL(URL(), item->url()), item->resourceDirectoryPath(), sandboxExtensionHandle); > m_backForwardList->goToItem(*item); > completionHandler(WTFMove(sandboxExtensionHandle)); > } >@@ -6789,6 +6796,15 @@ String WebPageProxy::currentURL() const > return url; > } > >+String WebPageProxy::currentResourceDirectoryPath() const >+{ >+ if (!m_pageLoadState.activeResourceDirectoryPath().isEmpty()) >+ return m_pageLoadState.activeResourceDirectoryPath(); >+ if (auto* item = m_backForwardList->currentItem()) >+ return item->resourceDirectoryPath(); >+ return { }; >+} >+ > void WebPageProxy::processDidTerminate(ProcessTerminationReason reason) > { > if (reason != ProcessTerminationReason::NavigationSwap) >diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h >index b7736b6a7d483bc34020f75960feade237cff726..4789df068d2c210deb3d08683b06327b3cf25f83 100644 >--- a/Source/WebKit/UIProcess/WebPageProxy.h >+++ b/Source/WebKit/UIProcess/WebPageProxy.h >@@ -1559,6 +1559,8 @@ public: > > void requestStorageSpace(uint64_t frameID, const String& originIdentifier, const String& databaseName, const String& displayName, uint64_t currentQuota, uint64_t currentOriginUsage, uint64_t currentDatabaseUsage, uint64_t expectedUsage, WTF::CompletionHandler<void(uint64_t)>&&); > >+ String currentResourceDirectoryPath() const; >+ > private: > WebPageProxy(PageClient&, WebProcessProxy&, WebCore::PageIdentifier, Ref<API::PageConfiguration>&&); > void platformInitialize(); >@@ -1895,7 +1897,7 @@ private: > void setPluginComplexTextInputState(uint64_t pluginComplexTextInputIdentifier, uint64_t complexTextInputState); > #endif > >- bool maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL&, SandboxExtension::Handle&); >+ void maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL&, const String&, SandboxExtension::Handle&); > > #if USE(AUTOMATIC_TEXT_REPLACEMENT) > void toggleSmartInsertDelete(); >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 46999cceae7f439eb9380eda97e8be0247e25937..756cd2c9ae48610b188143b24bbcca4bf447a304 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,15 @@ >+2019-06-19 Youenn Fablet <youenn@apple.com> >+ >+ WebPageProxy should use the right path for sandbox extension >+ https://bugs.webkit.org/show_bug.cgi?id=198902 >+ <rdar://problem/50772810> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm: >+ (TEST): >+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm: >+ > 2019-06-18 David Quesada <david_quesada@apple.com> > > REGRESSION: _WKDownload.OriginatingWebView and _WKDownload.CrashAfterDownloadDidFinishWhenDownloadProxyHoldsTheLastRefOnWebProcessPool failing >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm >index 24f229bf1772ee340428f8d61def75242d06635d..fd6f81cec25aacb34d17e62ff41a08b11e0053ec 100644 >--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm >@@ -29,6 +29,7 @@ > #import "Test.h" > #import "TestNavigationDelegate.h" > #import <WebKit/WebKit.h> >+#import <WebKit/WebViewPrivate.h> > #import <wtf/RetainPtr.h> > > >@@ -42,6 +43,7 @@ TEST(WKWebView, LoadTwoFiles) > NSURL *file = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]; > [webView loadFileURL:file allowingReadAccessToURL:file.URLByDeletingLastPathComponent]; > [delegate waitForDidFinishNavigation]; >+ EXPECT_WK_STREQ(webView.get()._resourceDirectoryPath, file.URLByDeletingLastPathComponent.path); > > // Load a second file: resource that is in a completely different directory from the above > file = [[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]; >@@ -49,8 +51,18 @@ TEST(WKWebView, LoadTwoFiles) > [[NSFileManager defaultManager] createDirectoryAtURL:targetURL.URLByDeletingLastPathComponent withIntermediateDirectories:YES attributes:nil error:nil]; > [[NSFileManager defaultManager] removeItemAtURL:targetURL error:nil]; > [[NSFileManager defaultManager] copyItemAtURL:file toURL:targetURL error:nil]; >- >+ > // If this second load succeeds (e.g. doesn't timeout due to a sandbox violation) the test passes > [webView loadFileURL:targetURL allowingReadAccessToURL:targetURL.URLByDeletingLastPathComponent]; > [delegate waitForDidFinishNavigation]; >+ EXPECT_WK_STREQ(webView.get()._resourceDirectoryPath, targetURL.URLByDeletingLastPathComponent.path); >+ >+ [webView _killWebContentProcessAndResetState]; >+ [webView reload]; >+ [delegate waitForDidFinishNavigation]; >+ EXPECT_WK_STREQ(webView.get()._resourceDirectoryPath, targetURL.URLByDeletingLastPathComponent.path); >+ >+ [webView goBack]; >+ [delegate waitForDidFinishNavigation]; >+ EXPECT_WK_STREQ(webView.get()._resourceDirectoryPath, file.URLByDeletingLastPathComponent.path); > } >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >index 6217c9898a56fc3720b9120f04dfc90cb52ac17a..19d1dd5f2c183ef399f732f4d3cf6de2b8761034 100644 >--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >@@ -6295,3 +6295,33 @@ TEST(ProcessSwap, SuspendAllMediaPlayback) > [webView synchronouslyLoadTestPageNamed:@"video-with-audio"]; > TestWebKitAPI::Util::run(¬Playing); > } >+ >+TEST(ProcessSwap, PassSandboxExtension) >+{ >+ auto processPoolConfiguration = psonProcessPoolConfiguration(); >+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]); >+ >+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]); >+ configuration.get().mediaTypesRequiringUserActionForPlayback = WKAudiovisualMediaTypeNone; >+#if TARGET_OS_IPHONE >+ configuration.get().allowsInlineMediaPlayback = YES; >+#endif >+ [configuration setProcessPool:processPool.get()]; >+ auto handler = adoptNS([[PSONScheme alloc] init]); >+ [configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"]; >+ >+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]); >+ >+ auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]); >+ [webView setNavigationDelegate:navigationDelegate.get()]; >+ >+ [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]]]; >+ TestWebKitAPI::Util::run(&done); >+ done = false; >+ >+ NSURL *file = [[NSBundle mainBundle] URLForResource:@"autoplay-with-controls" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]; >+ [webView loadFileURL:file allowingReadAccessToURL:file.URLByDeletingLastPathComponent]; >+ [webView waitForMessage:@"loaded"]; >+ >+ EXPECT_WK_STREQ(webView.get()._resourceDirectoryPath, file.URLByDeletingLastPathComponent.path); >+}
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 198902
:
372216
|
372218
|
372219
|
372275
|
372340
|
372350
|
372513
|
372556
|
372574