299 bytes, text/html
8.39 KB, text/plain
When loading sandboxed without a loading principal, just create a NullPrincipal instead of asserting and misbehaving
2.91 KB, patch
|Details | Diff | Splinter Review|
Assertion failure: !aIsSandboxed, at content/base/src/nsContentUtils.cpp:6439 I think this is a regression from within the last day or so.
So we're ending up in nsContentUtils::SetUpChannelOwner with a null loading principal but with aIsSandboxed=true. If loading from a file:// URI, this happens because we end up in nsDocShell::DisplayLoadError which calls nsDocShell::LoadErrorPage, which calls InternalLoad with a null owner and the INTERNAL_LOAD_FLAGS_INHERIT_OWNER. But this URI does not inherit security context, so we just end up with null owner. I actually get the same thing loading from Bugzilla, but for a different reason: a failing CSP check loads about:blank via the string LoadURI overload, which likewise has no owner for the load. This is all obviously a problem for the "every load should have a principal that started it" idea. I could just create a nullprincipal in SetUpChannelOwner if aIsSandboxed and there is no principal passed in, I guess. That would at least make sure to propagate the sandbox flag along. Olli, any better idaes?
Assignee: nobody → bzbarsky
No better ideas. Forcing null principal sounds ok to me.
Christoph, do you mind if I just put up a patch for this, since we need it on 33? Note that this will likely bitrot your patches in bug 1038756 somewhat.... If you'd prefer, I could land this Aurora-33-only, but then we need a credible plan for making sure bug 1038756 lands on 34 and fixes this issue there.
(In reply to Boris Zbarsky [:bz] from comment #5) > Christoph, do you mind if I just put up a patch for this, since we need it > on 33? Note that this will likely bitrot your patches in bug 1038756 > somewhat.... > > If you'd prefer, I could land this Aurora-33-only, but then we need a > credible plan for making sure bug 1038756 lands on 34 and fixes this issue > there. Please go ahead and put one up. I can merge your changes into our patches - no big deal. Thanks!
[Tracking Requested - why for this release]: Regression from bug 965413, which landed in 33.
Target Milestone: --- → mozilla34
Comment on attachment 8460750 [details] [diff] [review] When loading sandboxed without a loading principal, just create a NullPrincipal instead of asserting and misbehaving Approval Request Comment [Feature/regressing bug #]: Bug 965413 [User impact if declined]: Some cases that should end up sandboxed won't be. [Describe test coverage new/current, TBPL]: Passes tbpl tests [Risks and why]: Low risk [String/UUID change made/needed]: None
Attachment #8460750 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Attachment #8460750 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reproduced with Nightly 33.0a1 debug build (Build ID: 20140710152348) - 'Assertion failure: !aIsSandboxed, at /builds/slave/m-cen-osx64-d-0000000000000000/build/content/base/src/nsContentUtils.cpp:6439' message is displayed in Terminal after loading the attached TC. Verified as fixed on latest Aurora 34.0a2 and Firefox 33 beta 2 (Build ID: 20140908152100) debug builds: no assertion encountered.
You need to log in before you can comment on or make changes to this bug.