1118534 - Update TYPE_OTHER comment in nsIContentPolicy.idl

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

Since we are using TYPE_OTHER whenever there is no appropriate (more descriptive) content policy type available when creating a new channel (calling NewChannel2), we should update the comment to make that clear.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1118534_update_type_other_comment.patch

Jonas, I am not sure what the best comment should look like, I am wide open to suggestions.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8544891 [details] [diff] [review]
bug_1118534_update_type_other_comment.patch

Tanvi, if you have any suggestions, let us know - thanks.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8544891 [details] [diff] [review]
bug_1118534_update_type_other_comment.patch

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

::: dom/base/nsIContentPolicy.idl
@@ +32,5 @@
>     */
>    const nsContentPolicyType TYPE_INVALID = 0;
>  
>    /**
> +   * Gecko/Firefox developers: Do not use TYPE_OTHER when calling content

I'd rather say:

"Gecko/Firefox developers: Avoid using TYPE_OTHER. Especially for requests that are coming from webpages. Or requests in general which you expect that security checks will be done on.

Always use a more specific type if one is available. And don't be shy about adding more types as appropriate.

But if you are fairly sure that no one would care about your more specific type, then it's ok to use TYPE_OTHER."

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1118534_update_type_other_comment.patch

Updating comments based on Jonas' suggestions.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8544915 [details] [diff] [review]
bug_1118534_update_type_other_comment.patch

Tanvi, you feedback is still appreciated.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1118534_update_type_other_comment.patch

As discussed in our weekly 'REVAMP' meeting, those comment updates should also mention that if a uri is coming from a webpage, we should never use the systemPrincipal as the loadingPrincipal.

I incorporated that change - carrying over r+ from Jonas.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/839a5e109b35

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/839a5e109b35