300008 - chrome XBL method.eval allows arbitrary code execution

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[shutdown]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050707 Firefox/1.0+

ASAIK, this is a third problem that allows arbitrary code execution by using
chrome XBL. This time boundElement.chromeXblMethod.eval() allows it.

see also: bug 296397

Reproducible: Always

Steps to Reproduce:
1. load the testcase.
2. follow "invoke an exploit" link.

Actual Results:  
arbitrary code can be executed with elevated privileges.

Expected Results:  
deny privileged code execution.

Comment 1

[shutdown]

Attachment: testcase

arbitrary code execution testcase.

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050707 Firefox/1.0+
works

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.9) Gecko/20050707 Firefox/1.0.5
Error: function eval must be called directly, and not by way
of a function of another name.
Source File: file:///.../moz16.html Line: 23

Comment 2

[Boris Zbarsky [:bzbarsky]]

This regressed between 2005-07-05-07 and 2005-07-06-06.  Bonsai checkins:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-07-05+05%3A00%3A00&maxdate=2006-07-06+08%3A00%3A00&cvsroot=%2Fcvsroot

Comment 3

[Brendan Eich [:brendan]]

Regressed due to "fix" for bug 293933 (which didn't fix Venkman, heavy sigh).

I'm on it.

/be

Comment 4

[Brendan Eich [:brendan]]

The forthcoming patch should also fix bug 299797.

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: proposed fix, still needs testing

The JS engine has tried to treat JSPrincipals as opaque data (except for old
API glue that used the now-meaningless getPrincipalArray and
globalPrivilegesEnabled hooks in struct JSPrincipals).	For general security, I
had to change this so the so-called "belt and braces" error or scope-change
revisions made to eval and Script.prototype.exec native code could handle all
of these cases:

1.  Venkman chrome calling content window eval via __o.eval(__s) (bug 293933).

2.  Evil content stuffing eval in some shared DOM wrapper and tricking chrome
into calling it on a string content contrives to exploit the system (various
bugs dveditz may be able to cite).  This case is belt-for-braces since on the
trunk we have XPCNativeWrapper automation braces (bug 281988).

3.  This bug 300008, and any like it, where chrome XBL objects are exposed to
content script without any sandbox such as a window, which would check access. 
This is the hard case.

The patch introduces a notion of subsumption or partial order to principals. 
If two principals are equal, they subsume each other.  A stronger principal
such as the system principal subsumes itself and all weaker principals (except
for the not-yet-implemented null principal, described in comments in this patch
in caps/idl/nsIPrincipal.idl.  All non-system principals subsume only
themselves currently.

My tree is still rebuilding, so there may be a dumb bug here, but if others can
apply, build and test, that would help expedite getting this in.  We need to
test the above three cases, ideally by testing all the recent bugs reported on
problems in any of these cases.

/be

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 188615 [details] [diff] [review]
proposed fix, still needs testing

sr=shaver, but lose those vestigial members sooner rather than later!

Comment 7

[shutdown]

Attachment: testcase for more versions

This one works on all of:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050701 Firefox/1.0+
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050707 Firefox/1.0+
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.9) Gecko/20050707 Firefox/1.0.5

--- moz16.html			2005-07-08 05:47:46.000000000 +0900
+++ moz17.html			2005-07-08 12:33:38.000000000 +0900
@@ -20,7 +20,8 @@
 function invokeExploit() {
   var bound = document.getElementById("bound");
   alert(
-    bound.stop.eval("(function(C){return C.classes;})")(Components)
+    bound.stop.eval("(function(C){return C.classes;})", bound.stop.eval)
+      (Components)
   );
 }

Comment 8

[Brendan Eich [:brendan]]

Not sure caillon's around -- may need jst or dveditz to r=.

/be

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 188615 [details] [diff] [review]
proposed fix, still needs testing

Anyone tested, or is everyone waiting for respins?  I can't get the exploit in
the first attachment to work, but I get the spurious UnnamedClass error dveditz
got yesterday.

/be

Comment 10

[Daniel Veditz [:dveditz]]

r=dveditz, too, with a little testing. I'm not seeing the UnnamedClasses thing
on the trunk, that's from the first testcase against the branch I believe
(branch is the second testcase)

Comment 11

[Daniel Veditz [:dveditz]]

we don't need to wait for a respin and testing to get a branch version of this
patch.

Comment 12

[Mike Shaver (:shaver emeritus)]

Comment on attachment 188615 [details] [diff] [review]
proposed fix, still needs testing

a=shaver

Comment 13

[Brendan Eich [:brendan]]

Fixed on trunk.  Working on a branch patch.

/be

Comment 14

[Brendan Eich [:brendan]]

Attachment: AVIARY_1_0_1_20050124_BRANCH port of trunk patch

Should apply cleanly to a MOZILLA_1_7_BRANCH tree, which I don't have handy.

/be

Comment 15

[Jay Patel [:jay]]

Comment on attachment 188737 [details] [diff] [review]
AVIARY_1_0_1_20050124_BRANCH port of trunk patch

Let's get this checked in on the branches as well. a=jay

If everyone on this bug can pick up tomorrow mornings Aviary and 1.7 branches
and give this fix a good work out, that will be great.

Comment 16

[Brendan Eich [:brendan]]

Fixed on the branches.  My branch build testing shows that this patch fixes bug
299791, so I bet it also fixes bug 299797.  Venkman users, please confirm and
mark FIXED tomorrow if true.  I'm marking bug 299791 fixed now.

/be

Comment 17

[Darin Fisher]

Ugh.  Are we sure it is wise to change nsIPrincipal on our "stable" branch?  Why
not introduce an additional interface that consumers should QI to so that
existing code written against the 1.7 branch will not be broken by this change?
 If I had developed an extension or plugin that needed to access principals, I'd
be pretty ticked to find that the interface was changed like this.  We know from
our experiences prior to shipping 1.0 that people do use this interface :-(

Comment 18

[Brendan Eich [:brendan]]

(In reply to comment #17)
> Ugh.  Are we sure it is wise to change nsIPrincipal on our "stable" branch?  Why
> not introduce an additional interface that consumers should QI to so that
> existing code written against the 1.7 branch will not be broken by this change?

Because nsIPrincipal is not frozen, and that's enough for users to know that, if
they can't take the heat, they need to get out of the kitchen.  I mean it.

>  If I had developed an extension or plugin that needed to access principals, I'd
> be pretty ticked to find that the interface was changed like this.

Why?  Thanks to you and others, we have a clear frozen interface story.  If I
wanted a free lunch and a pony, I might be mad that I couldn't keep taking bites
and rides from someone else's (lunch|pony), but that's not how we do things
around here.

>  We know from
> our experiences prior to shipping 1.0 that people do use this interface :-(

What besides bug 293973 do you mean?  The Java plugin work mentioned there seems
able to cope with this backward-compatible (type-structurally speaking!) change.

IOW, where's the fire?  It's hot in that kitchen, but that's part of the job.

If we have to treat unfrozen interfaces as frozen, we should mark them frozen by
fiat and get on with eating the costs (giving the free lunch or ride).  We have
not done that.  I'm not going to make a mockery of the hard work done to freeze
other interfaces by doing that for nsIPrincipal right now.

/be

Comment 19

[Darin Fisher]

> Because nsIPrincipal is not frozen, and that's enough for users to know that, 
> if they can't take the heat, they need to get out of the kitchen.  I mean it.

That argument really only works if we actually provided a complete frozen API. 
We don't.  We are in a situation where in order to build anything interesting
with our platform, people must resort to using unfrozen APIs.  XUL comes to mind
in fact.  It's not wise to break interfaces on a stable branch especially when
stable alternatives to those interfaces do not exist.


> IOW, where's the fire?  It's hot in that kitchen, but that's part of the job.

I don't know where the fire is.  As you can imagine, we will find out when
someone complains to us that we killed their product.  Consider an extension or
plugin that may have been developed a year ago when we released 1.0.  The
company that produced it may no longer have the developers who wrote the
original code!  What are they to do?  Sucks to be them?  That's a crappy retort.


> If we have to treat unfrozen interfaces as frozen, we should mark them frozen 
> by fiat and get on with eating the costs (giving the free lunch or ride).  We 
> have not done that.  I'm not going to make a mockery of the hard work done to 
> freeze other interfaces by doing that for nsIPrincipal right now.

I'm not suggesting that we freeze nsIPrincipal.  Make all the changes you want
to the _trunk_, but changing interfaces on a stable branch is just a bad idea. 
Moreover, it is really easy in this case to avoid changing nsIPrincipal. 
(QueryInterface nsIPrincipal to some new interface that provides the "subsumes"
method.)  I just don't see why we need to take any risks here.

I think we all agree that it is imperative that we treat our extension community
with great care, and I think that not changing interfaces on our stable branches
is a big part of that.  So many things hinge on the fact that 1.0 = 1.0.x.  The
extension system is not designed to give extension authors the ability to say
that their extension only works with 1.0 - 1.0.4.  In fact, we have told people
that min/maxVersion of 1.0 in install.rdf means 1.0.x.  In other words, we have
said that interfaces do not change on the 1.0 branch.  We are shooting ourselves
in the foot by making interface changes on our stable branch.

Comment 20

[Daniel Veditz [:dveditz]]

Adding distributors

Comment 21

[Brendan Eich [:brendan]]

(In reply to comment #19)
> > Because nsIPrincipal is not frozen, and that's enough for users to know that, 
> > if they can't take the heat, they need to get out of the kitchen.  I mean it.
> 
> That argument really only works if we actually provided a complete frozen API.

There is no such thing.  I needn't haul in Goedel's proof here -- we will never
be complete and correct enough for some embedding, so we'll need nsIPrincipal2
or whatever.  I'm saying we don't need that now, while nsIPrincipal is unfrozen
and only one or two plugins depend on it.

It very much matters if ten hugely important consumers need utter compatibility
for an unfrozen API that we probably should have frozen.  But this ain't that
case.  I'm talking specifically about nsIPrincipal here.

If you are generalizing, not dealing with this case, then what is your rule for
not freezing an interface and allowing it to be changed (carefully, by adding at
the end and rev'ing the IID) on branches?

Apart from Firefox extensions, which have the version checking necessary to
automate disabling and notify about updates, XPCOM plugins that need unfrozen
interfaces are in the same hard place they've always been in.  Moving toward a
better world for XULRunner is fine, but claiming XPCOM APIs that aren't frozen,
suddenly are frozen, without marking them so and even enforcing that with CVS
commitinfo hacks, is not going to work.

/be

Comment 22

[Brendan Eich [:brendan]]

(In reply to comment #19)
> > IOW, where's the fire?  It's hot in that kitchen, but that's part of the job.
> 
> I don't know where the fire is.  As you can imagine, we will find out when
> someone complains to us that we killed their product.  Consider an extension or
> plugin that may have been developed a year ago when we released 1.0.  The
> company that produced it may no longer have the developers who wrote the
> original code!  What are they to do?  Sucks to be them?  That's a crappy retort.

We have to be able to say that, or we're their slave.  Since you don't mean
that, you must (again) mean that we should freeze all APIs that lack a frozen
counterpart, and that might be or are needed by any plugin or extension -- which
is to say, we must freeze all APIs.

If you disagree, please say so clearly.

If you agree, then certain things follow:

- We must mark all APIs @status FROZEN.

- We probably need to hack commitinfo to prevent substantive revisions, or just
use CVS locking, to prevent changes to any .idl files.

- We must stop trying to make competitive products and completing the platform,
instead starting to try to specify all the mess of now-frozen APIs that we were
not ready to freeze till now.  Some (most?) will be easy to swallow.  Some will
not.  Some may be impossible to support over time, and we may have to break
compatibility anyway.

- This affects everyone, not just you and me.  It needs to be discussed in a
wider forum than this resolved bug.

/be
> 
> 
> > If we have to treat unfrozen interfaces as frozen, we should mark them frozen 
> > by fiat and get on with eating the costs (giving the free lunch or ride).  We 
> > have not done that.  I'm not going to make a mockery of the hard work done to 
> > freeze other interfaces by doing that for nsIPrincipal right now.
> 
> I'm not suggesting that we freeze nsIPrincipal.  Make all the changes you want
> to the _trunk_, but changing interfaces on a stable branch is just a bad idea. 
> Moreover, it is really easy in this case to avoid changing nsIPrincipal. 
> (QueryInterface nsIPrincipal to some new interface that provides the "subsumes"
> method.)  I just don't see why we need to take any risks here.
> 
> I think we all agree that it is imperative that we treat our extension community
> with great care, and I think that not changing interfaces on our stable branches
> is a big part of that.  So many things hinge on the fact that 1.0 = 1.0.x.  The
> extension system is not designed to give extension authors the ability to say
> that their extension only works with 1.0 - 1.0.4.  In fact, we have told people
> that min/maxVersion of 1.0 in install.rdf means 1.0.x.  In other words, we have
> said that interfaces do not change on the 1.0 branch.  We are shooting ourselves
> in the foot by making interface changes on our stable branch.

Comment 23

[Brendan Eich [:brendan]]

Sorry, I hadn't finished replying.

(In reply to comment #19)
> I'm not suggesting that we freeze nsIPrincipal.  Make all the changes you want
> to the _trunk_, but changing interfaces on a stable branch is just a bad idea. 
> Moreover, it is really easy in this case to avoid changing nsIPrincipal. 
> (QueryInterface nsIPrincipal to some new interface that provides the
> "subsumes" method.)  I just don't see why we need to take any risks here.

You've just deferred the problem till 1.1, which is soon, a few months at most.

How can we ship the trunk nsIPrincipal under this proposal, since it is
different from the branch one?

> I think we all agree that it is imperative that we treat our extension 
> community with great care, and I think that not changing interfaces on our 
> stable branches is a big part of that.

I agree, and great care means discriminating between the hard cases that can
cope (that MRJ plugin) and the ones that can't.

> So many things hinge on the fact that 1.0 = 1.0.x.  The extension system 
> is not designed to give extension authors the ability to say that their
> extension only works with 1.0 - 1.0.4.

No XUL extension AFAIK uses nsIPrincipal.

Let's discriminate in the good sense.  What is at stake here?  Or if you are
generalizing to pull all IDL-declared unfrozen XPCOM interfaces into the toolkit
and Firefox extension model, then we need to talk more, and those consequences I
outlines (at least!) follow -- which means we stop doing other work, and suffer
the costs.

> In fact, we have told people
> that min/maxVersion of 1.0 in install.rdf means 1.0.x.  In other words, we 
> have said that interfaces do not change on the 1.0 branch.  We are shooting
> ourselves in the foot by making interface changes on our stable branch.

I never, ever signed up to be bound by install.rdf limitations in my branch *or*
trunk XPCOM IDL-declared interface revisions to *unscriptable*, *unfrozen*
interfaces such as nsIPrincipal.

Did anyone?  Did you?  Why does this make sense, in light of the alternatives
that are cheaper for everyone (including the one or two plugin vendors who need
caps interfaces)?  Where was the analysis of trade-offs and the buy-in from all
the CVS committers?  I'm not whooping this up to be mean, I seriously believe
that even if we mark all interfaces frozen, someone will change one and not know
to bump a version number.

What's the plan in this brave new world, other than saying every release is not
compatible with previous releases, or locking down all IDL with CVS locks?

/be

Comment 24

[Benjamin Smedberg]

Yes, the entire platform needs to become bound by the versioning system of
install.rdf, and yes, I believe that the mysterious collective "we" has made
that basic assumption for the 1.0.x branch, probably because we have *not*
bumped the extension version for each dot-release

Perhaps that system needs investigation for 1.1, and we should bump the
extension version with every dot-release, something I suggested when we took the
branch version of nativewrappers with 1.0.3. But you can't just wish this
assumption away and take the "we didn't say that" high ground when extension
authors have been playing by this set of rules in good faith.

The claim that extension/xulapp authors are "screwable" if they use unfrozen
APIs is laughable, which is why install.rdf versioning was introduced in the
first place.

FWIW, I am much less worried about the nsIPrincipal change than nsFileSpec or
some of the other changes that we have taken on the branch, but I think the
"it's not frozen, let's change it" methodology is a path of doom. It's not
frozen, let's be smart and give developers a good alternative.

Comment 25

[Brendan Eich [:brendan]]

(In reply to comment #24)
> Yes, the entire platform needs to become bound by the versioning system of
> install.rdf, and yes, I believe that the mysterious collective "we" has made
> that basic assumption for the 1.0.x branch, probably because we have *not*
> bumped the extension version for each dot-release

nsIPrincipal is not scriptable and not frozen.  No Firefox or other XUL app
extension written only in JS can use or implement it.  The plugins we know about
that care fit on one hand, and can cope.

> Perhaps that system needs investigation for 1.1, and we should bump the
> extension version with every dot-release, something I suggested when we took
> the branch version of nativewrappers with 1.0.3.

Why would that help?  That change was meant to be compatible.  If you want to
bump the incompatible version number on account of bugs, or fear of bugs, then
you should just bump on every release, including branch releases.

> But you can't just wish this
> assumption away and take the "we didn't say that" high ground when extension
> authors have been playing by this set of rules in good faith.

Anyone using nsIPrincipal and expecting it to be frozen was *not* playing by the
set of rules we've had since we started freezing XPCOM interfaces, and there's
no point in bringing up faith here.

> The claim that extension/xulapp authors are "screwable" if they use unfrozen
> APIs is laughable, which is why install.rdf versioning was introduced in the
> first place.

No, that versioning was invented because ben wisely knew Firefox's structural
XUL and so on (never mind "the new toolkit") would not be freeze-able long
before exensions would proliferate.  We counted on extension authors being
willing to revise to track all 1.x releases.  Agreed, changing 1.0.x
incompatibly for such extension authors is bad -- but this nsIPrincipal change
is not such a change!

Will someone please respond to the nsIPrincipal point and stop generalizing?

> FWIW, I am much less worried about the nsIPrincipal change than nsFileSpec or

Oh, thanks -- then can we move to another bug, or a newsgroup?  This bug is
where nsIPrincipal was being asserted frozen.  Maybe darin still wants to argue
that here.  If you don't, let's talk elsewhere.

> some of the other changes that we have taken on the branch, but I think the
> "it's not frozen, let's change it" methodology is a path of doom. It's not
> frozen, let's be smart and give developers a good alternative.

That sounds good in general, but everything has a cost, and there's no point in
worrying about trees falling in forests where no one can hear.  Generalizing and
arguing from those generalized new principles for arbitrarily high costs, and
platform-now-at-any-price efforts, on branches even, is just as wrong as arguing
across the board "screw you, you were warned".

/be

Comment 26

[Brendan Eich [:brendan]]

(In reply to comment #23)
> (In reply to comment #19)
> > I'm not suggesting that we freeze nsIPrincipal.  Make all the changes you want
> > to the _trunk_, but changing interfaces on a stable branch is just a bad idea. 
> > [...]
> 
> You've just deferred the problem till 1.1, which is soon, a few months at
> most.

Oh, I see -- you're simply saying it's ok for us to change nsIPrincipal for a
release that our extension version-tracking can distinguish (1.1 from 1.0x).

That's not good enough, by your own "people use it, don't break them" lights,
which is why I said you're only deferring the problem.

But it's also a mess for us.  Why should I diverge caps/src so much between
branch and trunk?  I'd just add nsIPrincipal2 to both and be done.  And perhaps
I should have, but I don't think so -- but if so, "we" (whoever royally decided
to put all unfrozen, even unscriptable XPCOM interfaces under the limitations of
install.rdf's version discrimination) needs to communicate a lot better and get
real, meaningful agreement.

Or this will go about as badly in the future, but with more versionitis.

/be

Comment 27

[Boris Zbarsky [:bzbarsky]]

Brendan, the key here is that the 1.7 branch is "API stable".  Whatever
Mozilla.org decides this to mean; up to now the general policy we've all been
following is to avoid changing an API unless absolutely forced to by a security
hole in it (see, eg, the changes that were made to the trunk docshell patches to
fix our frame injection vulnerabilities on branch).  I agree that this has some
cost, and that in cases when the cost is prohibitive we should think hard about
the issue.  But I don't believe the cost here is prohibitive.  Further, given
that callers who can't QI a principal to what they think is
NS_GET_IID(nsIPrincipal) suddenly have to deal with a null principal, which in
many cases is considered equivalent to system principal, I think just changing
the IID on an api-stable branch is asking for subtle security holes in any
"XPCOM plugins" (or embedding applications!) that do use nsIPrincipal.  And that
makes me rather worried.

Comment 28

[Brendan Eich [:brendan]]

It's too bad 1.0.5 shipped already (Firefox en-us anyway), so this change and
any like it (nsFileSpec/nsIFileSpec) couldn't be redone.

I don't know why 1.0.5 shipped so quickly.  I've been arguing openly that it
should wait till after 1.1a2, that it needs more time for testing, etc.  Anyone
who knew it was shipping before yesterday afternoon, please mail me with how you
knew, and when.

/be

Comment 29

[Brendan Eich [:brendan]]

In changing nsIPrincipal on the branch, I forgot all about bug 234169 -- shame
on me.

/be

Comment 30

[Brendan Eich [:brendan]]

Need an API-compatible branch patch.

/be

Comment 31

[Brendan Eich [:brendan]]

Attachment: API-compatible AVIARY_1_0_1_... branch patch

Need an approval1.7.10 flag too.

/be

Comment 32

[Chase Phillips]

Comment on attachment 189357 [details] [diff] [review]
API-compatible AVIARY_1_0_1_... branch patch

a=chase pending r+sr

Comment 33

[Darin Fisher]

Comment on attachment 189357 [details] [diff] [review]
API-compatible AVIARY_1_0_1_... branch patch

Chase asked me to review this patch...


>Index: idl/nsIPrincipal.idl

>+[uuid(a67c4736-9a95-4ce1-9ffc-3f88c0913a34)]
>+interface nsISubsumingPrincipal : nsIPrincipal
>+{

I presume that you are naming this nicely because you intend
to check the same patch in on the trunk, right?  I noticed
that nsIPrincipalObsolete only exists on the 1.7 / 1.0.1
branches.  It does not exist on the trunk.  Doesn't that
mean that principals have already changed on the trunk?
Maybe for the trunk editing nsIPrincipal would be fine.

NOTE: I'm not at all against having different interfaces in
this area on the trunk if it is desirable.  The interfaces
aren't frozen and we already have tons of unfrozen interface
changes in Gecko 1.8.



>Index: src/nsPrincipal.cpp

> NS_INTERFACE_MAP_BEGIN(nsPrincipal)
>+  NS_INTERFACE_MAP_ENTRY(nsISubsumingPrincipal)
>   NS_INTERFACE_MAP_ENTRY(nsIPrincipal)

nit: QI to nsIPrincipal is probably more common than QI to any
other, so list it first?  (same with nsSystemPrincipal)


r=darin

Comment 34

[Daniel Veditz [:dveditz]]

Comment on attachment 189357 [details] [diff] [review]
API-compatible AVIARY_1_0_1_... branch patch

sr=dveditz

Comment 35

[Brendan Eich [:brendan]]

(In reply to comment #33)
> I presume that you are naming this nicely because you intend
> to check the same patch in on the trunk, right?

Not sure, trying not to -- but shaver argued for a better name even if branch-only.

> I noticed
> that nsIPrincipalObsolete only exists on the 1.7 / 1.0.1
> branches.  It does not exist on the trunk.  Doesn't that
> mean that principals have already changed on the trunk?
> Maybe for the trunk editing nsIPrincipal would be fine.

We haven't yet felt the wrath from folks such as the asgardsw.com ones for
breaking compat in 1.1, but we felt it last October, and changed.  Is our only
hope that they'll feel differently this time that we have EM defense against
crashes, via auto-disabling?  That's not much better from their point of view.

I could see landing this on the trunk, but I haven't yet.

> >Index: src/nsPrincipal.cpp
> 
> > NS_INTERFACE_MAP_BEGIN(nsPrincipal)
> >+  NS_INTERFACE_MAP_ENTRY(nsISubsumingPrincipal)
> >   NS_INTERFACE_MAP_ENTRY(nsIPrincipal)
> 
> nit: QI to nsIPrincipal is probably more common than QI to any
> other, so list it first?  (same with nsSystemPrincipal)

Thanks, fixed.

Landed on the branches.  Still need fixed-aviary1.0.6, fixed1.7.10 keywords.

/be

Comment 36

[Boris Zbarsky [:bzbarsky]]

For what it's worth, there have been no nsIPrincipal changes on the trunk yet,
but I have some waiting for review and caillon wants to make some others because
currently nsIPrincipal actually exposes internal data that it really should not
be exposing...

Comment 37

[Brendan Eich [:brendan]]

(In reply to comment #36)
> For what it's worth, there have been no nsIPrincipal changes on the trunk yet,

Other than the UUID change and subsume extension I committed last Friday.

> but I have some waiting for review and caillon wants to make some others because
> currently nsIPrincipal actually exposes internal data that it really should not
> be exposing...

I think I'll leave the trunk be.

BTW, the original fix-patch for this bug was flawed; see bug 300867 if you can.

/be

Comment 38

[Brendan Eich [:brendan]]

Marking fixed.  On second thought, the API compatibility issue probably should
have been another bug.

Darin, bsmedberg: do you know who, if anyone, is evangelizing plugin vendors to
use EM-like versioning and packaging for Deer Park or 1.1 or whatever it'll be
called?

/be

Comment 39

[Bob Clary [:bc] (inactive)]

these tests are in the qa test farm.