Bug 196749

Summary: [iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations
Product: WebKit Reporter: Andy Estes <aestes>
Component: New BugsAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, bfulgham, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, kangil.han, thorton
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews202 for win-future
none
Patch none

Andy Estes
Reported 2019-04-09 16:02:59 PDT
[iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations
Attachments
Patch (17.55 KB, patch)
2019-04-09 16:13 PDT, Andy Estes
no flags
Archive of layout-test-results from ews202 for win-future (12.88 MB, application/zip)
2019-04-09 18:25 PDT, EWS Watchlist
no flags
Patch (17.46 KB, patch)
2019-04-11 21:50 PDT, Andy Estes
no flags
Andy Estes
Comment 1 2019-04-09 16:03:12 PDT
Andy Estes
Comment 2 2019-04-09 16:13:09 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 3 2019-04-09 18:25:33 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 4 2019-04-09 18:25:44 PDT Comment hidden (obsolete)
Andy Estes
Comment 5 2019-04-11 21:50:28 PDT
Daniel Bates
Comment 6 2019-04-22 10:19:24 PDT
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
Comment 7 2019-04-23 17:02:36 PDT
(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 8 2019-04-23 17:24:33 PDT
Comment on attachment 367288 [details] Patch Clearing flags on attachment: 367288 Committed r244573: <https://trac.webkit.org/changeset/244573>
WebKit Commit Bot
Comment 9 2019-04-23 17:24:35 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.