|Summary:||[iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations|
|Product:||WebKit||Reporter:||Andy Estes <aestes>|
|Component:||New Bugs||Assignee:||Andy Estes <aestes>|
|Severity:||Normal||CC:||achristensen, beidson, bfulgham, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, kangil.han, thorton|
|Version:||WebKit Nightly Build|
Description Andy Estes 2019-04-09 16:02:59 PDT
[iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations
Comment 2 Andy Estes 2019-04-09 16:13:09 PDT Comment hidden (obsolete)
Created attachment 367078 [details] Patch
Comment 3 EWS Watchlist 2019-04-09 18:25:33 PDT Comment hidden (obsolete)
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
Comment 4 EWS Watchlist 2019-04-09 18:25:44 PDT Comment hidden (obsolete)
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
Comment 6 Daniel Bates 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.
Comment 7 Andy Estes 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!
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2019-04-23 17:24:35 PDT
All reviewed patches have been landed. Closing bug.