From 2c953154936d41164c506f4ec7b76bc2a5c0d42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 12 Oct 2023 08:59:50 +0000 Subject: [PATCH] Bug 1855225 - Use system principal for chrome:// skin URIs too. r=dveditz Bug 1855151 has some example of confusion this causes, and bug 221490 has some more history. I don't see why chrome://foo/content should be different from chrome://foo/skin etc, in terms of privileges. I guess in the past this distinction probably made more sense? Per discussion in the review comments, we're not touching langpacks (yet). Differential Revision: https://phabricator.services.mozilla.com/D189232 --- chrome/nsChromeProtocolHandler.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/chrome/nsChromeProtocolHandler.cpp b/chrome/nsChromeProtocolHandler.cpp index ed926529fea5..b2e1e4d361de 100644 --- a/chrome/nsChromeProtocolHandler.cpp +++ b/chrome/nsChromeProtocolHandler.cpp @@ -142,12 +142,13 @@ nsChromeProtocolHandler::NewChannel(nsIURI* aURI, nsILoadInfo* aLoadInfo, rv = result->SetOriginalURI(aURI); if (NS_FAILED(rv)) return rv; - // Get a system principal for content files and set the owner - // property of the result - nsCOMPtr url = do_QueryInterface(aURI); + // Use a system principal for /content and /skin files. + // See bug 1855225 for discussion about whether to extend it more generally + // to other chrome:// URIs. nsAutoCString path; - rv = url->GetPathQueryRef(path); - if (StringBeginsWith(path, "/content/"_ns)) { + aURI->GetPathQueryRef(path); + if (StringBeginsWith(path, "/content/"_ns) || + StringBeginsWith(path, "/skin/"_ns)) { result->SetOwner(nsContentUtils::GetSystemPrincipal()); } @@ -158,8 +159,7 @@ nsChromeProtocolHandler::NewChannel(nsIURI* aURI, nsILoadInfo* aLoadInfo, // See bug 531886, bug 533038. result->SetContentCharset("UTF-8"_ns); - *aResult = result; - NS_ADDREF(*aResult); + result.forget(aResult); return NS_OK; }