[Cocoa] -loadFileURL:allowingReadAccessToURL: should fully resolve file URLs
authoraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Jul 2019 05:14:00 +0000 (05:14 +0000)
committeraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Jul 2019 05:14:00 +0000 (05:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199768
<rdar://problem/52002206>

Reviewed by Geoffrey Garen.

Source/WebKit:

-loadFileURL:allowingReadAccessToURL: used -_web_originalDataAsWTFString from WKNSURLExtras
to convert the file and read access NSURLs to strings, which under the hood calls
CFURLGetBytes(). CFURLGetBytes() gets the URL's string without considering the base URL, so
if the client creates a URL like this:

    NSURL *url = [NSURL fileURLWithPath:@"tmpfile.txt" relativeToURL:[NSURL fileURLWithPath:@"/tmp"]]

... then -_web_originalDataAsWTFString will merely return the string "tmpfile.txt". When
that is later converted back to a URL in WebPageProxy::loadFile(), we lose track of the base
component and refuse to load something that no longer looks like a file: URL.

Fixed this by fully resolving the URLs passed to -loadFileURL:allowingReadAccessToURL: when
converting to strings by using -[NSURL absoluteString] instead of -_web_originalDataAsWTFString.

* Shared/Cocoa/WKNSURLExtras.mm:
(-[NSURL _web_originalDataAsWTFString]):
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView loadFileURL:allowingReadAccessToURL:]):

Tools:

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

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

Source/WebKit/ChangeLog
Source/WebKit/Shared/Cocoa/WKNSURLExtras.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm

index 6111514..f66a667 100644 (file)
@@ -1,3 +1,30 @@
+2019-07-12  Andy Estes  <aestes@apple.com>
+
+        [Cocoa] -loadFileURL:allowingReadAccessToURL: should fully resolve file URLs
+        https://bugs.webkit.org/show_bug.cgi?id=199768
+        <rdar://problem/52002206>
+
+        Reviewed by Geoffrey Garen.
+
+        -loadFileURL:allowingReadAccessToURL: used -_web_originalDataAsWTFString from WKNSURLExtras
+        to convert the file and read access NSURLs to strings, which under the hood calls
+        CFURLGetBytes(). CFURLGetBytes() gets the URL's string without considering the base URL, so
+        if the client creates a URL like this:
+
+            NSURL *url = [NSURL fileURLWithPath:@"tmpfile.txt" relativeToURL:[NSURL fileURLWithPath:@"/tmp"]]
+
+        ... then -_web_originalDataAsWTFString will merely return the string "tmpfile.txt". When
+        that is later converted back to a URL in WebPageProxy::loadFile(), we lose track of the base
+        component and refuse to load something that no longer looks like a file: URL.
+
+        Fixed this by fully resolving the URLs passed to -loadFileURL:allowingReadAccessToURL: when
+        converting to strings by using -[NSURL absoluteString] instead of -_web_originalDataAsWTFString.
+
+        * Shared/Cocoa/WKNSURLExtras.mm:
+        (-[NSURL _web_originalDataAsWTFString]):
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView loadFileURL:allowingReadAccessToURL:]):
+
 2019-07-12  Megan Gardner  <megan_gardner@apple.com>
 
         Turn off two finger gestures for editable non-scaled content
index 519b9d8..260271a 100644 (file)
@@ -47,6 +47,7 @@
 
 - (String)_web_originalDataAsWTFString
 {
+    // FIXME: Why is it OK to ignore base URL here?
     CString originalData;
     WTF::getURLBytes((__bridge CFURLRef)self, originalData);
     return String::fromUTF8(originalData);
index 93629fe..3255f43 100644 (file)
@@ -952,7 +952,7 @@ static void validate(WKWebViewConfiguration *configuration)
     if (![readAccessURL isFileURL])
         [NSException raise:NSInvalidArgumentException format:@"%@ is not a file URL", readAccessURL];
 
-    return wrapper(_page->loadFile([URL _web_originalDataAsWTFString], [readAccessURL _web_originalDataAsWTFString]));
+    return wrapper(_page->loadFile(URL.absoluteString, readAccessURL.absoluteString));
 }
 
 - (WKNavigation *)loadHTMLString:(NSString *)string baseURL:(NSURL *)baseURL
index 32eaf2d..6b6f8a0 100644 (file)
@@ -1,3 +1,14 @@
+2019-07-12  Andy Estes  <aestes@apple.com>
+
+        [Cocoa] -loadFileURL:allowingReadAccessToURL: should fully resolve file URLs
+        https://bugs.webkit.org/show_bug.cgi?id=199768
+        <rdar://problem/52002206>
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm:
+        (TEST):
+
 2019-07-12  Aakash Jain  <aakash_jain@apple.com>
 
         [ews-build] Remove wincairo queue from old EWS and dashboard
index dd1fdc3..64a33ce 100644 (file)
@@ -66,3 +66,18 @@ TEST(WKWebView, LoadTwoFiles)
     [delegate waitForDidFinishNavigation];
     EXPECT_WK_STREQ(webView.get()._resourceDirectoryURL.path, file.URLByDeletingLastPathComponent.path);
 }
+
+TEST(WKWebView, LoadRelativeFileURL)
+{
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+    
+    auto delegate = adoptNS([[TestNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+    
+    NSString *path = [NSBundle.mainBundle pathForResource:@"simple" ofType:@"html" inDirectory:@"TestWebKitAPI.resources"];
+    NSURL *baseURL = [NSURL fileURLWithPath:path.stringByDeletingLastPathComponent];
+    NSURL *fileURL = [NSURL fileURLWithPath:path.lastPathComponent relativeToURL:baseURL];
+    EXPECT_NOT_NULL([webView loadFileURL:fileURL allowingReadAccessToURL:fileURL.URLByDeletingLastPathComponent]);
+    [delegate waitForDidFinishNavigation];
+    EXPECT_WK_STREQ([webView _resourceDirectoryURL].path, fileURL.URLByDeletingLastPathComponent.path);
+}