Improve and Clean up some privileged-mozilla-content-process code
Categories
(Core :: DOM: Navigation, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox126 | --- | fixed |
People
(Reporter: tjr, Assigned: tjr)
References
Details
Attachments
(3 files, 2 obsolete files)
While working to get a resource:// URI loading in the privileged mozilla content process, I stumbled around and made the following observations I made patches for.
1: I think, but am not 100% certain, that the parameter name given in PageDataService is incorrect and being ignored. I haven't really tested the code, I just noticed this when my code, that I copied from there, was wrong. It's saying remoteType
when it should be remoteTypeOverride
. That said, it probably shouldn't be there at all (see (3 below).
2: Passing remoteTypeOverride
for anything other than an about: page is dislloawed by spec and enforced.
3: This surprised me, because I was fiddling with the E10sUtils code for privilegedmozilla, making it support resource:// urls in addition to https. My code was passing through this function (lazy.E10SUtils.getRemoteTypeForURIObject
to browser.loadURI
) and was failing in the aforementioned remoteType restriction.
It seems like getRemoteTypeForURI[Object]
is most commonly used for addTab
, createBrowser
although PageDataService is a notable exception. There is also HeadlessShell.sys.mjs which does the lazy.E10SUtils.getRemoteTypeForURIObject
to browser.loadURI
pattern and might be wrong.
I did see 30 hits for the return PRIVILEGEDMOZILLA_REMOTE_TYPE; line in E10SUtils which made me suspicious, but I believe that is browser_e10s_mozillaweb_process.js and browser_privilegedmozilla_process_pref.js which notably don't navigate the browser to those domains, they just check the expected value from parsing.
lazy.E10SUtils.getRemoteTypeForURIObject
feels like a footgun when it looks like it can be used with browser.loadURI
. Or possibly I just did a triple-gainer-to-slug-in-foot and it's not that common of a concern. But I wonder if there's an assertion we can put somewhere
I don't know if there is a path from E10SUtils.getRemoteTypeForURIObject
into loading a URI that doesn't path through browser.loadURI
- but at best it seems like this is a footgun. I'm going to experiment with putting an assertion at that remoteType check in DocumentLoadListener.
4: After figuring out that the code I put into E10SUtils wasn't doing what I wanted (well, it was returning the correct remoteType, but I couldn't feed it in where I expected), I found the code duplicated in ProcessIsolation.cpp and updated it there also.
5: Completely unrelated to the above, my first attempt was to implement process restrictions using remoteTypes: ["webIsolated=resource://path/file.html"]
when registering the actor. This didn't work, and it has the limitation that it will only work for Desktop when Fission is enabled, but I kind of think it should work? Or maybe I'm just writing a footgun for someone else on Android.
Assignee | ||
Comment 1•3 months ago
|
||
Firstly, this parameter is actually named remoteTypeOverride and
secondly, it is only valid for about: urls when passed to loadURI
Assignee | ||
Comment 2•3 months ago
|
||
Admittedly, this won't work on Android or non-Fission users
but perhaps it has a place for certain Desktop actors.
Assignee | ||
Comment 3•3 months ago
|
||
It will still disallow http, but this will permit it to load resource://
URIs in particular. The URI must still be present in the appropriate
pref.
Assignee | ||
Comment 4•3 months ago
|
||
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Assignee | ||
Comment 5•3 months ago
|
||
Updated•3 months ago
|
Pushed by tritter@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fa60e8a2dd07 Do not pass remoteType r=mossop https://hg.mozilla.org/integration/autoland/rev/0669743f23df Add a couple of pointer comments to duplicated code r=nika https://hg.mozilla.org/integration/autoland/rev/993eba2cdb4f Assert on a passed remoteTypeOverride to avoid footguns r=nika
Comment 7•3 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fa60e8a2dd07
https://hg.mozilla.org/mozilla-central/rev/0669743f23df
https://hg.mozilla.org/mozilla-central/rev/993eba2cdb4f
Description
•