619800 - Enable scriptability for nsIPrincipal methods

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Asaf Romano (gone)]

Every once in a while I'm looking at the ever growing XXXmano list.  Today I found this bit in urlSecurityCheck (see contentAreaUtils):

76 function urlSecurityCheck(aURL, aPrincipal, aFlags)
...
90   } catch (e) {
91     // XXXmano: dump the principal url here too
92     throw "Load of " + aURL + " denied.";
93   }

But nsIPrincipal::URI is marked [noscript]. Any reason for that?

Comment 1

[Asaf Romano (gone)]

Oh, I just saw bug 327242 comment 3.  I guess we could add a new property.

Comment 2

[Boris Zbarsky [:bzbarsky]]

That comment predates bug 339822 being fixed.  At this point there's no reason to not make URI scriptable on principals, imo.

Comment 3

[Asaf Romano (gone)]

Does this require anything other than removing the [noscript] bit in the interface? We should probably enable scriptability of few more nsIPrincipal methods and attributes while we are at it.  For example, I cannot see a reason for subsumes not being scriptable.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Does this require anything other than removing the [noscript] bit

Nope.

> For example, I cannot see a reason for subsumes not being scriptable.

Agreed.

Comment 5

[Asaf Romano (gone)]

Attachment: Step

Still not scriptable:
 * The capacity methods - mostly because it seems they need some polish.
 * securityPolicy - Could we provide an interface here?
 * domain - because it's settable
 * hashValue - seems useless for JS.
 * getJSPrincipals

It would be nice to have this for 2.0, but it worries me that I cannot think of potential issues...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 498767 [details] [diff] [review]
Step

|csp| is also settable.  I don't think we should make it scriptable without some separate review from the csp folks.

The rest looks fine to me.

r=me with the above caveat.

Comment 7

[Asaf Romano (gone)]

Comment on attachment 498767 [details] [diff] [review]
Step

With bz's comment's fixed (that is, remove the last hunk of this patch), I'd like to get this for 2.0.  Drivers: This doesn't affect c++ usage, nor it could have any affect on any current js usage of principals.

Comment 8

[Asaf Romano (gone)]

Thanks Boris!

http://hg.mozilla.org/mozilla-central/rev/1cb5ff9e32f6

Comment 9

[Eric Shepherd [:sheppy]]

nsIPrincipal had not previously been documented; there is now a document, albeit incomplete, but this change is documented there. If anyone would like to fill out any of the details I don't know about nsIPrincipal, please do. Otherwise I'll be researching this and working on it as I can.

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIPrincipal

This is also properly mentioned on Firefox 4 for developers.