Bug 781126 - Part 2 - nsObjectLoadingContent should call shouldLoad and shouldProcess sanely. r=jst
authorJohn Schoenick <jschoenick@mozilla.com>
Thu, 16 Aug 2012 18:44:14 -0700
changeset 102682 812ea773f16642c714947a3b5acde6d7d7472250
parent 102681 85faeeb2306d513be0198fe8302d63699d316380
child 102683 d3bc83c14073521837cd51102e9b3e46f0d3e313
child 102715 54e0653278ba2cf62e3107a995399e37480878f1
push id13587
push userjschoenick@mozilla.com
push dateFri, 17 Aug 2012 21:46:42 +0000
treeherdermozilla-inbound@812ea773f166 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjst
bugs781126
milestone17.0a1
first release with
nightly linux32
812ea773f166 / 17.0a1 / 20120818030604 / files
nightly linux64
812ea773f166 / 17.0a1 / 20120818030604 / files
nightly mac
812ea773f166 / 17.0a1 / 20120818030604 / files
nightly win32
812ea773f166 / 17.0a1 / 20120818030604 / files
nightly win64
812ea773f166 / 17.0a1 / 20120818030604 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 781126 - Part 2 - nsObjectLoadingContent should call shouldLoad and shouldProcess sanely. r=jst
content/base/src/nsObjectLoadingContent.cpp
content/base/src/nsObjectLoadingContent.h
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -1064,98 +1064,102 @@ nsObjectLoadingContent::ObjectState() co
         case eFallbackVulnerableNoUpdate:
           return NS_EVENT_STATE_VULNERABLE_NO_UPDATE;
       }
   };
   NS_NOTREACHED("unknown type?");
   return NS_EVENT_STATE_LOADING;
 }
 
-// Helper to call CheckURILoad on URI -> BaseURI and BaseURI -> Origin
-bool nsObjectLoadingContent::CheckObjectURIs(PRInt16 *aContentPolicy,
-                                             PRInt32 aContentPolicyType)
+bool
+nsObjectLoadingContent::CheckLoadPolicy(PRInt16 *aContentPolicy)
 {
+  if (!aContentPolicy || !mURI) {
+    NS_NOTREACHED("Doing it wrong");
+    return false;
+  }
+
   nsCOMPtr<nsIContent> thisContent =
     do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
   NS_ASSERTION(thisContent, "Must be an instance of content");
 
-  nsCOMPtr<nsIURI> docBaseURI = thisContent->GetBaseURI();
-
-  // Must have these to load
-  if (!aContentPolicy || !mBaseURI) {
-    return false;
-  }
+  nsIDocument* doc = thisContent->OwnerDoc();
 
-  bool ret;
-  if (!URIEquals(mBaseURI, docBaseURI)) {
-    // If our object sets a new baseURI, make sure that base URI could be
-    // loaded by the document
-    ret = CheckURILoad(mBaseURI, aContentPolicy, aContentPolicyType);
-    if (!ret) {
-      return false;
-    }
-  }
-
-  if (mURI) {
-    return CheckURILoad(mURI, aContentPolicy, aContentPolicyType);
+  *aContentPolicy = nsIContentPolicy::ACCEPT;
+  nsresult rv = NS_CheckContentLoadPolicy(nsIContentPolicy::TYPE_OBJECT,
+                                          mURI,
+                                          doc->NodePrincipal(),
+                                          thisContent,
+                                          mContentType,
+                                          nullptr, //extra
+                                          aContentPolicy,
+                                          nsContentUtils::GetContentPolicy(),
+                                          nsContentUtils::GetSecurityManager());
+  NS_ENSURE_SUCCESS(rv, false);
+  if (NS_CP_REJECTED(*aContentPolicy)) {
+    nsCAutoString uri;
+    nsCAutoString baseUri;
+    mURI->GetSpec(uri);
+    mURI->GetSpec(baseUri);
+    LOG(("OBJLC [%p]: Content policy denied load of %s (base %s)",
+         this, uri.get(), baseUri.get()));
+    return false;
   }
 
   return true;
 }
 
-bool nsObjectLoadingContent::CheckURILoad(nsIURI *aURI,
-                                          PRInt16 *aContentPolicy,
-                                          PRInt32 aContentPolicyType)
+bool
+nsObjectLoadingContent::CheckProcessPolicy(PRInt16 *aContentPolicy)
 {
-  nsIScriptSecurityManager* secMan = nsContentUtils::GetSecurityManager();
-  NS_ASSERTION(secMan, "No security manager!?");
+  if (!aContentPolicy) {
+    NS_NOTREACHED("Null out variable");
+    return false;
+  }
 
   nsCOMPtr<nsIContent> thisContent =
     do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
   NS_ASSERTION(thisContent, "Must be an instance of content");
 
-  nsCOMPtr<nsIURI> docBaseURI = thisContent->GetBaseURI();
-
   nsIDocument* doc = thisContent->OwnerDoc();
-  nsresult rv =
-    secMan->CheckLoadURIWithPrincipal(thisContent->NodePrincipal(), aURI, 0);
   
-  if (NS_FAILED(rv)) {
-    nsCAutoString uri;
-    nsCAutoString baseUri;
-    aURI->GetSpec(uri);
-    aURI->GetSpec(baseUri);
-    LOG(("OBJLC [%p]: CheckLoadURIWithPrincipal denied load of %s (base %s)",
-         this, uri.get(), baseUri.get()));
-    return false;
+  PRInt32 objectType;
+  switch (mType) {
+    case eType_Image:
+      objectType = nsIContentPolicy::TYPE_IMAGE;
+      break;
+    case eType_Document:
+      objectType = nsIContentPolicy::TYPE_DOCUMENT;
+      break;
+    case eType_Plugin:
+      objectType = nsIContentPolicy::TYPE_OBJECT;
+      break;
+    default:
+      NS_NOTREACHED("Calling checkProcessPolicy with a unloadable type");
+      return false;
   }
 
-  PRInt16 shouldLoad = nsIContentPolicy::ACCEPT; // default permit
-  rv = NS_CheckContentLoadPolicy(aContentPolicyType,
-                                 aURI,
+  *aContentPolicy = nsIContentPolicy::ACCEPT;
+  nsresult rv =
+    NS_CheckContentProcessPolicy(objectType,
+                                 mURI,
                                  doc->NodePrincipal(),
                                  static_cast<nsIImageLoadingContent*>(this),
                                  mContentType,
                                  nullptr, //extra
-                                 &shouldLoad,
+                                 aContentPolicy,
                                  nsContentUtils::GetContentPolicy(),
-                                 secMan);
+                                 nsContentUtils::GetSecurityManager());
   NS_ENSURE_SUCCESS(rv, false);
-  if (aContentPolicy) {
-    *aContentPolicy = shouldLoad;
-  }
-  if (NS_CP_REJECTED(shouldLoad)) {
-    nsCAutoString uri;
-    nsCAutoString baseUri;
-    aURI->GetSpec(uri);
-    aURI->GetSpec(baseUri);
-    LOG(("OBJLC [%p]: Content policy denied load of %s (base %s)",
-         this, uri.get(), baseUri.get()));
+
+  if (NS_CP_REJECTED(*aContentPolicy)) {
+    LOG(("OBJLC [%p]: CheckContentProcessPolicy rejected load", this));
     return false;
   }
+
   return true;
 }
 
 nsObjectLoadingContent::ParameterUpdateFlags
 nsObjectLoadingContent::UpdateObjectParameters()
 {
   nsCOMPtr<nsIContent> thisContent =
     do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
@@ -1591,50 +1595,43 @@ nsObjectLoadingContent::LoadObject(bool 
     NS_NOTREACHED("Loading with a channel, but state doesn't make sense");
     return NS_OK;
   }
 
   //
   // Security checks
   //
 
-  // NOTE For eType_Loading we'll try all three types, as we want to go ahead
-  //      with the channel if it could be any acceptable type. This type is
-  //      passed to OpenChannel() as the LoadType. We pass through LoadObject
-  //      again once the channel is opened and we're actually loading, so if
-  //      the final URI doesn't pass the now-known type, we'll abort.
-  PRInt32 policyType;
   if (mType != eType_Null) {
-    bool allowLoad = false;
     PRInt16 contentPolicy = nsIContentPolicy::ACCEPT;
-    PRUint32 caps = GetCapabilities();
-    bool supportImage = caps & eSupportImages;
-    bool supportDoc = (caps & eSupportDocuments) || (caps & eSupportSVG);
-    bool supportPlugin = caps & eSupportPlugins;
-    if (mType == eType_Image || (mType == eType_Loading && supportImage)) {
-      policyType = nsIContentPolicy::TYPE_IMAGE;
-      allowLoad = CheckObjectURIs(&contentPolicy, policyType);
+    bool allowLoad = false;
+    // We check load policy before opening a channel, and process policy before
+    // going ahead with any final-type load
+    if (mType == eType_Loading) {
+      nsCOMPtr<nsIScriptSecurityManager> secMan =
+        nsContentUtils::GetSecurityManager();
+      if (!secMan) {
+        NS_NOTREACHED("No security manager?");
+      } else {
+        rv = secMan->CheckLoadURIWithPrincipal(thisContent->NodePrincipal(),
+                                               mURI, 0);
+        allowLoad = NS_SUCCEEDED(rv) && CheckLoadPolicy(&contentPolicy);
+      }
+    } else {
+      allowLoad = CheckProcessPolicy(&contentPolicy);
     }
-    if (!allowLoad &&
-        (mType == eType_Document || (mType == eType_Loading && supportDoc))) {
-      contentPolicy = nsIContentPolicy::ACCEPT;
-      policyType = nsIContentPolicy::TYPE_SUBDOCUMENT;
-      allowLoad = CheckObjectURIs(&contentPolicy, policyType);
-    }
-    if (!allowLoad &&
-        (mType == eType_Plugin || (mType == eType_Loading && supportPlugin))) {
-      contentPolicy = nsIContentPolicy::ACCEPT;
-      policyType = nsIContentPolicy::TYPE_OBJECT;
-      allowLoad = CheckObjectURIs(&contentPolicy, policyType);
-    }
-
+    
     // Load denied, switch to fallback and set disabled/suppressed if applicable
     if (!allowLoad) {
+      LOG(("OBJLC [%p]: Load denied by policy", this));
       mType = eType_Null;
       if (contentPolicy == nsIContentPolicy::REJECT_TYPE) {
+        // XXX(johns) This is assuming that we were rejected by
+        //            nsContentBlocker, which rejects by type if permissions
+        //            reject plugins
         fallbackType = eFallbackUserDisabled;
       } else {
         fallbackType = eFallbackSuppressed;
       }
     }
   }
 
   // If we're a plugin but shouldn't start yet, load fallback with
@@ -1776,17 +1773,17 @@ nsObjectLoadingContent::LoadObject(bool 
         mSrcStreamLoading = true;
         rv = mFinalListener->OnStartRequest(mChannel, nullptr);
         mSrcStreamLoading = false;
       }
     }
     break;
     case eType_Loading:
       // If our type remains Loading, we need a channel to proceed
-      rv = OpenChannel(policyType);
+      rv = OpenChannel();
       if (NS_FAILED(rv)) {
         LOG(("OBJLC [%p]: OpenChannel returned failure (%u)", this, rv));
       }
     break;
     case eType_Null:
       // Handled below, silence compiler warnings
     break;
   };
@@ -1845,17 +1842,17 @@ nsObjectLoadingContent::CloseChannel()
       mFinalListener = nullptr;
     }
     mChannel = nullptr;
   }
   return NS_OK;
 }
 
 nsresult
-nsObjectLoadingContent::OpenChannel(PRInt32 aPolicyType)
+nsObjectLoadingContent::OpenChannel()
 {
   nsCOMPtr<nsIContent> thisContent = 
     do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
   NS_ASSERTION(thisContent, "must be a content");
   nsIDocument* doc = thisContent->OwnerDoc();
   NS_ASSERTION(doc, "No owner document?");
   NS_ASSERTION(!mInstanceOwner && !mInstantiating,
                "opening a new channel with already loaded content");
@@ -1872,17 +1869,17 @@ nsObjectLoadingContent::OpenChannel(PRIn
   nsCOMPtr<nsIChannel> chan;
   nsCOMPtr<nsIChannelPolicy> channelPolicy;
   nsCOMPtr<nsIContentSecurityPolicy> csp;
   rv = doc->NodePrincipal()->GetCsp(getter_AddRefs(csp));
   NS_ENSURE_SUCCESS(rv, rv);
   if (csp) {
     channelPolicy = do_CreateInstance("@mozilla.org/nschannelpolicy;1");
     channelPolicy->SetContentSecurityPolicy(csp);
-    channelPolicy->SetLoadType(aPolicyType);
+    channelPolicy->SetLoadType(nsIContentPolicy::TYPE_OBJECT);
   }
   rv = NS_NewChannel(getter_AddRefs(chan), mURI, nullptr, group, this,
                      nsIChannel::LOAD_CALL_CONTENT_SNIFFERS |
                      nsIChannel::LOAD_CLASSIFY_URI,
                      channelPolicy);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Referrer
--- a/content/base/src/nsObjectLoadingContent.h
+++ b/content/base/src/nsObjectLoadingContent.h
@@ -270,63 +270,49 @@ class nsObjectLoadingContent : public ns
      * @return Returns a bitmask of ParameterUpdateFlags values
      */
     ParameterUpdateFlags UpdateObjectParameters();
 
     void NotifyContentObjectWrapper();
 
     /**
      * Opens the channel pointed to by mURI into mChannel.
-     *
-     * @param aPolicyType The value to be passed to channelPolicy->SetLoadType
      */
-    nsresult OpenChannel(PRInt32 aPolicyType);
+    nsresult OpenChannel();
 
     /**
      * Closes and releases references to mChannel and, if opened, mFinalListener
      */
     nsresult CloseChannel();
 
     /**
      * If this object is allowed to play plugin content, or if it would display
      * click-to-play instead.
      * NOTE that this does not actually check if the object is a loadable plugin
      */
     bool ShouldPlay(FallbackType &aReason);
 
     /**
-     * Checks if a URI passes security checks and content policy, relative to
-     * the current document's principal
+     * Helper to check if our current URI passes policy
+     *
+     * @param aContentPolicy [out] The result of the content policy decision
      *
-     * @param aURI                The URI to consider
-     * @param aContentPolicy      [in/out] A pointer to the initial content
-     *                            policy, that will be updated to contain the
-     *                            final determined policy
-     * @param aContentPolicyType  The 'contentType' parameter passed to
-     *                            NS_CheckContentLoadPolicy
-     *
-     * @return true if this URI is acceptable for loading
+     * @return true if call succeeded and NS_CP_ACCEPTED(*aContentPolicy)
      */
-    bool CheckURILoad(nsIURI *aURI,
-                      PRInt16 *aContentPolicy,
-                      PRInt32 aContentPolicyType);
+    bool CheckLoadPolicy(PRInt16 *aContentPolicy);
 
     /**
-     * Checks if the current mURI and mBaseURI pass content policy and security
-     * checks for loading
+     * Helper to check if the object passes process policy. Assumes we have a
+     * final determined type.
      *
-     * @param aContentPolicy     [in/out] A pointer to the initial content
-     *                           policy, that will be updated to contain the
-     *                           final determined policy if a URL is rejected
-     * @param aContentPolicyType The 'contentType' parameter passed to
-     *                           NS_CheckContentLoadPolicy
+     * @param aContentPolicy [out] The result of the content policy decision
      *
-     * @return true if the URIs are acceptable for loading
+     * @return true if call succeeded and NS_CP_ACCEPTED(*aContentPolicy)
      */
-    bool CheckObjectURIs(PRInt16 *aContentPolicy, PRInt32 aContentPolicyType);
+    bool CheckProcessPolicy(PRInt16 *aContentPolicy);
 
     /**
      * Checks whether the given type is a supported document type
      *
      * NOTE Does not take content policy or capabilities into account
      */
     bool IsSupportedDocument(const nsCString& aType);