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> | ||||||||
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
Andy Estes
2019-04-09 16:02:59 PDT
Created attachment 367078 [details]
Patch
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 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
Created attachment 367288 [details]
Patch
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. (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 on attachment 367288 [details] Patch Clearing flags on attachment: 367288 Committed r244573: <https://trac.webkit.org/changeset/244573> All reviewed patches have been landed. Closing bug. |