Bug 989584 - Allow sites to set window.opener to any value. r=peterv, a=sledru
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 03 Jun 2014 11:38:37 -0400
changeset 199407 065972d5ac215d0f5113c52dff4a97ec21aafdee
parent 199406 3253bb6f2a281521de0f7b1a9cf5ffd7c91a4dc9
child 199408 a132629caa5fb19ed4ae9e5fb6651a28d6f6d3dc
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, sledru
bugs989584
milestone31.0a2
Bug 989584 - Allow sites to set window.opener to any value. r=peterv, a=sledru
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
dom/base/test/test_setting_opener.html
dom/interfaces/base/nsIDOMWindow.idl
dom/webidl/Window.webidl
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -4245,23 +4245,17 @@ nsGlobalWindow::GetOwnPropertyNames(JSCo
     nameSpaceManager->EnumerateGlobalNames(EnumerateGlobalName, &closure);
   }
 }
 
 /* static */ bool
 nsGlobalWindow::IsChromeWindow(JSContext* aCx, JSObject* aObj)
 {
   // For now, have to deal with XPConnect objects here.
-  nsGlobalWindow* win;
-  nsresult rv = UNWRAP_OBJECT(Window, aObj, win);
-  if (NS_FAILED(rv)) {
-    nsCOMPtr<nsPIDOMWindow> piWin = do_QueryWrapper(aCx, aObj);
-    win = static_cast<nsGlobalWindow*>(piWin.get());
-  }
-  return win->IsChromeWindow();
+  return xpc::WindowOrNull(aObj)->IsChromeWindow();
 }
 
 nsIDOMOfflineResourceList*
 nsGlobalWindow::GetApplicationCache(ErrorResult& aError)
 {
   FORWARD_TO_INNER_OR_THROW(GetApplicationCache, (aError), aError, nullptr);
 
   if (!mApplicationCache) {
@@ -4377,19 +4371,19 @@ nsGlobalWindow::GetControllers(nsIContro
   ErrorResult rv;
   nsCOMPtr<nsIControllers> controllers = GetControllers(rv);
   controllers.forget(aResult);
 
   return rv.ErrorCode();
 }
 
 nsIDOMWindow*
-nsGlobalWindow::GetOpener(ErrorResult& aError)
-{
-  FORWARD_TO_OUTER_OR_THROW(GetOpener, (aError), aError, nullptr);
+nsGlobalWindow::GetOpenerWindow(ErrorResult& aError)
+{
+  FORWARD_TO_OUTER_OR_THROW(GetOpenerWindow, (aError), aError, nullptr);
 
   nsCOMPtr<nsPIDOMWindow> opener = do_QueryReferent(mOpener);
   if (!opener) {
     return nullptr;
   }
 
   // First, check if we were called from a privileged chrome script
   if (nsContentUtils::IsCallerChrome()) {
@@ -4418,81 +4412,115 @@ nsGlobalWindow::GetOpener(ErrorResult& a
         return opener;
       }
     }
   }
 
   return nullptr;
 }
 
+JS::Value
+nsGlobalWindow::GetOpener(JSContext* aCx, ErrorResult& aError)
+{
+  nsCOMPtr<nsIDOMWindow> opener = GetOpenerWindow(aError);
+  if (aError.Failed() || !opener) {
+    return JS::NullValue();
+  }
+
+  JS::Rooted<JS::Value> val(aCx);
+  aError = nsContentUtils::WrapNative(aCx, opener, &val);
+  return val;
+}
+
+NS_IMETHODIMP
+nsGlobalWindow::GetScriptableOpener(JSContext* aCx,
+                                    JS::MutableHandle<JS::Value> aOpener)
+{
+  ErrorResult rv;
+  aOpener.set(GetOpener(aCx, rv));
+
+  return rv.ErrorCode();
+}
+
 NS_IMETHODIMP
 nsGlobalWindow::GetOpener(nsIDOMWindow** aOpener)
 {
   ErrorResult rv;
-  nsCOMPtr<nsIDOMWindow> opener = GetOpener(rv);
+  nsCOMPtr<nsIDOMWindow> opener = GetOpenerWindow(rv);
   opener.forget(aOpener);
-
-  return rv.ErrorCode();
-}
-
-void
-nsGlobalWindow::SetOpener(nsIDOMWindow* aOpener, ErrorResult& aError)
+  return rv.ErrorCode();
+}
+
+void
+nsGlobalWindow::SetOpener(JSContext* aCx, JS::Handle<JS::Value> aOpener,
+                          ErrorResult& aError)
 {
   // Check if we were called from a privileged chrome script.  If not, and if
   // aOpener is not null, just define aOpener on our inner window's JS object,
   // wrapped into the current compartment so that for Xrays we define on the
   // Xray expando object, but don't set it on the outer window, so that it'll
   // get reset on navigation.  This is just like replaceable properties, but
   // we're not quite readonly.
-  if (aOpener && !nsContentUtils::IsCallerChrome()) {
-    // JS_WrapObject will outerize, so we don't care if aOpener is an inner.
-    nsCOMPtr<nsIGlobalObject> glob = do_QueryInterface(aOpener);
-    if (!glob) {
-      aError.Throw(NS_ERROR_UNEXPECTED);
-      return;
-    }
-
-    AutoJSContext cx;
-    JSAutoRequest ar(cx);
-    // Note we explicitly do NOT enter any particular compartment here; we want
-    // the caller compartment in cases when we have a caller, so that we define
-    // expandos on Xrays as needed.
-
-    JS::Rooted<JSObject*> otherObj(cx, glob->GetGlobalJSObject());
-    if (!otherObj) {
-      aError.Throw(NS_ERROR_UNEXPECTED);
-      return;
-    }
-
-    JS::Rooted<JSObject*> thisObj(cx, GetWrapperPreserveColor());
+  if (!aOpener.isNull() && !nsContentUtils::IsCallerChrome()) {
+    JS::Rooted<JSObject*> thisObj(aCx, GetWrapperPreserveColor());
     if (!thisObj) {
       aError.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
 
-    if (!JS_WrapObject(cx, &otherObj) ||
-        !JS_WrapObject(cx, &thisObj) ||
-        !JS_DefineProperty(cx, thisObj, "opener", otherObj, JSPROP_ENUMERATE,
+    if (!JS_WrapObject(aCx, &thisObj) ||
+        !JS_DefineProperty(aCx, thisObj, "opener", aOpener, JSPROP_ENUMERATE,
                            JS_PropertyStub, JS_StrictPropertyStub)) {
       aError.Throw(NS_ERROR_FAILURE);
     }
 
     return;
   }
 
-  SetOpenerWindow(aOpener, false);
+  if (!aOpener.isObjectOrNull()) {
+    // Chrome code trying to set some random value as opener
+    aError.Throw(NS_ERROR_INVALID_ARG);
+    return;
+  }
+
+  nsGlobalWindow* win = nullptr;
+  if (aOpener.isObject()) {
+    JSObject* unwrapped = js::CheckedUnwrap(&aOpener.toObject(),
+                                            /* stopAtOuter = */ false);
+    if (!unwrapped) {
+      aError.Throw(NS_ERROR_DOM_SECURITY_ERR);
+      return;
+    }
+
+    win = xpc::WindowOrNull(unwrapped);
+    if (!win) {
+      // Wasn't a window
+      aError.Throw(NS_ERROR_INVALID_ARG);
+      return;
+    }
+  }
+
+  SetOpenerWindow(win, false);
+}
+
+NS_IMETHODIMP
+nsGlobalWindow::SetScriptableOpener(JSContext* aCx,
+                                    JS::Handle<JS::Value> aOpener)
+{
+  ErrorResult rv;
+  SetOpener(aCx, aOpener, rv);
+
+  return rv.ErrorCode();
 }
 
 NS_IMETHODIMP
 nsGlobalWindow::SetOpener(nsIDOMWindow* aOpener)
 {
-  ErrorResult rv;
-  SetOpener(aOpener, rv);
-
-  return rv.ErrorCode();
+  SetOpenerWindow(aOpener, false);
+  return NS_OK;
 }
 
 void
 nsGlobalWindow::GetStatus(nsAString& aStatus, ErrorResult& aError)
 {
   FORWARD_TO_OUTER_OR_THROW(GetStatus, (aStatus, aError), aError, );
 
   aStatus = mStatus;
@@ -13526,23 +13554,17 @@ nsGlobalModalWindow::SetReturnValue(nsIV
   return NS_OK;
 }
 
 /* static */
 bool
 nsGlobalWindow::IsModalContentWindow(JSContext* aCx, JSObject* aGlobal)
 {
   // For now, have to deal with XPConnect objects here.
-  nsGlobalWindow* win;
-  nsresult rv = UNWRAP_OBJECT(Window, aGlobal, win);
-  if (NS_FAILED(rv)) {
-    nsCOMPtr<nsPIDOMWindow> piWin = do_QueryWrapper(aCx, aGlobal);
-    win = static_cast<nsGlobalWindow*>(piWin.get());
-  }
-  return win->IsModalContentWindow();
+  return xpc::WindowOrNull(aGlobal)->IsModalContentWindow();
 }
 
 NS_IMETHODIMP
 nsGlobalWindow::GetConsole(JSContext* aCx,
                            JS::MutableHandle<JS::Value> aConsole)
 {
   ErrorResult rv;
   nsRefPtr<Console> console = GetConsole(rv);
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -820,18 +820,22 @@ public:
   already_AddRefed<nsIDOMWindow> GetFrames(mozilla::ErrorResult& aError);
   uint32_t Length();
   already_AddRefed<nsIDOMWindow> GetTop(mozilla::ErrorResult& aError)
   {
     nsCOMPtr<nsIDOMWindow> top;
     aError = GetScriptableTop(getter_AddRefs(top));
     return top.forget();
   }
-  nsIDOMWindow* GetOpener(mozilla::ErrorResult& aError);
-  void SetOpener(nsIDOMWindow* aOpener, mozilla::ErrorResult& aError);
+protected:
+  nsIDOMWindow* GetOpenerWindow(mozilla::ErrorResult& aError);
+public:
+  JS::Value GetOpener(JSContext* aCx, mozilla::ErrorResult& aError);
+  void SetOpener(JSContext* aCx, JS::Handle<JS::Value> aOpener,
+                 mozilla::ErrorResult& aError);
   using nsIDOMWindow::GetParent;
   already_AddRefed<nsIDOMWindow> GetParent(mozilla::ErrorResult& aError);
   mozilla::dom::Element* GetFrameElement(mozilla::ErrorResult& aError);
   already_AddRefed<nsIDOMWindow> Open(const nsAString& aUrl,
                                       const nsAString& aName,
                                       const nsAString& aOptions,
                                       mozilla::ErrorResult& aError);
   mozilla::dom::Navigator* GetNavigator(mozilla::ErrorResult& aError);
--- a/dom/base/test/test_setting_opener.html
+++ b/dom/base/test/test_setting_opener.html
@@ -54,16 +54,35 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   function continueOpenerTest(win) {
     is(win.opener, window, "Navigating a window should have reset the opener we stashed on it temporarily");
     is(evalsb("win.opener", sb1), window,
        "Navigating a window should have reset the opener in sb1");
     is(evalsb("win.opener", sb2), window,
        "Navigating a window should have reset the opener in sb2");
 
+    win.opener = 5;
+    evalsb("win.opener = 5", sb1);
+    evalsb("win.opener = 5", sb2);
+    is(win.opener, 5, "Should be able to set an opener to a primitive");
+    is(evalsb("win.opener", sb1), 5,
+       "Should be able to set the opener to a primitive in a sandbox one");
+    is(evalsb("win.opener", sb2), 5,
+       "Should be able to set the opener to a primitive in a sandbox two");
+    win.location = "data:text/html,<script>opener.setTimeout(opener.continueOpenerTest2, 0, this);</" + "script>";
+  }
+
+  function continueOpenerTest2(win) {
+    is(win.opener, window,
+       "Navigating a window again should have reset the opener we stashed on it temporarily");
+    is(evalsb("win.opener", sb1), window,
+       "Navigating a window again should have reset the opener in sb1");
+    is(evalsb("win.opener", sb2), window,
+       "Navigating a window again should have reset the opener in sb2");
+
     win.opener = null;
     is(win.opener, null, "Should be able to set the opener to null");
     is(evalsb("win.opener", sb1), null,
        "Setting the opener to null should be visible in sb1");
     is(evalsb("win.opener", sb2), null,
        "Setting the opener to null should be visible in sb2");
 
     win.location = "data:text/html,Loaded";
--- a/dom/interfaces/base/nsIDOMWindow.idl
+++ b/dom/interfaces/base/nsIDOMWindow.idl
@@ -19,17 +19,17 @@ interface nsIVariant;
  * The nsIDOMWindow interface is the primary interface for a DOM
  * window object. It represents a single window object that may
  * contain child windows if the document in the window contains a
  * HTML frameset document or if the document contains iframe elements.
  *
  * @see <http://www.whatwg.org/html/#window>
  */
 
-[scriptable, uuid(8c115ab3-cf96-492c-850c-3b18056b45e2)]
+[scriptable, uuid(1b4a23a2-2ccf-4690-9da7-f3a7a8308381)]
 interface nsIDOMWindow : nsISupports
 {
   // the current browsing context
   readonly attribute nsIDOMWindow                       window;
 
   /* [replaceable] self */
   readonly attribute nsIDOMWindow                       self;
 
@@ -146,17 +146,21 @@ interface nsIDOMWindow : nsISupports
 
   %{C++
   inline nsresult GetParent(nsIDOMWindow **aWindow)
   {
     return GetRealParent(aWindow);
   }
   %}
 
-           attribute nsIDOMWindow                       opener;
+  [implicit_jscontext, binaryname(ScriptableOpener)]
+           attribute jsval                              opener;
+
+  [noscript, binaryname(Opener)]
+           attribute nsIDOMWindow                       openerWindow;
 
   /**
    * |frameElement| gets this window's <iframe> or <frame> element, if it has
    * one.
    *
    * When script reads this property (or when C++ calls
    * ScriptableFrameElement), we return |null| if the window is inside an
    * <iframe mozbrowser>.  In contrast, when C++ calls GetFrameElement, we
--- a/dom/webidl/Window.webidl
+++ b/dom/webidl/Window.webidl
@@ -48,17 +48,17 @@ typedef any Transferable;
   [Throws] void stop();
   [Throws, CrossOriginCallable] void focus();
   [Throws, CrossOriginCallable] void blur();
 
   // other browsing contexts
   [Replaceable, Throws, CrossOriginReadable] readonly attribute WindowProxy frames;
   [Replaceable, CrossOriginReadable] readonly attribute unsigned long length;
   [Unforgeable, Throws, CrossOriginReadable] readonly attribute WindowProxy top;
-  [Throws, CrossOriginReadable] attribute WindowProxy? opener;
+  [Throws, CrossOriginReadable] attribute any opener;
   //[Throws] readonly attribute WindowProxy parent;
   [Replaceable, Throws, CrossOriginReadable] readonly attribute WindowProxy? parent;
   [Throws] readonly attribute Element? frameElement;
   //[Throws] WindowProxy open(optional DOMString url = "about:blank", optional DOMString target = "_blank", [TreatNullAs=EmptyString] optional DOMString features = "", optional boolean replace = false);
   [Throws] WindowProxy? open(optional DOMString url = "", optional DOMString target = "", [TreatNullAs=EmptyString] optional DOMString features = "");
   // We think the indexed getter is a bug in the spec, it actually needs to live
   // on the WindowProxy
   //getter WindowProxy (unsigned long index);