Bug 196749 – [iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andy Estes]

[iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations

[Andy Estes]

rdar://problem/35773454

[Andy Estes]

Created attachment 367078 [details]
Patch

[EWS Watchlist]

Comment on attachment 367078 [details]
Patch

Attachment 367078 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11823867

New failing tests:
http/tests/preload/download_resources_from_header_iframe.html

[EWS Watchlist]

Created attachment 367092 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Andy Estes]

Created attachment 367288 [details]
Patch

[Daniel Bates]

Comment on attachment 367288 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367288&action=review

> Source/WebCore/page/SecurityOrigin.cpp:378
> +    if (url.isLocalFile() && url.fileSystemPath() == m_filePath)
> +        return true;
> +

This is ok as-is. It's a hack of the SecurityOrigin design 😕. I think what we want is a new concept for fine-grained file system permission for what path(s) a Security Origin can access. You're running into the the all-or-nothing design (<--- what grantLoadLocalResources() is controlling) we have now and hacking it to support a single non-file URL that maps to a single file path. Works for QuickLook and I think that's all we need to care about (though depends on the impl detail that QuickLook can load multiple? files from the same file path, right? – we're only equal matching one path, not even checking if url.fileSystemPath() is a sub-directory or file under the "dirname(m_filePath)"). 

Maybe all this concept is semi-related if not entirely SecurityPolicy. Something about this code feels weird, but seems like it will work for now. Might want to take a look at SecurityPolicy.

[Andy Estes]

(In reply to Daniel Bates from comment #6)
> though depends on the impl detail that QuickLook can load multiple? files from the same file path, right?

Right. QuickLook should never generate a preview document that loads other file: URLs. It should only generate a preview document that loads x-apple-ql-id: URLs.

> Maybe all this concept is semi-related if not entirely SecurityPolicy. Something about this code feels weird, but seems like it will work for now. Might want to take a look at SecurityPolicy.

FWIW, I did look at solving this with a combination of SecurityOriginPolicy and SecurityPolicy's origin access whitelist, but decided against it because it ended up being significantly more complicated for something that only QuickLook seems to need. I'm open to other designs, though.

Thanks for reviewing!

[WebKit Commit Bot]

Comment on attachment 367288 [details]
Patch

Clearing flags on attachment: 367288

Committed r244573: <https://trac.webkit.org/changeset/244573>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.