[webkit-reviews] review granted: [Bug 196749] [iOS] QuickLook documents loaded from file: URLs should be allowed to perform same-document navigations : [Attachment 367288] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 196749: [iOS] QuickLook documents loaded from file: URLs should be allowed
to perform same-document navigations
https://bugs.webkit.org/show_bug.cgi?id=196749

Attachment 367288: Patch

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




--- Comment #6 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 367288
  --> https://bugs.webkit.org/attachment.cgi?id=367288
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.