Closed
Bug 1407258
Opened 7 years ago
Closed 7 years ago
Crash in nsNodeInfoManager::SetDocumentPrincipal
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | fixed |
People
(Reporter: calixte, Assigned: kmag)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
This bug was filed from the Socorro interface and is report bp-b5fa0ee1-ae01-4924-9143-be4110171010. ============================================================= There are 50 crashes in nightly 58 with buildid 20171010100200. They're probably related to fix for bug 1391693. :bkelly, could you investigate please ?
Flags: needinfo?(bkelly)
Comment 1•7 years ago
|
||
I don't think this is related to bug 1391693. That bug changed code in the parent process, but this crash is clearly happening in the child process. While e10s does also use the parent process channel, we don't use it to provide data like this stack shows. I think this is a different problem. Also, this stack is using nsBaseChannel which HttpChannelChild and InterceptedHttpChannel don't use.
Flags: needinfo?(bkelly)
Comment 2•7 years ago
|
||
A bunch of principal changes in bug 1406278.
Comment 3•7 years ago
|
||
Kris, can you take a look at this? It seems we're getting an expanded principal someplace we should not. Could this be fallout from bug 1406278?
Flags: needinfo?(kmaglione+bmo)
Updated•7 years ago
|
Component: DOM: Service Workers → DOM
Assignee | ||
Comment 5•7 years ago
|
||
I guess we're loading a URL that inherits principal from a content script? I thought we already refused to inherit expanded principals that way...
Assignee: nobody → kmaglione+bmo
Flags: needinfo?(kmaglione+bmo)
Comment 6•7 years ago
|
||
Since calixte filed this there are now over 1129 crashes/239 installs.
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8917152 [details] Bug 1407258: Dowgrade document loads with expanded principals to their last sub-principal. https://reviewboard.mozilla.org/r/188166/#review193320 Please make sure a followup for this is filed and referenced in the code comments or commit message. ::: dom/base/nsDocument.cpp:2497 (Diff revision 1) > + // extension principal, in the case of extension content scripts). > + auto* basePrin = BasePrincipal::Cast(aPrincipal); > + if (basePrin->Is<ExpandedPrincipal>()) { > + auto* expanded = basePrin->As<ExpandedPrincipal>(); > + > + nsTArray<nsCOMPtr<nsIPrincipal>>* whitelist; Ugh. Why does GetWhiteList not return a pointer-to-const-array??? I guess we have the one consumer that wants to do edit it; we should have a GetMutableWhiteList() for that or something. Because I was going to suggest using .forget(), and it would have totally "worked"... Could you file a followup on this?
Attachment #8917152 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8917152 [details] Bug 1407258: Dowgrade document loads with expanded principals to their last sub-principal. https://reviewboard.mozilla.org/r/188166/#review193320 > Ugh. Why does GetWhiteList not return a pointer-to-const-array??? I guess we have the one consumer that wants to do edit it; we should have a GetMutableWhiteList() for that or something. Because I was going to suggest using .forget(), and it would have totally "worked"... > > Could you file a followup on this? I think it's just because it's an XPIDL-defined noscript attribute, and that's the signature XPIDL generates. Part of the documented contract is that it's not allowed to be changed, so it's not mutable on purpose. I actually seriously considered changing the signature to `const nsTArray<nsCOMPtr<nsIPrincipal>>& WhiteList()`, but this didn't seem to be the place to do it. I'll file a follow-up, though.
Updated•7 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3aad26ddfedf4f2765b121d0aa9032b55c01076c Bug 1407258: Dowgrade document loads with expanded principals to their last sub-principal. r=bz
Comment 12•7 years ago
|
||
> I think it's just because it's an XPIDL-defined noscript attribute I'm talking about the actual argument type, not the function itself being const. > Part of the documented contract is that it's not allowed to be changed Ah, I misread PrincipalToPrincipalInfo. In that case, we should definitely only hand out const refs or ptrs to it. And yes, followup bug.
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3aad26ddfedf
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•