Closed Bug 1118534 Opened 5 years ago Closed 5 years ago

Update TYPE_OTHER comment in nsIContentPolicy.idl

Categories

(Core :: DOM: Security, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: ckerschb, Assigned: ckerschb)

Details

Attachments

(1 file, 2 obsolete files)

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.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Jonas, I am not sure what the best comment should look like, I am wide open to suggestions.
Attachment #8544891 - Flags: review?(jonas)
Comment on attachment 8544891 [details] [diff] [review]
bug_1118534_update_type_other_comment.patch

Tanvi, if you have any suggestions, let us know - thanks.
Attachment #8544891 - Flags: feedback?(tanvi)
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."
Attachment #8544891 - Flags: review?(jonas) → review+
Updating comments based on Jonas' suggestions.
Attachment #8544891 - Attachment is obsolete: true
Attachment #8544891 - Flags: feedback?(tanvi)
Attachment #8544915 - Flags: review+
Comment on attachment 8544915 [details] [diff] [review]
bug_1118534_update_type_other_comment.patch

Tanvi, you feedback is still appreciated.
Attachment #8544915 - Flags: feedback?(tanvi)
Attachment #8544915 - Flags: feedback?(tanvi) → feedback+
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.
Attachment #8544915 - Attachment is obsolete: true
Attachment #8546033 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/839a5e109b35
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.