1254320 - Remove SEC_NORMAL from dom/json/

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1254320_remove_sec_normal_from_dom_json.patch

Boris, sooner or later we should not use SEC_NORMAL as a securityFlag anymore, but rather only one of the 5 flags defined here [1]. I honestly can't recall why we decided to use a nullPrincipal as the loadingPrincipal in the first place [2]. Should we rather use the systemPrincipal as the loadingPrincipal?

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsILoadInfo.idl#41
[2] http://hg.mozilla.org/mozilla-central/rev/7921e44f3544#l1.3

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8727628 [details] [diff] [review]
bug_1254320_remove_sec_normal_from_dom_json.patch

Let's back up a sec... why does this this code even use a channel?  I see absolutely no reason for it to do so.  Maybe it meant to work with other sorts of channels sometimes, but it doesn't right now.

So really, it totally doesn't matter what we pass here (e.g. we never open this channel!).  But it sure would be nice to just stop doing this insanity in general instead of continuing to wallpaper over it.

I'm not sure whether to mark this "r+, it just doesn't matter" or "r-, please make this code sane"...  I guess I'll go with the former, but please do consider the latter!  If not, at least make sure a followup is filed.

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] from comment #2)
> So really, it totally doesn't matter what we pass here (e.g. we never open
> this channel!).

Indeed, I forgot to mention that in my initial review request - we never open that channel, hence using the most restrictive securityflag is what we should use in case we keep that NewChannel code there.

It would definitely be nice to remove that code insanity and replace it with something nicer. If I don't have the time I'll definitely leave a comment in the code and file a follow up.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1254320_remove_sec_normal_from_dom_json.patch

Added a comment; carrying over r+ from bz.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/241d48f051ef

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/241d48f051ef