in the file dom/base/DOMParser.cpp the DOMParser::Init function takes an optional principal as a parameter. if one is not passed in, it calls GetSimpleCodebasePrincipal which is wrong. Here's the code: > 344 rv = > 345 secMan->GetSimpleCodebasePrincipal(mDocumentURI, > 346 getter_AddRefs(mPrincipal)); i think the correct solution is to take the document pointer that is also passed in and ask its docshell parent what the correct user context id is. then we need to populate an origin attributes object form the mDocumentURI and then slam the user context ID to match the one we got from the docshell.

Jonas, I'd like to meet with you to talk about this one. It appears to be a chicken/egg problem that I think you can help straighten out for me.

I also asked some related question to Jonas in https://bugzilla.mozilla.org/show_bug.cgi?id=1259871#c12 it's also related to DOMparser as it's also in the commit made by him.

This just changes the assert the right place to try to catch any calls of DOMParser::Init without a principal. The latest try push is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=01bdfb09c5b9 I've been going through previous try pushes to make sure nothing calls this function without a valid principal. The idea is that if we always have a principal, then we no longer need to create one in ::Init and can change the invariants for the ::Init method.

Jonas and I walked through the code paths and in the case where we're creating a simple codebase principal with default origin attributes only happens in chrome scripted situations (e.g. add-ons). we don't want to break existing add-ons, but we need to start moving towards requiring a principal so that the add-ons and other chrome code is properly sand boxing any caching/requests from parsing a stream into a DOM.

are either of you still reviewing patches?

Here's the try push for the patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0036f02afe72

nvrmnd, found the right reviewer.

Comment on attachment 8760016 [details] [diff] [review] Bug_1237080.patch baku, you're listed as a peer on DOM and there is no other more specific module that owns the DOMParser. Will you please review this? Thanks! --dave

Comment on attachment 8760016 [details] [diff] [review] Bug_1237080.patch Review of attachment 8760016 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/DOMParser.cpp @@ +326,4 @@ > NS_ENSURE_ARG(principal || documentURI); > > mDocumentURI = documentURI; > remove this too. @@ +342,5 @@ > if (!mPrincipal) { > + // BUG 1237080 -- in this case we're getting a chrome privilege scripted > + // DOMParser object creation without an explicit principal set. This is > + // now deprecated. > + remove empty spaces.

already r+ by :baku, cleans up the review nitpicks.

Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/79fba69d9631 adds deprecation msg for chrome scripted DOMParser without principal. r=baku