[webkit-reviews] review denied: [Bug 79704] Restrict access to notifications for unique origins and file URLs with no local file access : [Attachment 132555] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Mar 19 00:57:31 PDT 2012
Adam Barth <abarth at webkit.org> has denied Jon Lee <jonlee at apple.com>'s request
for review:
Bug 79704: Restrict access to notifications for unique origins and file URLs
with no local file access
https://bugs.webkit.org/show_bug.cgi?id=79704
Attachment 132555: Patch
https://bugs.webkit.org/attachment.cgi?id=132555&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=132555&action=review
> Source/WebCore/notifications/NotificationCenter.cpp:62
> + if (scriptExecutionContext()->securityOrigin()->isLocal())
> + return NotificationClient::PermissionAllowed;
Why should local URLs be automatically whitelisted to have PermissionAllowed?
That sounds like a security decision that should either be made by
SecurityOrigin or the client (probably the client).
> Source/WebCore/notifications/NotificationCenter.cpp:70
> + if (!scriptExecutionContext()->securityOrigin()->canShowNotifications()
|| scriptExecutionContext()->securityOrigin()->isLocal()) {
We shouldn't be magically whitelisting isLocal.
> Source/WebCore/notifications/NotificationCenter.cpp:71
> + callback->handleEvent();
Does this mean we're calling the callback synchronously when before we called
it async?
> Source/WebCore/page/SecurityOrigin.cpp:360
> + return !isUnique() && (m_universalAccess ||
!m_enforceFilePathSeparation);
I'm not sure I understand this condition. Is this the same thing we use for,
e.g., localStorage?
> Source/WebCore/page/SecurityOrigin.h:117
> bool canAccessCookies() const { return !isUnique(); }
> bool canAccessPasswordManager() const { return !isUnique(); }
> bool canAccessFileSystem() const { return !isUnique(); }
Why is canShowNotifications different from the above? Should we change the
above? I think it make sense allow access to these things when
m_universalAccess is true, but I don't understand the connection with
m_enforceFilePathSeparation.
More information about the webkit-reviews
mailing list