Bug 739796 - Make js_TransplantObjectWithWrapper and GetSameCompartmentSecurityWrapper play nicely together. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Thu, 05 Apr 2012 12:21:12 -0700
changeset 91100 850547d0b635
parent 91099 6a4c590bf1d2
child 91101 fe8ffd7166eb
push id22414
push usermbrubeck@mozilla.com
push date2012-04-06 18:15 +0000
treeherdermozilla-central@a402c2064466 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs739796
milestone14.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 739796 - Make js_TransplantObjectWithWrapper and GetSameCompartmentSecurityWrapper play nicely together. r=mrbkap
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/tests/chrome/test_wrappers.xul
js/xpconnect/tests/mochitest/chrome_wrappers_helper.html
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1664,36 +1664,75 @@ XPCWrappedNative::ReparentWrapperIfFound
 
                 JS_SetPrivate(flat, nsnull);
 
                 JSObject *propertyHolder =
                     JS_NewObjectWithGivenProto(ccx, NULL, NULL, aNewParent);
                 if (!propertyHolder || !JS_CopyPropertiesFrom(ccx, propertyHolder, flat))
                     return NS_ERROR_OUT_OF_MEMORY;
 
+                // Before proceeding, eagerly create any same-compartment security wrappers
+                // that the object might have. This forces us to take the 'WithWrapper' path
+                // while transplanting that handles this stuff correctly.
+                {
+                    JSAutoEnterCompartment innerAC;
+                    if (!innerAC.enter(ccx, aOldScope->GetGlobalJSObject()) ||
+                        !wrapper->GetSameCompartmentSecurityWrapper(ccx))
+                        return NS_ERROR_FAILURE;
+                }
+
                 JSObject *ww = wrapper->GetWrapper();
                 if (ww) {
                     JSObject *newwrapper;
                     if (xpc::WrapperFactory::IsLocationObject(flat)) {
                         newwrapper = xpc::WrapperFactory::WrapLocationObject(ccx, newobj);
                         if (!newwrapper)
                             return NS_ERROR_FAILURE;
                     } else {
                         NS_ASSERTION(wrapper->NeedsSOW(), "weird wrapper wrapper");
                         newwrapper = xpc::WrapperFactory::WrapSOWObject(ccx, newobj);
                         if (!newwrapper)
                             return NS_ERROR_FAILURE;
                     }
 
+                    // Ok, now we do the special object-plus-wrapper transplant.
+                    //
+                    // This is some pretty serious brain surgery.
+                    //
+                    // In the case where we wrap a Location object from a same-
+                    // origin compartment, we actually want our cross-compartment
+                    // wrapper to point to the same-compartment wrapper in the
+                    // other compartment. This double-wrapping allows expandos to
+                    // be shared. So our wrapping callback (in WrapperFactory.cpp)
+                    // calls XPCWrappedNative::GetSameCompartmentSecurityWrapper
+                    // before wrapping same-origin Location objects.
+                    //
+                    // This normally works fine, but gets tricky here.
+                    // js_TransplantObjectWithWrapper needs to update the old
+                    // same-compartment security wrapper to be a cross-compartment
+                    // wrapper to the newly transplanted object. So it needs to go
+                    // through the aforementioned double-wrapping mechanism.
+                    // But during the call, things aren't really in a consistent
+                    // state, because mFlatJSObject hasn't yet been updated to
+                    // point to the object in the new compartment.
+                    //
+                    // So we need to cache the new same-compartment security
+                    // wrapper on the XPCWN before the call, so that
+                    // GetSameCompartmentSecurityWrapper can return early before
+                    // getting confused. Hold your breath.
+                    JSObject *wwsaved = ww;
+                    wrapper->SetWrapper(newwrapper);
                     ww = js_TransplantObjectWithWrapper(ccx, flat, ww, newobj,
                                                         newwrapper);
-                    if (!ww)
+                    if (!ww) {
+                        wrapper->SetWrapper(wwsaved);
                         return NS_ERROR_FAILURE;
+                    }
+
                     flat = newobj;
-                    wrapper->SetWrapper(ww);
                 } else {
                     flat = JS_TransplantObject(ccx, flat, newobj);
                     if (!flat)
                         return NS_ERROR_FAILURE;
                 }
 
                 wrapper->mFlatJSObject = flat;
                 if (cache)
@@ -2168,28 +2207,32 @@ XPCWrappedNative::InitTearOffJSObject(XP
 
 JSObject*
 XPCWrappedNative::GetSameCompartmentSecurityWrapper(JSContext *cx)
 {
     // Grab the current state of affairs.
     JSObject *flat = GetFlatJSObject();
     JSObject *wrapper = GetWrapper();
 
+    // If we already have a wrapper, it must be what we want.
+    //
+    // NB: This must come before anything below because of some trickiness
+    // with brain transplants. See the "pretty serious brain surgery" comment
+    // in ReparentWrapperIfFound.
+    if (wrapper)
+        return wrapper;
+
     // Chrome callers don't need same-compartment security wrappers.
     JSCompartment *cxCompartment = js::GetContextCompartment(cx);
     MOZ_ASSERT(cxCompartment == js::GetObjectCompartment(flat));
     if (xpc::AccessCheck::isChrome(cxCompartment)) {
         MOZ_ASSERT(wrapper == NULL);
         return flat;
     }
 
-    // If we already have a wrapper, it must be what we want.
-    if (wrapper)
-        return wrapper;
-
     // Check the possibilities. Note that we need to check for null in each
     // case in order to distinguish between the 'no need for wrapper' and
     // 'wrapping failed' cases.
     if (xpc::WrapperFactory::IsLocationObject(flat)) {
         wrapper = xpc::WrapperFactory::WrapLocationObject(cx, flat);
         if (!wrapper)
             return NULL;
     } else if (NeedsSOW()) {
--- a/js/xpconnect/tests/chrome/test_wrappers.xul
+++ b/js/xpconnect/tests/chrome/test_wrappers.xul
@@ -21,17 +21,17 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   /** Test for Bug 533596 **/
 
   function go() {
     var win = $('ifr').contentWindow;
     var utils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
                       .getInterface(Components.interfaces.nsIDOMWindowUtils);
     is(utils.getClassName(window), "Proxy", "our window is wrapped correctly")
-    is(utils.getClassName(location), "Proxy", "our location is wrapped correctly")
+    is(utils.getClassName(location), "Location", "chrome doesn't have location wrappers")
     is(utils.getClassName(win), "Proxy", "win is an Proxy");
     is(utils.getClassName(win.location), "Proxy", "deep wrapping works");
     is(win.location.href, "http://example.org/tests/js/xpconnect/tests/mochitest/chrome_wrappers_helper.html",
        "can still get strings out");
 
     var unsafeWin = win.wrappedJSObject;
     is(utils.getClassName(unsafeWin), "Proxy", "can get a Proxy");
     is(utils.getClassName(unsafeWin.location), "Proxy", "deep wrapping works");
--- a/js/xpconnect/tests/mochitest/chrome_wrappers_helper.html
+++ b/js/xpconnect/tests/mochitest/chrome_wrappers_helper.html
@@ -15,16 +15,19 @@
             }
             function run_test(ok, xpcnw, sjow) {
                 // both wrappers should point to our window: XOW
                 check_wrapper(ok, ok, "Proxy", "functions are wrapped properly")
                 check_parent(ok, ok, window, "ok is parented correctly");
                 check_wrapper(ok, xpcnw, "Proxy", "XPCNWs are transformed correctly");
                 check_wrapper(ok, sjow, "Proxy", "SJOWs are transformed correctly");
 
+                check_wrapper(ok, window.location, "Proxy",
+                              "Content needs a same-compartment security wrappers around location");
+
                 ok(defprop1 === 1, "defprop1 exists");
                 window.defprop1 = 2;
                 ok(defprop1 === 2, "defprop1 is properly writable");
 
                 // defprop2 = {}; disabled because the test doesn't work
             }
         </script>
     </head>