Fix for bug 838692 - Navigating named targets from sandboxed iframes (r=smaug)
authorBob Owen <bobowencode@gmail.com>
Wed, 24 Apr 2013 18:13:25 +0100
changeset 135368 c873354a9242
parent 135367 35b943a379dd
child 135369 65cd5eac8b2e
push id24839
push useremorley@mozilla.com
push dateTue, 18 Jun 2013 10:52:57 +0000
treeherdermozilla-central@2bcc1f75895a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs838692
milestone24.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
Fix for bug 838692 - Navigating named targets from sandboxed iframes (r=smaug)
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3082,24 +3082,28 @@ nsDocShell::FindItemWithName(const PRUni
     NS_ENSURE_ARG_POINTER(_retval);
 
     // If we don't find one, we return NS_OK and a null result
     *_retval = nullptr;
 
     if (!*aName)
         return NS_OK;
 
-    if (!aRequestor)
-    {
-        nsCOMPtr<nsIDocShellTreeItem> foundItem;
+    if (aRequestor) {
+        // If aRequestor is not null we don't need to check special names, so
+        // just hand straight off to the search by actual name function.
+        return DoFindItemWithName(aName, aRequestor, aOriginalRequestor,
+                                  _retval);
+    } else {
 
         // This is the entry point into the target-finding algorithm.  Check
         // for special names.  This should only be done once, hence the check
         // for a null aRequestor.
 
+        nsCOMPtr<nsIDocShellTreeItem> foundItem;
         nsDependentString name(aName);
         if (name.LowerCaseEqualsLiteral("_self")) {
             foundItem = this;
         }
         else if (name.LowerCaseEqualsLiteral("_blank"))
         {
             // Just return null.  Caller must handle creating a new window with
             // a blank name himself.
@@ -3138,26 +3142,29 @@ nsDocShell::FindItemWithName(const PRUni
                 // Note: _content should always exist.  If we don't have one
                 // hanging off the treeowner, just create a named window....
                 // so don't return here, in case we did that and can now find
                 // it.                
                 // XXXbz should we be using |root| instead of creating
                 // a new window?
             }
 #endif
+        } else {
+            // Do the search for item by an actual name.
+            DoFindItemWithName(aName, aRequestor, aOriginalRequestor,
+                               getter_AddRefs(foundItem));
         }
 
         if (foundItem && !CanAccessItem(foundItem, aOriginalRequestor)) {
             foundItem = nullptr;
         }
 
+        // DoFindItemWithName only returns active items and we don't check if
+        // the item is active for the special cases.
         if (foundItem) {
-            // We return foundItem here even if it's not an active
-            // item since all the names we've dealt with so far are
-            // special cases that we won't bother looking for further.
 
             // If our document is sandboxed, we need to do some extra checks.
             uint32_t sandboxFlags = 0;
 
             nsCOMPtr<nsIDocument> doc = do_GetInterface(aOriginalRequestor);
 
             if (doc) {
                 sandboxFlags = doc->GetSandboxFlags();
@@ -3169,27 +3176,24 @@ nsDocShell::FindItemWithName(const PRUni
 
                 // Is the found item not a top level browsing context and not ourself ?
                 nsCOMPtr<nsIDocShellTreeItem> selfAsItem = static_cast<nsIDocShellTreeItem *>(this);
                 if (foundItem != root && foundItem != selfAsItem) {
                     // Are we an ancestor of the foundItem ?
                     bool isAncestor = false;
 
                     nsCOMPtr<nsIDocShellTreeItem> parentAsItem;
-                    GetSameTypeParent(getter_AddRefs(parentAsItem));
-
+                    foundItem->GetSameTypeParent(getter_AddRefs(parentAsItem));
                     while (parentAsItem) {
-                        nsCOMPtr<nsIDocShellTreeItem> tmp;
-                        parentAsItem->GetParent(getter_AddRefs(tmp));
-
-                        if (tmp && tmp == selfAsItem) {
+                        if (parentAsItem == selfAsItem) {
                             isAncestor = true;
                             break;
                         }
-                        parentAsItem = tmp;
+                        nsCOMPtr<nsIDocShellTreeItem> tmp = parentAsItem;
+                        tmp->GetSameTypeParent(getter_AddRefs(parentAsItem));
                     }
 
                     if (!isAncestor) {
                         // No, we are not an ancestor and our document is
                         // sandboxed, we can't allow this.
                         foundItem = nullptr;
                     }
                 } else {
@@ -3207,22 +3211,27 @@ nsDocShell::FindItemWithName(const PRUni
                             break;
                         }
                         tmp->GetParent(getter_AddRefs(tmp));
                     }
                 }
             }
 
             foundItem.swap(*_retval);
-            return NS_OK;
-        }
-    }
-
-    // Keep looking
-        
+        }
+        return NS_OK;
+    }
+}
+
+nsresult
+nsDocShell::DoFindItemWithName(const PRUnichar* aName,
+                               nsISupports* aRequestor,
+                               nsIDocShellTreeItem* aOriginalRequestor,
+                               nsIDocShellTreeItem** _retval)
+{
     // First we check our name.
     if (mName.Equals(aName) && ItemIsActive(this) &&
         CanAccessItem(this, aOriginalRequestor)) {
         NS_ADDREF(*_retval = this);
         return NS_OK;
     }
 
     // This QI may fail, but the places where we want to compare, comparing
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -869,16 +869,23 @@ protected:
 private:
     nsCString         mForcedCharset;
     nsCString         mParentCharset;
     nsTObserverArray<nsWeakPtr> mPrivacyObservers;
     nsTObserverArray<nsWeakPtr> mReflowObservers;
     int32_t           mParentCharsetSource;
     nsCString         mOriginalUriString;
 
+    // Separate function to do the actual name (i.e. not _top, _self etc.)
+    // searching for FindItemWithName.
+    nsresult DoFindItemWithName(const PRUnichar* aName,
+                                nsISupports* aRequestor,
+                                nsIDocShellTreeItem* aOriginalRequestor,
+                                nsIDocShellTreeItem** _retval);
+
 #ifdef DEBUG
     // We're counting the number of |nsDocShells| to help find leaks
     static unsigned long gNumberOfDocShells;
 #endif /* DEBUG */
 
 public:
     class InterfaceRequestorProxy : public nsIInterfaceRequestor {
     public: