STR: 1. Update to 1e7eb0625d3e and build. 2. Switch <https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7eb0625d3e#l8.20> and <https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7eb0625d3e#l8.36> to call Cu.getObjectPrincipal(). getObjectPrincipal() gets the principal off of the compartment, but getSandboxPrincipal() in the patch gets it off the SandboxPrivate object, and for some reason they seem to be different. I have not investigated why. (Follow-up from bug 1297687.)

I'm in meetings all week and then on PTO. Gabor, do you have a minute to repro the steps and see if there's anything crazy going on we should be worried about?

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #1) > I'm in meetings all week and then on PTO. Gabor, do you have a minute to > repro the steps and see if there's anything crazy going on we should be > worried about? Sure, I'll do this today.

By the way, at first glance I think the patch has a bug. For IPC we must take care of serializing this extra bit of added info to nsEPs, no? http://searchfox.org/mozilla-central/source/caps/nsJSPrincipals.cpp#261

(In reply to :Ehsan Akhgari from comment #0) > STR: > > 1. Update to 1e7eb0625d3e and build. > 2. Switch > <https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7eb0625d3e#l8.20> > and > <https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7eb0625d3e#l8.36> > to call Cu.getObjectPrincipal(). > > getObjectPrincipal() gets the principal off of the compartment, but > getSandboxPrincipal() in the patch gets it off the SandboxPrivate object, > and for some reason they seem to be different. I have not investigated why. > > (Follow-up from bug 1297687.) Sorry but I could not reproduce this. The tests pass with both Cu.getObjectPrincipal and getSandboxPrincipal. I've added an 'is' check to compare the two principals they return and that returns true as well, so they seem to return the same object. Are you sure you didn't have any other modification in your copy of 1e7eb0625d3e ?

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #3) > By the way, at first glance I think the patch has a bug. For IPC we must > take care of serializing this extra bit of added info to nsEPs, no? > > http://searchfox.org/mozilla-central/source/caps/nsJSPrincipals.cpp#261 Yeah looks like you're right! But it's OK, the patch has been backed out already. (In reply to Gabor Krizsanits [:krizsa :gabor] from comment #4) > (In reply to :Ehsan Akhgari from comment #0) > > STR: > > > > 1. Update to 1e7eb0625d3e and build. > > 2. Switch > > <https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7eb0625d3e#l8.20> > > and > > <https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7eb0625d3e#l8.36> > > to call Cu.getObjectPrincipal(). > > > > getObjectPrincipal() gets the principal off of the compartment, but > > getSandboxPrincipal() in the patch gets it off the SandboxPrivate object, > > and for some reason they seem to be different. I have not investigated why. > > > > (Follow-up from bug 1297687.) > > Sorry but I could not reproduce this. The tests pass with both > Cu.getObjectPrincipal and > getSandboxPrincipal. I've added an 'is' check to compare the two principals > they return and > that returns true as well, so they seem to return the same object. Are you > sure you didn't > have any other modification in your copy of 1e7eb0625d3e ? Yeah, quite certain. Did you test in both e10s and non-e10s mode? I don't remember where I saw the bug...

(In reply to :Ehsan Akhgari from comment #5) > Yeah, quite certain. Did you test in both e10s and non-e10s mode? I don't > remember where I saw the bug... I just did, but I get the same result for non-e10s mode as well.

OK, I don't have anything more to add then, sorry! Unfortunately I'm dealing with a lot of more important things, so I won't have time to try to reproduce any time soon. (I have already purged the build from last Friday.)

Assuming Ehsan doesn't have concrete plans to try to reproduce this, I'm going to mark this WFM given gabor's attempt to reproduce and my general skepticism that this could ever happen.