Bug 1297300 - Add missing checks to GetSpec() calls in caps/ and js/. r=mrbkap.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 30 Aug 2016 14:22:04 +1000
changeset 354656 ee83d585ff9db5e419c9ed5bf21884c8689df19a
parent 354655 fbe1cc85a7e6b96ba60f8b61f5721f18ab2ee2d6
child 354657 d6663520be22a4f183f5b2e227bbda0f178a101b
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs1297300
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1297300 - Add missing checks to GetSpec() calls in caps/ and js/. r=mrbkap. This required making GetScriptLocation() fallible.
caps/nsJSPrincipals.cpp
caps/nsJSPrincipals.h
caps/nsNullPrincipal.cpp
caps/nsNullPrincipal.h
caps/nsNullPrincipalURI.cpp
caps/nsPrincipal.cpp
caps/nsPrincipal.h
caps/nsSystemPrincipal.cpp
caps/nsSystemPrincipal.h
js/xpconnect/loader/mozJSSubScriptLoader.cpp
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/XPCJSRuntime.cpp
--- a/caps/nsJSPrincipals.cpp
+++ b/caps/nsJSPrincipals.cpp
@@ -83,18 +83,19 @@ nsJSPrincipals::Destroy(JSPrincipals *js
 #ifdef DEBUG
 
 // Defined here so one can do principals->dump() in the debugger
 JS_PUBLIC_API(void)
 JSPrincipals::dump()
 {
     if (debugToken == nsJSPrincipals::DEBUG_TOKEN) {
       nsAutoCString str;
-      static_cast<nsJSPrincipals *>(this)->GetScriptLocation(str);
-      fprintf(stderr, "nsIPrincipal (%p) = %s\n", static_cast<void*>(this), str.get());
+      nsresult rv = static_cast<nsJSPrincipals *>(this)->GetScriptLocation(str);
+      fprintf(stderr, "nsIPrincipal (%p) = %s\n", static_cast<void*>(this),
+              NS_SUCCEEDED(rv) ? str.get() : "(unknown)");
     } else if (debugToken == dom::workers::kJSPrincipalsDebugToken) {
         fprintf(stderr, "Web Worker principal singleton (%p)\n", this);
     } else {
         fprintf(stderr,
                 "!!! JSPrincipals (%p) is not nsJSPrincipals instance - bad token: "
                 "actual=0x%x expected=0x%x\n",
                 this, unsigned(debugToken), unsigned(nsJSPrincipals::DEBUG_TOKEN));
     }
--- a/caps/nsJSPrincipals.h
+++ b/caps/nsJSPrincipals.h
@@ -48,17 +48,17 @@ public:
   nsJSPrincipals() {
     refcount = 0;
     setDebugToken(DEBUG_TOKEN);
   }
 
   /**
    * Return a string that can be used as JS script filename in error reports.
    */
-  virtual void GetScriptLocation(nsACString &aStr) = 0;
+  virtual nsresult GetScriptLocation(nsACString &aStr) = 0;
   static const uint32_t DEBUG_TOKEN = 0x0bf41760;
 
 protected:
   virtual ~nsJSPrincipals() {
     setDebugToken(0);
   }
 
 };
--- a/caps/nsNullPrincipal.cpp
+++ b/caps/nsNullPrincipal.cpp
@@ -73,20 +73,20 @@ nsNullPrincipal::Init(const PrincipalOri
   mOriginAttributes = aOriginAttributes;
 
   mURI = nsNullPrincipalURI::Create();
   NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_AVAILABLE);
 
   return NS_OK;
 }
 
-void
+nsresult
 nsNullPrincipal::GetScriptLocation(nsACString &aStr)
 {
-  mURI->GetSpec(aStr);
+  return mURI->GetSpec(aStr);
 }
 
 /**
  * nsIPrincipal implementation
  */
 
 NS_IMETHODIMP
 nsNullPrincipal::GetHashValue(uint32_t *aResult)
--- a/caps/nsNullPrincipal.h
+++ b/caps/nsNullPrincipal.h
@@ -52,17 +52,17 @@ public:
 
   static already_AddRefed<nsNullPrincipal> CreateWithInheritedAttributes(nsIDocShell* aDocShell);
 
   static already_AddRefed<nsNullPrincipal>
   Create(const mozilla::PrincipalOriginAttributes& aOriginAttributes = mozilla::PrincipalOriginAttributes());
 
   nsresult Init(const mozilla::PrincipalOriginAttributes& aOriginAttributes = mozilla::PrincipalOriginAttributes());
 
-  virtual void GetScriptLocation(nsACString &aStr) override;
+  virtual nsresult GetScriptLocation(nsACString &aStr) override;
 
   PrincipalKind Kind() override { return eNullPrincipal; }
 
  protected:
   virtual ~nsNullPrincipal() {}
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override
   {
--- a/caps/nsNullPrincipalURI.cpp
+++ b/caps/nsNullPrincipalURI.cpp
@@ -91,17 +91,18 @@ nsNullPrincipalURI::GetAsciiHostPort(nsA
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsNullPrincipalURI::GetAsciiSpec(nsACString &_spec)
 {
   nsAutoCString buffer;
-  (void)GetSpec(buffer);
+  // Ignore the return value -- nsNullPrincipalURI::GetSpec() is infallible.
+  Unused << GetSpec(buffer);
   NS_EscapeURL(buffer, esc_OnlyNonASCII | esc_AlwaysCopy, _spec);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNullPrincipalURI::GetHost(nsACString &_host)
 {
   _host.Truncate();
--- a/caps/nsPrincipal.cpp
+++ b/caps/nsPrincipal.cpp
@@ -94,20 +94,20 @@ nsPrincipal::Init(nsIURI *aCodebase, con
 
   mCodebase = NS_TryToMakeImmutable(aCodebase);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
   mOriginAttributes = aOriginAttributes;
 
   return NS_OK;
 }
 
-void
+nsresult
 nsPrincipal::GetScriptLocation(nsACString &aStr)
 {
-  mCodebase->GetSpec(aStr);
+  return mCodebase->GetSpec(aStr);
 }
 
 /* static */ nsresult
 nsPrincipal::GetOriginForURI(nsIURI* aURI, nsACString& aOrigin)
 {
   if (!aURI) {
     return NS_ERROR_FAILURE;
   }
@@ -801,32 +801,34 @@ bool
 nsExpandedPrincipal::IsOnCSSUnprefixingWhitelist()
 {
   // CSS Unprefixing Whitelist is a per-origin thing; doesn't really make sense
   // for an expanded principal. (And probably shouldn't be needed.)
   return false;
 }
 
 
-void
+nsresult
 nsExpandedPrincipal::GetScriptLocation(nsACString& aStr)
 {
   aStr.Assign("[Expanded Principal [");
   for (size_t i = 0; i < mPrincipals.Length(); ++i) {
     if (i != 0) {
       aStr.AppendLiteral(", ");
     }
 
     nsAutoCString spec;
-    nsJSPrincipals::get(mPrincipals.ElementAt(i))->GetScriptLocation(spec);
+    nsresult rv =
+      nsJSPrincipals::get(mPrincipals.ElementAt(i))->GetScriptLocation(spec);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     aStr.Append(spec);
-
   }
   aStr.Append("]]");
+  return NS_OK;
 }
 
 //////////////////////////////////////////
 // Methods implementing nsISerializable //
 //////////////////////////////////////////
 
 NS_IMETHODIMP
 nsExpandedPrincipal::Read(nsIObjectInputStream* aStream)
--- a/caps/nsPrincipal.h
+++ b/caps/nsPrincipal.h
@@ -29,17 +29,17 @@ public:
   bool IsCodebasePrincipal() const override { return true; }
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   nsPrincipal();
 
   // Init() must be called before the principal is in a usable state.
   nsresult Init(nsIURI* aCodebase, const mozilla::PrincipalOriginAttributes& aOriginAttributes);
 
-  virtual void GetScriptLocation(nsACString& aStr) override;
+  virtual nsresult GetScriptLocation(nsACString& aStr) override;
   void SetURI(nsIURI* aURI);
 
   /**
    * Computes the puny-encoded origin of aURI.
    */
   static nsresult GetOriginForURI(nsIURI* aURI, nsACString& aOrigin);
 
   /**
@@ -76,17 +76,17 @@ public:
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
   NS_IMETHOD GetHashValue(uint32_t* aHashValue) override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   virtual bool AddonHasPermission(const nsAString& aPerm) override;
   virtual bool IsOnCSSUnprefixingWhitelist() override;
-  virtual void GetScriptLocation(nsACString &aStr) override;
+  virtual nsresult GetScriptLocation(nsACString &aStr) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   PrincipalKind Kind() override { return eExpandedPrincipal; }
 
 protected:
   virtual ~nsExpandedPrincipal();
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override;
--- a/caps/nsSystemPrincipal.cpp
+++ b/caps/nsSystemPrincipal.cpp
@@ -26,20 +26,21 @@ NS_IMPL_QUERY_INTERFACE_CI(nsSystemPrinc
                            nsIPrincipal,
                            nsISerializable)
 NS_IMPL_CI_INTERFACE_GETTER(nsSystemPrincipal,
                             nsIPrincipal,
                             nsISerializable)
 
 #define SYSTEM_PRINCIPAL_SPEC "[System Principal]"
 
-void
+nsresult
 nsSystemPrincipal::GetScriptLocation(nsACString &aStr)
 {
     aStr.AssignLiteral(SYSTEM_PRINCIPAL_SPEC);
+    return NS_OK;
 }
 
 ///////////////////////////////////////
 // Methods implementing nsIPrincipal //
 ///////////////////////////////////////
 
 NS_IMETHODIMP
 nsSystemPrincipal::GetHashValue(uint32_t *result)
--- a/caps/nsSystemPrincipal.h
+++ b/caps/nsSystemPrincipal.h
@@ -33,17 +33,17 @@ public:
   NS_IMETHOD EnsureCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP) override;
   NS_IMETHOD EnsurePreloadCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   nsSystemPrincipal() {}
 
-  virtual void GetScriptLocation(nsACString &aStr) override;
+  virtual nsresult GetScriptLocation(nsACString &aStr) override;
 
 protected:
   virtual ~nsSystemPrincipal(void) {}
 
   bool SubsumesInternal(nsIPrincipal *aOther, DocumentDomainConsideration aConsideration) override
   {
     return true;
   }
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -362,24 +362,24 @@ AsyncScriptLoader::OnStreamComplete(nsII
 
     if (aLength > INT32_MAX) {
         return ReportError(cx, LOAD_ERROR_CONTENTTOOBIG, uri);
     }
 
     RootedFunction function(cx);
     RootedScript script(cx);
     nsAutoCString spec;
-    uri->GetSpec(spec);
+    nsresult rv = uri->GetSpec(spec);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     RootedObject target_obj(cx, mTargetObj);
 
-    nsresult rv = PrepareScript(uri, cx, target_obj, spec.get(),
-                                mCharset,
-                                reinterpret_cast<const char*>(aBuf), aLength,
-                                mReuseGlobal, &script, &function);
+    rv = PrepareScript(uri, cx, target_obj, spec.get(), mCharset,
+                       reinterpret_cast<const char*>(aBuf), aLength,
+                       mReuseGlobal, &script, &function);
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     JS::Rooted<JS::Value> retval(cx);
     rv = EvalScript(cx, target_obj, &retval, uri, mCache, script, function);
 
     if (NS_SUCCEEDED(rv)) {
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -1759,17 +1759,18 @@ xpc::EvalInSandbox(JSContext* cx, Handle
     nsCOMPtr<nsIPrincipal> prin = sop->GetPrincipal();
     NS_ENSURE_TRUE(prin, NS_ERROR_FAILURE);
 
     nsAutoCString filenameBuf;
     if (!filename.IsVoid() && filename.Length() != 0) {
         filenameBuf.Assign(filename);
     } else {
         // Default to the spec of the principal.
-        nsJSPrincipals::get(prin)->GetScriptLocation(filenameBuf);
+        nsresult rv = nsJSPrincipals::get(prin)->GetScriptLocation(filenameBuf);
+        NS_ENSURE_SUCCESS(rv, rv);
         lineNo = 1;
     }
 
     // We create a separate cx to do the sandbox evaluation. Scope it.
     RootedValue v(cx, UndefinedValue());
     RootedValue exn(cx, UndefinedValue());
     bool ok = true;
     {
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -1663,17 +1663,20 @@ GetCompartmentName(JSCompartment* c, nsC
                    bool replaceSlashes)
 {
     if (js::IsAtomsCompartment(c)) {
         name.AssignLiteral("atoms");
     } else if (*anonymizeID && !js::IsSystemCompartment(c)) {
         name.AppendPrintf("<anonymized-%d>", *anonymizeID);
         *anonymizeID += 1;
     } else if (JSPrincipals* principals = JS_GetCompartmentPrincipals(c)) {
-        nsJSPrincipals::get(principals)->GetScriptLocation(name);
+        nsresult rv = nsJSPrincipals::get(principals)->GetScriptLocation(name);
+        if (NS_FAILED(rv)) {
+            name.AssignLiteral("(unknown)");
+        }
 
         // If the compartment's location (name) differs from the principal's
         // script location, append the compartment's location to allow
         // differentiation of multiple compartments owned by the same principal
         // (e.g. components owned by the system or null principal).
         CompartmentPrivate* compartmentPrivate = CompartmentPrivate::Get(c);
         if (compartmentPrivate) {
             const nsACString& location = compartmentPrivate->GetLocation();