From 163be910bbd3eda281b3d7ae18e5c6269021aa9e Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Fri, 25 Aug 2017 16:44:40 +0200 Subject: [PATCH] Backed out changeset 05fc8d2d7ca9 (bug 863246) for failing various reftests, e.g. parser/htmlparser/tests/reftest/bug535530-2.html. r=backout on a CLOSED TREE --- browser/extensions/activity-stream/jar.mn | 2 +- browser/extensions/onboarding/jar.mn | 4 +- .../extensions/shield-recipe-client/jar.mn | 4 +- caps/nsScriptSecurityManager.cpp | 54 ++++--------------- chrome/RegistryMessageUtils.h | 10 +--- chrome/nsChromeRegistryChrome.cpp | 10 +--- chrome/nsChromeRegistryContent.cpp | 2 +- .../res/SubstitutingProtocolHandler.cpp | 53 ++++-------------- .../res/SubstitutingProtocolHandler.h | 26 ++------- .../protocol/res/nsIResProtocolHandler.idl | 1 - .../res/nsISubstitutingProtocolHandler.idl | 10 ---- netwerk/protocol/res/nsResProtocolHandler.cpp | 37 ++----------- netwerk/protocol/res/nsResProtocolHandler.h | 3 +- xpcom/components/ManifestParser.cpp | 4 +- 14 files changed, 37 insertions(+), 183 deletions(-) diff --git a/browser/extensions/activity-stream/jar.mn b/browser/extensions/activity-stream/jar.mn index 5c2856941733..2dea506abf4d 100644 --- a/browser/extensions/activity-stream/jar.mn +++ b/browser/extensions/activity-stream/jar.mn @@ -3,7 +3,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. [features/activity-stream@mozilla.org] chrome.jar: -% resource activity-stream %content/ contentaccessible=yes +% resource activity-stream %content/ content/lib/ (./lib/*) content/common/ (./common/*) content/vendor/Redux.jsm (./vendor/Redux.jsm) diff --git a/browser/extensions/onboarding/jar.mn b/browser/extensions/onboarding/jar.mn index 1e08dec5b04a..37e046b8b0a3 100644 --- a/browser/extensions/onboarding/jar.mn +++ b/browser/extensions/onboarding/jar.mn @@ -3,9 +3,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. [features/onboarding@mozilla.org] chrome.jar: - # resource://onboarding/ is referenced in about:home and about:newtab, - # so make it content-accessible. -% resource onboarding %content/ contentaccessible=yes +% resource onboarding %content/ content/ (content/*) # Package UITour-lib.js in here rather than under # /browser/components/uitour to avoid "unreferenced files" error when diff --git a/browser/extensions/shield-recipe-client/jar.mn b/browser/extensions/shield-recipe-client/jar.mn index d723c4836a04..926cde00b35c 100644 --- a/browser/extensions/shield-recipe-client/jar.mn +++ b/browser/extensions/shield-recipe-client/jar.mn @@ -7,7 +7,7 @@ lib/ (./lib/*) data/ (./data/*) skin/ (skin/*) -% resource shield-recipe-client-content %content/ contentaccessible=yes +% resource shield-recipe-client-content %content/ content/ (./content/*) -% resource shield-recipe-client-vendor %vendor/ contentaccessible=yes +% resource shield-recipe-client-vendor %vendor/ vendor/ (./vendor/*) diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp index 4b797faf1eb3..5dcb7f7ec424 100644 --- a/caps/nsScriptSecurityManager.cpp +++ b/caps/nsScriptSecurityManager.cpp @@ -53,7 +53,6 @@ #include "nsIURIFixup.h" #include "nsCDefaultURIFixup.h" #include "nsIChromeRegistry.h" -#include "nsIResProtocolHandler.h" #include "nsIContentSecurityPolicy.h" #include "nsIAsyncVerifyRedirectCallback.h" #include "mozilla/Preferences.h" @@ -916,9 +915,10 @@ nsScriptSecurityManager::CheckLoadURIFlags(nsIURI *aSourceURI, NS_ENSURE_SUCCESS(rv, rv); if (hasFlags) { if (aFlags & nsIScriptSecurityManager::ALLOW_CHROME) { - // For now, don't change behavior for moz-icon:// and just allow it. - if (!targetScheme.EqualsLiteral("chrome") - && !targetScheme.EqualsLiteral("resource")) { + + // For now, don't change behavior for resource:// or moz-icon:// and + // just allow them. + if (!targetScheme.EqualsLiteral("chrome")) { return NS_OK; } @@ -939,51 +939,15 @@ nsScriptSecurityManager::CheckLoadURIFlags(nsIURI *aSourceURI, return NS_OK; } - if (targetScheme.EqualsLiteral("resource")) { - // Mochitests that need to load resource:// URIs not declared - // content-accessible in manifests should set the preference - // "security.all_resource_uri_content_accessible" true. - static bool sSecurityPrefCached = false; - static bool sAllResourceUriContentAccessible = false; - if (!sSecurityPrefCached) { - sSecurityPrefCached = true; - Preferences::AddBoolVarCache( - &sAllResourceUriContentAccessible, - "security.all_resource_uri_content_accessible", - false); - } - if (sAllResourceUriContentAccessible) { - return NS_OK; - } - - nsCOMPtr ph; - rv = sIOService->GetProtocolHandler("resource", getter_AddRefs(ph)); - NS_ENSURE_SUCCESS(rv, rv); - if (!ph) { - return NS_ERROR_DOM_BAD_URI; - } - - nsCOMPtr rph = do_QueryInterface(ph); - if (!rph) { - return NS_ERROR_DOM_BAD_URI; - } - + // Allow the load only if the chrome package is whitelisted. + nsCOMPtr reg(do_GetService( + NS_CHROMEREGISTRY_CONTRACTID)); + if (reg) { bool accessAllowed = false; - rph->AllowContentToAccess(aTargetBaseURI, &accessAllowed); + reg->AllowContentToAccess(aTargetBaseURI, &accessAllowed); if (accessAllowed) { return NS_OK; } - } else { - // Allow the load only if the chrome package is whitelisted. - nsCOMPtr reg( - do_GetService(NS_CHROMEREGISTRY_CONTRACTID)); - if (reg) { - bool accessAllowed = false; - reg->AllowContentToAccess(aTargetBaseURI, &accessAllowed); - if (accessAllowed) { - return NS_OK; - } - } } } diff --git a/chrome/RegistryMessageUtils.h b/chrome/RegistryMessageUtils.h index d8cfa616852a..cb9db55b7ccd 100644 --- a/chrome/RegistryMessageUtils.h +++ b/chrome/RegistryMessageUtils.h @@ -42,14 +42,12 @@ struct SubstitutionMapping nsCString scheme; nsCString path; SerializedURI resolvedURI; - uint32_t flags; bool operator ==(const SubstitutionMapping& rhs) const { return scheme.Equals(rhs.scheme) && path.Equals(rhs.path) && - resolvedURI == rhs.resolvedURI && - flags == rhs.flags; + resolvedURI == rhs.resolvedURI; } }; @@ -142,23 +140,19 @@ struct ParamTraits WriteParam(aMsg, aParam.scheme); WriteParam(aMsg, aParam.path); WriteParam(aMsg, aParam.resolvedURI); - WriteParam(aMsg, aParam.flags); } static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) { nsCString scheme, path; SerializedURI resolvedURI; - uint32_t flags; if (ReadParam(aMsg, aIter, &scheme) && ReadParam(aMsg, aIter, &path) && - ReadParam(aMsg, aIter, &resolvedURI) && - ReadParam(aMsg, aIter, &flags)) { + ReadParam(aMsg, aIter, &resolvedURI)) { aResult->scheme = scheme; aResult->path = path; aResult->resolvedURI = resolvedURI; - aResult->flags = flags; return true; } return false; diff --git a/chrome/nsChromeRegistryChrome.cpp b/chrome/nsChromeRegistryChrome.cpp index ae10e65bd381..8df9079d1cbd 100644 --- a/chrome/nsChromeRegistryChrome.cpp +++ b/chrome/nsChromeRegistryChrome.cpp @@ -927,15 +927,7 @@ nsChromeRegistryChrome::ManifestResource(ManifestProcessingContext& cx, int line return; } - // By default, Firefox resources are not content-accessible unless the - // manifests opts in. - bool contentAccessible = (flags & nsChromeRegistry::CONTENT_ACCESSIBLE); - - uint32_t substitutionFlags = 0; - if (contentAccessible) { - substitutionFlags |= nsIResProtocolHandler::ALLOW_CONTENT_ACCESS; - } - rv = rph->SetSubstitutionWithFlags(host, resolved, substitutionFlags); + rv = rph->SetSubstitution(host, resolved); if (NS_FAILED(rv)) { LogMessageWithContext(cx.GetManifestURI(), lineno, nsIScriptError::warningFlag, "Warning: cannot set substitution for '%s'.", diff --git a/chrome/nsChromeRegistryContent.cpp b/chrome/nsChromeRegistryContent.cpp index 75141204d0b5..a066f32f13d2 100644 --- a/chrome/nsChromeRegistryContent.cpp +++ b/chrome/nsChromeRegistryContent.cpp @@ -114,7 +114,7 @@ nsChromeRegistryContent::RegisterSubstitution(const SubstitutionMapping& aSubsti return; } - rv = sph->SetSubstitutionWithFlags(aSubstitution.path, resolvedURI, aSubstitution.flags); + rv = sph->SetSubstitution(aSubstitution.path, resolvedURI); if (NS_FAILED(rv)) return; } diff --git a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp index 1fd362bfb1ff..d6971b036bdb 100644 --- a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp +++ b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp @@ -117,14 +117,13 @@ nsresult SubstitutingProtocolHandler::CollectSubstitutions(InfallibleTArray& aMappings) { for (auto iter = mSubstitutions.ConstIter(); !iter.Done(); iter.Next()) { - SubstitutionEntry& entry = iter.Data(); - nsCOMPtr uri = entry.baseURI; + nsCOMPtr uri = iter.Data(); SerializedURI serialized; if (uri) { nsresult rv = uri->GetSpec(serialized.spec); NS_ENSURE_SUCCESS(rv, rv); } - SubstitutionMapping substitution = { mScheme, nsCString(iter.Key()), serialized, entry.flags }; + SubstitutionMapping substitution = { mScheme, nsCString(iter.Key()), serialized }; aMappings.AppendElement(substitution); } @@ -132,7 +131,7 @@ SubstitutingProtocolHandler::CollectSubstitutions(InfallibleTArrayGetSpec(mapping.resolvedURI.spec); NS_ENSURE_SUCCESS(rv, rv); } - mapping.flags = aFlags; for (uint32_t i = 0; i < parents.Length(); i++) { Unused << parents[i]->SendRegisterChromeItem(mapping); @@ -294,19 +292,11 @@ SubstitutingProtocolHandler::AllowPort(int32_t port, const char *scheme, bool *_ nsresult SubstitutingProtocolHandler::SetSubstitution(const nsACString& root, nsIURI *baseURI) -{ - // Add-ons use this API but they should not be able to make anything - // content-accessible. - return SetSubstitutionWithFlags(root, baseURI, 0); -} - -nsresult -SubstitutingProtocolHandler::SetSubstitutionWithFlags(const nsACString& root, nsIURI *baseURI, uint32_t flags) { if (!baseURI) { mSubstitutions.Remove(root); NotifyObservers(root, baseURI); - return SendSubstitution(root, baseURI, flags); + return SendSubstitution(root, baseURI); } // If baseURI isn't a same-scheme URI, we can set the substitution immediately. @@ -320,11 +310,9 @@ SubstitutingProtocolHandler::SetSubstitutionWithFlags(const nsACString& root, ns return NS_ERROR_INVALID_ARG; } - SubstitutionEntry& entry = mSubstitutions.GetOrInsert(root); - entry.baseURI = baseURI; - entry.flags = flags; + mSubstitutions.Put(root, baseURI); NotifyObservers(root, baseURI); - return SendSubstitution(root, baseURI, flags); + return SendSubstitution(root, baseURI); } // baseURI is a same-type substituting URI, let's resolve it first. @@ -336,11 +324,9 @@ SubstitutingProtocolHandler::SetSubstitutionWithFlags(const nsACString& root, ns rv = mIOService->NewURI(newBase, nullptr, nullptr, getter_AddRefs(newBaseURI)); NS_ENSURE_SUCCESS(rv, rv); - SubstitutionEntry& entry = mSubstitutions.GetOrInsert(root); - entry.baseURI = newBaseURI; - entry.flags = flags; + mSubstitutions.Put(root, newBaseURI); NotifyObservers(root, baseURI); - return SendSubstitution(root, newBaseURI, flags); + return SendSubstitution(root, newBaseURI); } nsresult @@ -348,29 +334,10 @@ SubstitutingProtocolHandler::GetSubstitution(const nsACString& root, nsIURI **re { NS_ENSURE_ARG_POINTER(result); - SubstitutionEntry entry; - if (mSubstitutions.Get(root, &entry)) { - nsCOMPtr baseURI = entry.baseURI; - baseURI.forget(result); + if (mSubstitutions.Get(root, result)) return NS_OK; - } - uint32_t flags; - return GetSubstitutionInternal(root, result, &flags); -} - -nsresult -SubstitutingProtocolHandler::GetSubstitutionFlags(const nsACString& root, uint32_t* flags) -{ - *flags = 0; - SubstitutionEntry entry; - if (mSubstitutions.Get(root, &entry)) { - *flags = entry.flags; - return NS_OK; - } - - nsCOMPtr baseURI; - return GetSubstitutionInternal(root, getter_AddRefs(baseURI), flags); + return GetSubstitutionInternal(root, result); } nsresult diff --git a/netwerk/protocol/res/SubstitutingProtocolHandler.h b/netwerk/protocol/res/SubstitutingProtocolHandler.h index 874eb1f9d04f..a08bd40de427 100644 --- a/netwerk/protocol/res/SubstitutingProtocolHandler.h +++ b/netwerk/protocol/res/SubstitutingProtocolHandler.h @@ -9,9 +9,9 @@ #include "nsISubstitutingProtocolHandler.h" +#include "nsInterfaceHashtable.h" #include "nsIOService.h" #include "nsISubstitutionObserver.h" -#include "nsDataHashtable.h" #include "nsStandardURL.h" #include "mozilla/chrome/RegistryMessageUtils.h" #include "mozilla/Maybe.h" @@ -44,16 +44,13 @@ protected: virtual ~SubstitutingProtocolHandler() {} void ConstructInternal(); - MOZ_MUST_USE nsresult SendSubstitution(const nsACString& aRoot, nsIURI* aBaseURI, uint32_t aFlags); - - nsresult GetSubstitutionFlags(const nsACString& root, uint32_t* flags); + MOZ_MUST_USE nsresult SendSubstitution(const nsACString& aRoot, nsIURI* aBaseURI); // Override this in the subclass to try additional lookups after checking // mSubstitutions. - virtual MOZ_MUST_USE nsresult GetSubstitutionInternal(const nsACString& aRoot, nsIURI** aResult, uint32_t* aFlags) + virtual MOZ_MUST_USE nsresult GetSubstitutionInternal(const nsACString& aRoot, nsIURI** aResult) { *aResult = nullptr; - *aFlags = 0; return NS_ERROR_NOT_AVAILABLE; } @@ -77,28 +74,13 @@ protected: nsIIOService* IOService() { return mIOService; } private: - struct SubstitutionEntry - { - SubstitutionEntry() - : flags(0) - { - } - - ~SubstitutionEntry() - { - } - - nsCOMPtr baseURI; - uint32_t flags; - }; - // Notifies all observers that a new substitution from |aRoot| to // |aBaseURI| has been set/installed for this protocol handler. void NotifyObservers(const nsACString& aRoot, nsIURI* aBaseURI); nsCString mScheme; Maybe mFlags; - nsDataHashtable mSubstitutions; + nsInterfaceHashtable mSubstitutions; nsCOMPtr mIOService; // The list of observers added with AddObserver that will be diff --git a/netwerk/protocol/res/nsIResProtocolHandler.idl b/netwerk/protocol/res/nsIResProtocolHandler.idl index 7046f2f1d452..56c597f4c73f 100644 --- a/netwerk/protocol/res/nsIResProtocolHandler.idl +++ b/netwerk/protocol/res/nsIResProtocolHandler.idl @@ -11,5 +11,4 @@ [scriptable, uuid(241d34ac-9ed5-46d7-910c-7a9d914aa0c5)] interface nsIResProtocolHandler : nsISubstitutingProtocolHandler { - boolean allowContentToAccess(in nsIURI url); }; diff --git a/netwerk/protocol/res/nsISubstitutingProtocolHandler.idl b/netwerk/protocol/res/nsISubstitutingProtocolHandler.idl index af0f02c28493..11be8a5b1e0b 100644 --- a/netwerk/protocol/res/nsISubstitutingProtocolHandler.idl +++ b/netwerk/protocol/res/nsISubstitutingProtocolHandler.idl @@ -14,11 +14,6 @@ interface nsISubstitutionObserver; [scriptable, uuid(154c64fd-a69e-4105-89f8-bd7dfe621372)] interface nsISubstitutingProtocolHandler : nsIProtocolHandler { - /** - * Content script may access files in this package. - */ - const short ALLOW_CONTENT_ACCESS = 1; - /** * Sets the substitution for the root key: * resource://root/path ==> baseURI.resolve(path) @@ -30,11 +25,6 @@ interface nsISubstitutingProtocolHandler : nsIProtocolHandler */ [must_use] void setSubstitution(in ACString root, in nsIURI baseURI); - /** - * Same as setSubstitution, but with specific flags. - */ - [must_use] void setSubstitutionWithFlags(in ACString root, in nsIURI baseURI, in uint32_t flags); - /** * Gets the substitution for the root key. * diff --git a/netwerk/protocol/res/nsResProtocolHandler.cpp b/netwerk/protocol/res/nsResProtocolHandler.cpp index 57b615e67b07..265bab9ec9dc 100644 --- a/netwerk/protocol/res/nsResProtocolHandler.cpp +++ b/netwerk/protocol/res/nsResProtocolHandler.cpp @@ -61,36 +61,16 @@ NS_IMPL_QUERY_INTERFACE(nsResProtocolHandler, nsIResProtocolHandler, NS_IMPL_ADDREF_INHERITED(nsResProtocolHandler, SubstitutingProtocolHandler) NS_IMPL_RELEASE_INHERITED(nsResProtocolHandler, SubstitutingProtocolHandler) -NS_IMETHODIMP -nsResProtocolHandler::AllowContentToAccess(nsIURI *aURI, bool *aResult) -{ - *aResult = false; - - nsAutoCString host; - nsresult rv = aURI->GetAsciiHost(host); - NS_ENSURE_SUCCESS(rv, rv); - - uint32_t flags; - rv = GetSubstitutionFlags(host, &flags); - NS_ENSURE_SUCCESS(rv, rv); - - *aResult = flags & nsISubstitutingProtocolHandler::ALLOW_CONTENT_ACCESS; - return NS_OK; -} - nsresult -nsResProtocolHandler::GetSubstitutionInternal(const nsACString& aRoot, - nsIURI** aResult, - uint32_t* aFlags) +nsResProtocolHandler::GetSubstitutionInternal(const nsACString& root, nsIURI **result) { nsAutoCString uri; - if (!ResolveSpecialCases(aRoot, NS_LITERAL_CSTRING("/"), NS_LITERAL_CSTRING("/"), uri)) { + if (!ResolveSpecialCases(root, NS_LITERAL_CSTRING("/"), NS_LITERAL_CSTRING("/"), uri)) { return NS_ERROR_NOT_AVAILABLE; } - *aFlags = 0; // No content access. - return NS_NewURI(aResult, uri); + return NS_NewURI(result, uri); } bool @@ -118,14 +98,3 @@ nsResProtocolHandler::SetSubstitution(const nsACString& aRoot, nsIURI* aBaseURI) MOZ_ASSERT(!aRoot.Equals(kGRE)); return SubstitutingProtocolHandler::SetSubstitution(aRoot, aBaseURI); } - -nsresult -nsResProtocolHandler::SetSubstitutionWithFlags(const nsACString& aRoot, - nsIURI* aBaseURI, - uint32_t aFlags) -{ - MOZ_ASSERT(!aRoot.Equals("")); - MOZ_ASSERT(!aRoot.Equals(kAPP)); - MOZ_ASSERT(!aRoot.Equals(kGRE)); - return SubstitutingProtocolHandler::SetSubstitutionWithFlags(aRoot, aBaseURI, aFlags); -} diff --git a/netwerk/protocol/res/nsResProtocolHandler.h b/netwerk/protocol/res/nsResProtocolHandler.h index 56bde73c0c1c..3f6243667586 100644 --- a/netwerk/protocol/res/nsResProtocolHandler.h +++ b/netwerk/protocol/res/nsResProtocolHandler.h @@ -34,7 +34,6 @@ public: MOZ_MUST_USE nsresult Init(); NS_IMETHOD SetSubstitution(const nsACString& aRoot, nsIURI* aBaseURI) override; - NS_IMETHOD SetSubstitutionWithFlags(const nsACString& aRoot, nsIURI* aBaseURI, uint32_t aFlags) override; NS_IMETHOD GetSubstitution(const nsACString& aRoot, nsIURI** aResult) override { @@ -62,7 +61,7 @@ public: } protected: - MOZ_MUST_USE nsresult GetSubstitutionInternal(const nsACString& aRoot, nsIURI** aResult, uint32_t* aFlags) override; + MOZ_MUST_USE nsresult GetSubstitutionInternal(const nsACString& aRoot, nsIURI** aResult) override; virtual ~nsResProtocolHandler() {} MOZ_MUST_USE bool ResolveSpecialCases(const nsACString& aHost, diff --git a/xpcom/components/ManifestParser.cpp b/xpcom/components/ManifestParser.cpp index 63d7d56523d4..ea1a243bae1f 100644 --- a/xpcom/components/ManifestParser.cpp +++ b/xpcom/components/ManifestParser.cpp @@ -56,7 +56,7 @@ struct ManifestDirective bool allowbootstrap; - // The contentaccessible flags only apply to content/resource directives. + // The contentaccessible flags only apply to content directives. bool contentflags; // Function to handle this directive. This isn't a union because C++ still @@ -123,7 +123,7 @@ static const ManifestDirective kParsingTable[] = { nullptr, &nsChromeRegistry::ManifestOverride, nullptr }, { - "resource", 2, false, true, true, true, true, + "resource", 2, false, true, true, true, false, nullptr, &nsChromeRegistry::ManifestResource, nullptr } };