Handle system principals in nsPermissionManager when taking principals

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 643089 [details] [diff] [review]
Patch
Attachment #643089 - Flags: review?(jonas)
(Assignee)

Updated

5 years ago
Blocks: 769594
(Assignee)

Comment 1

5 years ago
Created attachment 643090 [details] [diff] [review]
Patch

Without compilation errors ;)
Attachment #643089 - Attachment is obsolete: true
Attachment #643089 - Flags: review?(jonas)
Attachment #643090 - Flags: review?(jonas)
(Assignee)

Comment 2

5 years ago
Created attachment 643091 [details] [diff] [review]
Patch

... with `hg qref` :)
Attachment #643090 - Attachment is obsolete: true
Attachment #643090 - Flags: review?(jonas)
Attachment #643091 - Flags: review?(jonas)
Comment on attachment 643091 [details] [diff] [review]
Patch

Review of attachment 643091 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm.. I feel like I commented on this one already but it might have gotten lost. I don't have connectivity right now so I can't check.

::: extensions/cookie/nsPermissionManager.cpp
@@ +514,5 @@
>  
> +  // We don't add the system principal because it actually has no URI and we
> +  // always allow action for them.
> +  if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
> +    return NS_OK;

Make this MOZ_ASSERT and return an error.

@@ +709,5 @@
>    NS_ENSURE_ARG_POINTER(aPrincipal);
>  
> +  // System principals are never added to the database, no need to remove them.
> +  if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
> +    return NS_OK;

Make this MOZ_ASSERT and return an error.

::: netwerk/base/public/nsIPermissionManager.idl
@@ +93,5 @@
>     * Add permission information for a given principal.
>     * It is internally calling the other add() method using the nsIURI from the
>     * principal.
> +   * Passing a system principal will be a no-op because they will always be
> +   * granted permissions.

Update this comment.

@@ +118,5 @@
>    /**
>     * Remove permission information for a given principal.
>     * This is internally calling remove() with the host from the principal's URI.
> +   * Passing system principal will be a no-op because we never add them to the
> +   * database.

Update this comment.
Attachment #643091 - Flags: review?(jonas) → review+
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla17
(Assignee)

Updated

5 years ago
Attachment #643091 - Flags: checkin+

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/d26ee2e87945
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.