Attachment 345885 Details for Bug 188078 – Patch

[patch] Patch

bug-188078-20180726160037.patch (text/plain), 10.93 KB, created by Chris Dumez on 2018-07-26 16:00:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-07-26 16:00:37 PDT

Size: 10.93 KB

patch

obsolete

>Subversion Revision: 234280
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e4b95a9787aafa447ed4ff94a5d75d517e480118..b63ce97afb3908bb0b37ce3af2eef579357ae8c8 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-07-26  Chris Dumez  <cdumez@apple.com>
>+
>+        Loading a file URL and then issuing a reload right away causes the load to fail due to sandboxing
>+        https://bugs.webkit.org/show_bug.cgi?id=188078
>+        <rdar://problem/42562493>
>+
>+        Reviewed by Geoff Garen.
>+
>+        When WebPageProxy::loadFile() is called, we create a SandboxExtension::Handle for the resource path provided
>+        by the caller and pass it to the WebProcess. WebPage::loadRequest() then calls SandboxExtensionTracker::beginLoad()
>+        to store this handle in m_provisionalSandboxExtension for later consumption.
>+
>+        If a reload is issued before this sandbox extension has been consumed, then the following happens:
>+        1. WebPageProxy::reload() does NOT create a SandboxExtension::Handle because it has already issued one earlier.
>+           maybeInitializeSandboxExtensionHandle() returns early due to m_process->hasAssumedReadAccessToURL(url) check.
>+        2. WebPage::reload() then calls SandboxExtensionTracker::beginLoad() with a null handle, which overwrites the
>+           previous m_provisionalSandboxExtension its needs.
>+        3. The load fails because the WebContent process is missing the sandbox extension.
>+
>+        To address the issue, SandboxExtensionTracker::beginLoad() is updated to only overwrite m_provisionalSandboxExtension
>+        if the new handle is not null. This avoids inadvertently clearing a valid sandbox extension we may need for the load,
>+        since the UIProcess sends us a null handle if it previously sent us a sandbox extension for the path in question.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::SandboxExtensionTracker::beginLoad):
>+
> 2018-07-26  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         String(View) should have a splitAllowingEmptyEntries function instead of a flag parameter
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 0153080a3d9de6a6df157aca14799f51aa3f03ff..6470733381de3d1ac4c000b613364b0f4676dbf0 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -4086,7 +4086,8 @@ void WebPage::SandboxExtensionTracker::beginLoad(WebFrame* frame, SandboxExtensi
> {
>     ASSERT_UNUSED(frame, frame->isMainFrame());
> 
>-    setPendingProvisionalSandboxExtension(SandboxExtension::create(WTFMove(handle)));
>+    if (auto sandboxExtension = SandboxExtension::create(WTFMove(handle)))
>+        setPendingProvisionalSandboxExtension(WTFMove(sandboxExtension));
> }
> 
> void WebPage::SandboxExtensionTracker::setPendingProvisionalSandboxExtension(RefPtr<SandboxExtension>&& pendingProvisionalSandboxExtension)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 28e437adb892bd1be3ff6b0b12da12022e867c81..54af4d1ea10c08b27a0f7df88cb705240798afdc 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-07-26  Chris Dumez  <cdumez@apple.com>
>+
>+        Loading a file URL and then issuing a reload right away causes the load to fail due to sandboxing
>+        https://bugs.webkit.org/show_bug.cgi?id=188078
>+        <rdar://problem/42562493>
>+
>+        Reviewed by Geoff Garen.
>+
>+        Add API test coverage. It exercises the right code path but unfortunately is not yet a regression
>+        test because TestWebKitAPI does not appear to be sandboxed (rdar://problem/42638129).
>+
>+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
>+        * TestWebKitAPI/Tests/WebKitCocoa/LoadFileThenReload.mm: Added.
>+        (-[LoadFileThenReloadDelegate webView:didFinishNavigation:]):
>+        (-[LoadFileThenReloadDelegate webView:didFailProvisionalNavigation:withError:]):
>+        (-[LoadFileThenReloadDelegate webView:didFailNavigation:withError:]):
>+        (TEST):
>+
> 2018-07-26  Aakash Jain  <aakash_jain@apple.com>
> 
>         [ews-build] Add build step to run API Tests
>diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>index af7b554af24a67242dff8c877af92b55d57c599d..ac532418009ad463f111939c8ce1bfdb8079284f 100644
>--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>@@ -157,6 +157,7 @@
> 		4433A396208044140091ED57 /* SynchronousTimeoutTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4433A395208044130091ED57 /* SynchronousTimeoutTests.mm */; };
> 		44817A2F1F0486BF00003810 /* WKRequestActivatedElementInfo.mm in Sources */ = {isa = PBXBuildFile; fileRef = 44817A2E1F0486BF00003810 /* WKRequestActivatedElementInfo.mm */; };
> 		448D7E471EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 448D7E451EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp */; };
>+		4612C2B9210A6ACE00B788A6 /* LoadFileThenReload.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4612C2B8210A6ABF00B788A6 /* LoadFileThenReload.mm */; };
> 		46397B951DC2C850009A78AE /* DOMNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46397B941DC2C850009A78AE /* DOMNode.mm */; };
> 		4647B1261EBA3B850041D7EF /* ProcessDidTerminate.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */; };
> 		466C3843210637DE006A88DE /* notify-resourceLoadObserver.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 466C3842210637CE006A88DE /* notify-resourceLoadObserver.html */; };
>@@ -1398,6 +1399,7 @@
> 		44817A2E1F0486BF00003810 /* WKRequestActivatedElementInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKRequestActivatedElementInfo.mm; sourceTree = "<group>"; };
> 		448D7E451EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = EnvironmentUtilitiesTest.cpp; sourceTree = "<group>"; };
> 		44A622C114A0E2B60048515B /* WTFStringUtilities.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WTFStringUtilities.h; sourceTree = "<group>"; };
>+		4612C2B8210A6ABF00B788A6 /* LoadFileThenReload.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = LoadFileThenReload.mm; sourceTree = "<group>"; };
> 		46397B941DC2C850009A78AE /* DOMNode.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DOMNode.mm; sourceTree = "<group>"; };
> 		4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ProcessDidTerminate.cpp; sourceTree = "<group>"; };
> 		4656A75720F9054F0002E21F /* SimpleServiceWorkerRegistrations-3.sqlite3 */ = {isa = PBXFileReference; lastKnownFileType = file; path = "SimpleServiceWorkerRegistrations-3.sqlite3"; sourceTree = "<group>"; };
>@@ -2290,6 +2292,7 @@
> 				C25CCA051E51380B0026CB8A /* LineBreaking.mm */,
> 				37D36ED61AF42ECD00BAF5D9 /* LoadAlternateHTMLString.mm */,
> 				A125478D1DB18B9400358564 /* LoadDataWithNilMIMEType.mm */,
>+				4612C2B8210A6ABF00B788A6 /* LoadFileThenReload.mm */,
> 				57901FAC1CAF12C200ED64F9 /* LoadInvalidURLRequest.mm */,
> 				51E6A8921D2F1BEC00C004B6 /* LocalStorageClear.mm */,
> 				CA38459520AE012E00990D3B /* LocalStorageDatabaseTracker.mm */,
>@@ -3760,6 +3763,7 @@
> 				7CCE7EFE1A411AE600447C4C /* LoadAlternateHTMLStringWithNonDirectoryURL.cpp in Sources */,
> 				7CCE7EFF1A411AE600447C4C /* LoadCanceledNoServerRedirectCallback.cpp in Sources */,
> 				A125478F1DB18B9400358564 /* LoadDataWithNilMIMEType.mm in Sources */,
>+				4612C2B9210A6ACE00B788A6 /* LoadFileThenReload.mm in Sources */,
> 				5C838F7F1DB04F900082858F /* LoadInvalidURLRequest.mm in Sources */,
> 				7C83E0C01D0A652700FEBCF3 /* LoadInvalidURLRequest.mm in Sources */,
> 				7CCE7F001A411AE600447C4C /* LoadPageOnCrash.cpp in Sources */,
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileThenReload.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileThenReload.mm
>new file mode 100644
>index 0000000000000000000000000000000000000000..511a8a1d30c122f28e76754d7fce608f090a53d9
>--- /dev/null
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileThenReload.mm
>@@ -0,0 +1,73 @@
>+/*
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
>+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
>+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>+ * THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+
>+#if WK_API_ENABLED
>+
>+#import "PlatformUtilities.h"
>+#import "Test.h"
>+#import <WebKit/WebKit.h>
>+#import <wtf/RetainPtr.h>
>+
>+static bool done;
>+
>+@interface LoadFileThenReloadDelegate : NSObject <WKNavigationDelegate>
>+@end
>+
>+@implementation LoadFileThenReloadDelegate
>+
>+- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
>+{
>+    done = true;
>+}
>+
>+- (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(WKNavigation *)navigation withError:(NSError *)error
>+{
>+    EXPECT_TRUE(false);
>+}
>+
>+- (void)webView:(WKWebView *)webView didFailNavigation:(WKNavigation *)navigation withError:(NSError *)error
>+{
>+    EXPECT_TRUE(false);
>+}
>+
>+@end
>+
>+TEST(WKWebView, LoadFileThenReload)
>+{
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>+
>+    auto delegate = adoptNS([[LoadFileThenReloadDelegate alloc] init]);
>+    [webView setNavigationDelegate:delegate.get()];
>+
>+    NSURL *file = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
>+    [webView loadFileURL:file allowingReadAccessToURL:file.URLByDeletingLastPathComponent];
>+    [webView reload];
>+
>+    TestWebKitAPI::Util::run(&done);
>+}
>+
>+#endif // WK_API_ENABLED

Actions: View | Formatted Diff | Diff

Attachments on bug 188078: 345873 | 345882 | 345885