Bug 1297300 - Add missing checks to GetSpec() calls in image/. r=tnikkel.
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 29 Aug 2016 15:34:32 +1000
changeset 313379 d008348cd4f450faf68fdeba7ca2d8146ddce83a
parent 313378 d8fd0a41c62f7a22a2c288f0eda2174166a22e56
child 313380 540bd4b5aa3dda58a032982b1bab1d0c0eff783a
push id30679
push usercbook@mozilla.com
push dateFri, 09 Sep 2016 10:03:06 +0000
treeherdermozilla-central@feff79e5b137 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
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 image/. r=tnikkel.
image/ImageCacheKey.cpp
image/ImageCacheKey.h
image/ImageURL.h
image/decoders/icon/nsIconURI.cpp
image/imgICache.idl
image/imgLoader.cpp
image/imgLoader.h
image/imgRequest.cpp
image/imgRequest.h
--- a/image/ImageCacheKey.cpp
+++ b/image/ImageCacheKey.cpp
@@ -42,22 +42,25 @@ BlobSerial(ImageURL* aURI)
     return Some(blob->GetSerialNumber());
   }
 
   return Nothing();
 }
 
 ImageCacheKey::ImageCacheKey(nsIURI* aURI,
                              const PrincipalOriginAttributes& aAttrs,
-                             nsIDocument* aDocument)
-  : mURI(new ImageURL(aURI))
+                             nsIDocument* aDocument,
+                             nsresult& aRv)
+  : mURI(new ImageURL(aURI, aRv))
   , mOriginAttributes(aAttrs)
   , mControlledDocument(GetControlledDocumentToken(aDocument))
   , mIsChrome(URISchemeIs(mURI, "chrome"))
 {
+  NS_ENSURE_SUCCESS_VOID(aRv);
+
   MOZ_ASSERT(NS_IsMainThread());
 
   if (URISchemeIs(mURI, "blob")) {
     mBlobSerial = BlobSerial(mURI);
   }
 
   mHash = ComputeHash(mURI, mBlobSerial, mOriginAttributes, mControlledDocument);
 }
--- a/image/ImageCacheKey.h
+++ b/image/ImageCacheKey.h
@@ -29,17 +29,17 @@ class ImageURL;
  * canonicalization applied. See ComputeHash() for the details.
  * Controlled documents do not share their cache entries with
  * non-controlled documents, or other controlled documents.
  */
 class ImageCacheKey final
 {
 public:
   ImageCacheKey(nsIURI* aURI, const PrincipalOriginAttributes& aAttrs,
-                nsIDocument* aDocument);
+                nsIDocument* aDocument, nsresult& aRv);
   ImageCacheKey(ImageURL* aURI, const PrincipalOriginAttributes& aAttrs,
                 nsIDocument* aDocument);
 
   ImageCacheKey(const ImageCacheKey& aOther);
   ImageCacheKey(ImageCacheKey&& aOther);
 
   bool operator==(const ImageCacheKey& aOther) const;
   uint32_t Hash() const { return mHash; }
--- a/image/ImageURL.h
+++ b/image/ImageURL.h
@@ -23,22 +23,28 @@ namespace image {
  * intentional; functionality is limited, and is only useful for imagelib code.
  * By not implementing nsIURI, external code cannot unintentionally be given an
  * nsIURI pointer with this limited class behind it; instead, conversion to a
  * fully implemented nsIURI is required (e.g. through NS_NewURI).
  */
 class ImageURL
 {
 public:
-  explicit ImageURL(nsIURI* aURI)
+  explicit ImageURL(nsIURI* aURI, nsresult& aRv)
   {
     MOZ_ASSERT(NS_IsMainThread(), "Cannot use nsIURI off main thread!");
-    aURI->GetSpec(mSpec);
-    aURI->GetScheme(mScheme);
-    aURI->GetRef(mRef);
+
+    aRv = aURI->GetSpec(mSpec);
+    NS_ENSURE_SUCCESS_VOID(aRv);
+
+    aRv = aURI->GetScheme(mScheme);
+    NS_ENSURE_SUCCESS_VOID(aRv);
+
+    aRv = aURI->GetRef(mRef);
+    NS_ENSURE_SUCCESS_VOID(aRv);
   }
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ImageURL)
 
   nsresult GetSpec(nsACString& result)
   {
     result = mSpec;
     return NS_OK;
--- a/image/decoders/icon/nsIconURI.cpp
+++ b/image/decoders/icon/nsIconURI.cpp
@@ -382,24 +382,28 @@ NS_IMETHODIMP
 nsMozIconURI::SetRef(const nsACString& aRef)
 {
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsMozIconURI::Equals(nsIURI* other, bool* result)
 {
+  *result = false;
   NS_ENSURE_ARG_POINTER(other);
   NS_PRECONDITION(result, "null pointer");
 
   nsAutoCString spec1;
   nsAutoCString spec2;
 
-  other->GetSpec(spec2);
-  GetSpec(spec1);
+  nsresult rv = GetSpec(spec1);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = other->GetSpec(spec2);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   if (!PL_strcasecmp(spec1.get(), spec2.get())) {
     *result = true;
   } else {
     *result = false;
   }
   return NS_OK;
 }
 
--- a/image/imgICache.idl
+++ b/image/imgICache.idl
@@ -40,17 +40,19 @@ interface imgICache : nsISupports
    * succeed, but come back empty.
    *
    * Hopefully this will be removed with bug 805119
    *
    * @param uri The URI to look up.
    * @param doc Optional pointer to the document that the cache entry belongs to.
    * @returns NULL if the URL was not found in the cache
    */
-  nsIProperties findEntryProperties(in nsIURI uri, [optional] in nsIDOMDocument doc);
+  [must_use]
+  nsIProperties findEntryProperties(in nsIURI uri,
+                                    [optional] in nsIDOMDocument doc);
 
   /**
    * Make this cache instance respect private browsing notifications. This
    * entails clearing the chrome and content caches whenever the
    * last-pb-context-exited notification is observed.
    */
   void respectPrivacyNotifications();
 
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -1308,17 +1308,19 @@ imgLoader::FindEntryProperties(nsIURI* u
   PrincipalOriginAttributes attrs;
   if (doc) {
     nsCOMPtr<nsIPrincipal> principal = doc->NodePrincipal();
     if (principal) {
       attrs = BasePrincipal::Cast(principal)->OriginAttributesRef();
     }
   }
 
-  ImageCacheKey key(uri, attrs, doc);
+  nsresult rv;
+  ImageCacheKey key(uri, attrs, doc, rv);
+  NS_ENSURE_SUCCESS(rv, rv);
   imgCacheTable& cache = GetCache(key);
 
   RefPtr<imgCacheEntry> entry;
   if (cache.Get(key, getter_AddRefs(entry)) && entry) {
     if (mCacheTracker && entry->HasNoProxies()) {
       mCacheTracker->MarkUsed(entry);
     }
 
@@ -2066,17 +2068,18 @@ imgLoader::LoadImage(nsIURI* aURI,
   // Look in the cache for our URI, and then validate it.
   // XXX For now ignore aCacheKey. We will need it in the future
   // for correctly dealing with image load requests that are a result
   // of post data.
   PrincipalOriginAttributes attrs;
   if (aLoadingPrincipal) {
     attrs = BasePrincipal::Cast(aLoadingPrincipal)->OriginAttributesRef();
   }
-  ImageCacheKey key(aURI, attrs, aLoadingDocument);
+  ImageCacheKey key(aURI, attrs, aLoadingDocument, rv);
+  NS_ENSURE_SUCCESS(rv, rv);
   imgCacheTable& cache = GetCache(key);
 
   if (cache.Get(key, getter_AddRefs(entry)) && entry) {
     if (ValidateEntry(entry, aURI, aInitialDocumentURI, aReferrerURI,
                       aReferrerPolicy, aLoadGroup, aObserver, aLoadingDocument,
                       requestFlags, aContentPolicyType, true, _retval,
                       aLoadingPrincipal, corsmode)) {
       request = entry->GetRequest();
@@ -2137,19 +2140,22 @@ imgLoader::LoadImage(nsIURI* aURI,
                        getter_AddRefs(entry));
 
     MOZ_LOG(gImgLog, LogLevel::Debug,
            ("[this=%p] imgLoader::LoadImage -- Created new imgRequest"
             " [request=%p]\n", this, request.get()));
 
     nsCOMPtr<nsILoadGroup> channelLoadGroup;
     newChannel->GetLoadGroup(getter_AddRefs(channelLoadGroup));
-    request->Init(aURI, aURI, /* aHadInsecureRedirect = */ false,
-                  channelLoadGroup, newChannel, entry, aLoadingDocument,
-                  aLoadingPrincipal, corsmode, aReferrerPolicy);
+    rv = request->Init(aURI, aURI, /* aHadInsecureRedirect = */ false,
+                       channelLoadGroup, newChannel, entry, aLoadingDocument,
+                       aLoadingPrincipal, corsmode, aReferrerPolicy);
+    if (NS_FAILED(rv)) {
+      return NS_ERROR_FAILURE;
+    }
 
     // Add the initiator type for this image load
     nsCOMPtr<nsITimedChannel> timedChannel = do_QueryInterface(newChannel);
     if (timedChannel) {
       timedChannel->SetInitiatorType(initiatorType);
     }
 
     // create the proxy listener
@@ -2275,17 +2281,19 @@ imgLoader::LoadImageWithChannel(nsIChann
   NS_ENSURE_TRUE(channel, NS_ERROR_FAILURE);
   nsCOMPtr<nsILoadInfo> loadInfo = channel->GetLoadInfo();
 
   PrincipalOriginAttributes attrs;
   if (loadInfo) {
     attrs.InheritFromNecko(loadInfo->GetOriginAttributes());
   }
 
-  ImageCacheKey key(uri, attrs, doc);
+  nsresult rv;
+  ImageCacheKey key(uri, attrs, doc, rv);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   nsLoadFlags requestFlags = nsIRequest::LOAD_NORMAL;
   channel->GetLoadFlags(&requestFlags);
 
   RefPtr<imgCacheEntry> entry;
 
   if (requestFlags & nsIRequest::LOAD_BYPASS_CACHE) {
     RemoveFromCache(key);
@@ -2356,17 +2364,17 @@ imgLoader::LoadImageWithChannel(nsIChann
   }
 
   nsCOMPtr<nsILoadGroup> loadGroup;
   channel->GetLoadGroup(getter_AddRefs(loadGroup));
 
   // Filter out any load flags not from nsIRequest
   requestFlags &= nsIRequest::LOAD_REQUESTMASK;
 
-  nsresult rv = NS_OK;
+  rv = NS_OK;
   if (request) {
     // we have this in our cache already.. cancel the current (document) load
 
     // this should fire an OnStopRequest
     channel->Cancel(NS_ERROR_PARSED_DATA_CACHED);
 
     *listener = nullptr; // give them back a null nsIStreamListener
 
@@ -2377,17 +2385,18 @@ imgLoader::LoadImageWithChannel(nsIChann
     // We use originalURI here to fulfil the imgIRequest contract on GetURI.
     nsCOMPtr<nsIURI> originalURI;
     channel->GetOriginalURI(getter_AddRefs(originalURI));
 
     // XXX(seth): We should be able to just use |key| here, except that |key| is
     // constructed above with the *current URI* and not the *original URI*. I'm
     // pretty sure this is a bug, and it's preventing us from ever getting a
     // cache hit in LoadImageWithChannel when redirects are involved.
-    ImageCacheKey originalURIKey(originalURI, attrs, doc);
+    ImageCacheKey originalURIKey(originalURI, attrs, doc, rv);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     // Default to doing a principal check because we don't know who
     // started that load and whether their principal ended up being
     // inherited on the channel.
     NewRequestAndEntry(/* aForcePrincipalCheckForCacheEntry = */ true,
                        this, originalURIKey,
                        getter_AddRefs(request),
                        getter_AddRefs(entry));
@@ -2395,19 +2404,20 @@ imgLoader::LoadImageWithChannel(nsIChann
     // No principal specified here, because we're not passed one.
     // In LoadImageWithChannel, the redirects that may have been
     // assoicated with this load would have gone through necko.
     // We only have the final URI in ImageLib and hence don't know
     // if the request went through insecure redirects.  But if it did,
     // the necko cache should have handled that (since all necko cache hits
     // including the redirects will go through content policy).  Hence, we
     // can set aHadInsecureRedirect to false here.
-    request->Init(originalURI, uri, /* aHadInsecureRedirect = */ false,
-                  channel, channel, entry, aCX, nullptr,
-                  imgIRequest::CORS_NONE, RP_Default);
+    rv = request->Init(originalURI, uri, /* aHadInsecureRedirect = */ false,
+                       channel, channel, entry, aCX, nullptr,
+                       imgIRequest::CORS_NONE, RP_Default);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     RefPtr<ProxyListener> pl =
       new ProxyListener(static_cast<nsIStreamListener*>(request.get()));
     pl.forget(listener);
 
     // Try to add the new request into the cache.
     PutIntoCache(originalURIKey, entry);
 
@@ -2768,18 +2778,22 @@ imgCacheValidator::OnStartRequest(nsIReq
   mRequest->RemoveFromCache();
 
   mRequest->SetValidator(nullptr);
   mRequest = nullptr;
 
   // We use originalURI here to fulfil the imgIRequest contract on GetURI.
   nsCOMPtr<nsIURI> originalURI;
   channel->GetOriginalURI(getter_AddRefs(originalURI));
-  mNewRequest->Init(originalURI, uri, mHadInsecureRedirect, aRequest, channel,
-                    mNewEntry, context, loadingPrincipal, corsmode, refpol);
+  nsresult rv =
+    mNewRequest->Init(originalURI, uri, mHadInsecureRedirect, aRequest, channel,
+                      mNewEntry, context, loadingPrincipal, corsmode, refpol);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   mDestListener = new ProxyListener(mNewRequest);
 
   // Try to add the new request into the cache. Note that the entry must be in
   // the cache before the proxies' ownership changes, because adding a proxy
   // changes the caching behaviour for imgRequests.
   mImgLoader->PutIntoCache(mNewRequest->CacheKey(), mNewEntry);
 
--- a/image/imgLoader.h
+++ b/image/imgLoader.h
@@ -285,36 +285,37 @@ public:
    * All the same, even though what these add-ons are doing is a no-op,
    * removing the nsIServiceManager.getService method of creating/getting an
    * imgLoader objects would cause an exception in these add-ons that could
    * break things.
    */
   imgLoader();
   nsresult Init();
 
-  nsresult LoadImage(nsIURI* aURI,
-                     nsIURI* aInitialDocumentURI,
-                     nsIURI* aReferrerURI,
-                     ReferrerPolicy aReferrerPolicy,
-                     nsIPrincipal* aLoadingPrincipal,
-                     nsILoadGroup* aLoadGroup,
-                     imgINotificationObserver* aObserver,
-                     nsINode* aContext,
-                     nsIDocument* aLoadingDocument,
-                     nsLoadFlags aLoadFlags,
-                     nsISupports* aCacheKey,
-                     nsContentPolicyType aContentPolicyType,
-                     const nsAString& initiatorType,
-                     imgRequestProxy** _retval);
+  MOZ_MUST_USE nsresult LoadImage(nsIURI* aURI,
+                                  nsIURI* aInitialDocumentURI,
+                                  nsIURI* aReferrerURI,
+                                  ReferrerPolicy aReferrerPolicy,
+                                  nsIPrincipal* aLoadingPrincipal,
+                                  nsILoadGroup* aLoadGroup,
+                                  imgINotificationObserver* aObserver,
+                                  nsINode* aContext,
+                                  nsIDocument* aLoadingDocument,
+                                  nsLoadFlags aLoadFlags,
+                                  nsISupports* aCacheKey,
+                                  nsContentPolicyType aContentPolicyType,
+                                  const nsAString& initiatorType,
+                                  imgRequestProxy** _retval);
 
-  nsresult LoadImageWithChannel(nsIChannel* channel,
-                                imgINotificationObserver* aObserver,
-                                nsISupports* aCX,
-                                nsIStreamListener** listener,
-                                imgRequestProxy** _retval);
+  MOZ_MUST_USE nsresult
+  LoadImageWithChannel(nsIChannel* channel,
+                       imgINotificationObserver* aObserver,
+                       nsISupports* aCX,
+                       nsIStreamListener** listener,
+                       imgRequestProxy** _retval);
 
   static nsresult GetMimeTypeFromContent(const char* aContents,
                                          uint32_t aLength,
                                          nsACString& aContentType);
 
   /**
    * Returns true if the given mime type may be interpreted as an image.
    *
--- a/image/imgRequest.cpp
+++ b/image/imgRequest.cpp
@@ -103,17 +103,20 @@ imgRequest::Init(nsIURI *aURI,
   MOZ_ASSERT(aURI, "No uri");
   MOZ_ASSERT(aCurrentURI, "No current uri");
   MOZ_ASSERT(aRequest, "No request");
   MOZ_ASSERT(aChannel, "No channel");
 
   mProperties = do_CreateInstance("@mozilla.org/properties;1");
 
   // Use ImageURL to ensure access to URI data off main thread.
-  mURI = new ImageURL(aURI);
+  nsresult rv;
+  mURI = new ImageURL(aURI, rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   mCurrentURI = aCurrentURI;
   mRequest = aRequest;
   mChannel = aChannel;
   mTimedChannel = do_QueryInterface(mChannel);
 
   mLoadingPrincipal = aLoadingPrincipal;
   mCORSMode = aCORSMode;
   mReferrerPolicy = aReferrerPolicy;
--- a/image/imgRequest.h
+++ b/image/imgRequest.h
@@ -60,26 +60,26 @@ public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSIASYNCVERIFYREDIRECTCALLBACK
 
-  nsresult Init(nsIURI* aURI,
-                nsIURI* aCurrentURI,
-                bool aHadInsecureRedirect,
-                nsIRequest* aRequest,
-                nsIChannel* aChannel,
-                imgCacheEntry* aCacheEntry,
-                nsISupports* aCX,
-                nsIPrincipal* aLoadingPrincipal,
-                int32_t aCORSMode,
-                ReferrerPolicy aReferrerPolicy);
+  MOZ_MUST_USE nsresult Init(nsIURI* aURI,
+                             nsIURI* aCurrentURI,
+                             bool aHadInsecureRedirect,
+                             nsIRequest* aRequest,
+                             nsIChannel* aChannel,
+                             imgCacheEntry* aCacheEntry,
+                             nsISupports* aCX,
+                             nsIPrincipal* aLoadingPrincipal,
+                             int32_t aCORSMode,
+                             ReferrerPolicy aReferrerPolicy);
 
   void ClearLoader();
 
   // Callers must call imgRequestProxy::Notify later.
   void AddProxy(imgRequestProxy* proxy);
 
   nsresult RemoveProxy(imgRequestProxy* proxy, nsresult aStatus);