Bug 738244 - Supporting DOM specific collection properties through xray wrappers; r=mrbkap

dom/base/nsDOMClassInfo.cpp

@@ -7197,7 +7197,8 @@ nsWindowSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
   // method on an interface that would let us just call into the
   // window code directly...
 
-  if (!ObjectIsNativeWrapper(cx, obj)) {
+  if (!ObjectIsNativeWrapper(cx, obj) ||
+      xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id)) {
     nsCOMPtr<nsIDocShellTreeNode> dsn(do_QueryInterface(win->GetDocShell()));
 
     int32_t count = 0;
@@ -9221,7 +9222,8 @@ nsHTMLDocumentSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
 
     JSAutoRequest ar(cx);
 
-    if (!ObjectIsNativeWrapper(cx, obj)) {
+    if (!ObjectIsNativeWrapper(cx, obj) ||
+        xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id)) {
       nsCOMPtr<nsISupports> result;
       nsWrapperCache *cache;
       nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result),
@@ -9313,23 +9315,20 @@ nsHTMLDocumentSH::GetProperty(nsIXPConnectWrappedNative *wrapper,
                               JSContext *cx, JSObject *obj, jsid id,
                               jsval *vp, bool *_retval)
 {
-  // For native wrappers, do not get random names on document
+  nsCOMPtr<nsISupports> result;
-  if (!ObjectIsNativeWrapper(cx, obj)) {
-    nsCOMPtr<nsISupports> result;
 
-    JSAutoRequest ar(cx);
+  JSAutoRequest ar(cx);
 
-    nsWrapperCache *cache;
+  nsWrapperCache *cache;
-    nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result), &cache);
+  nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result), &cache);
-    NS_ENSURE_SUCCESS(rv, rv);
+  NS_ENSURE_SUCCESS(rv, rv);
 
-    if (result) {
+  if (result) {
-      rv = WrapNative(cx, obj, result, cache, true, vp);
+    rv = WrapNative(cx, obj, result, cache, true, vp);
-      if (NS_SUCCEEDED(rv)) {
+    if (NS_SUCCEEDED(rv)) {
-        rv = NS_SUCCESS_I_DID_SOMETHING;
+      rv = NS_SUCCESS_I_DID_SOMETHING;
-      }
-      return rv;
     }
+    return rv;
   }
 
   return NS_OK;
@@ -9371,7 +9370,8 @@ nsHTMLFormElementSH::NewResolve(nsIXPConnectWrappedNative *wrapper,
 {
   // For native wrappers, do not resolve random names on form
   if ((!(JSRESOLVE_ASSIGNING & flags)) && JSID_IS_STRING(id) &&
-      !ObjectIsNativeWrapper(cx, obj)) {
+      (!ObjectIsNativeWrapper(cx, obj) ||
+       xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id))) {
     nsCOMPtr<nsIForm> form(do_QueryWrappedNative(wrapper, obj));
     nsCOMPtr<nsISupports> result;
     nsWrapperCache *cache;
@@ -9402,18 +9402,16 @@ nsHTMLFormElementSH::GetProperty(nsIXPConnectWrappedNative *wrapper,
 
   if (JSID_IS_STRING(id)) {
     // For native wrappers, do not get random names on form
-    if (!ObjectIsNativeWrapper(cx, obj)) {
+    nsCOMPtr<nsISupports> result;
-      nsCOMPtr<nsISupports> result;
+    nsWrapperCache *cache;
-      nsWrapperCache *cache;
 
-      FindNamedItem(form, id, getter_AddRefs(result), &cache);
+    FindNamedItem(form, id, getter_AddRefs(result), &cache);
 
-      if (result) {
+    if (result) {
-        // Wrap result, result can be either an element or a list of
+      // Wrap result, result can be either an element or a list of
-        // elements
+      // elements
-        nsresult rv = WrapNative(cx, obj, result, cache, true, vp);
+      nsresult rv = WrapNative(cx, obj, result, cache, true, vp);
-        return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
+      return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
-      }
     }
   } else {
     int32_t n = GetArrayIndexFromId(cx, id);

dom/base/nsDOMClassInfo.h

@@ -132,6 +132,11 @@ public:
    * dealing with document.domain, it's possible to end up in a scriptable
    * helper with a wrapper, even though we should be treating the lookup as a
    * transparent one.
+   *
+   * Note: So ObjectIsNativeWrapper(cx, obj) check usually means "through xray
+   * wrapper this part is not visible" while combined with
+   * || xpc::WrapperFactory::XrayWrapperNotShadowing(obj) it means "through
+   * xray wrapper it is visible only if it does not hide any native property."
    */
   static bool ObjectIsNativeWrapper(JSContext* cx, JSObject* obj);
 

js/xpconnect/tests/chrome/Makefile.in

@@ -32,6 +32,7 @@ MOCHITEST_CHROME_FILES = \
 		test_bug679861.xul \
 		test_bug706301.xul \
 		test_bug726949.xul \
+		test_bug738244.xul \
 		test_bug743843.xul \
 		test_bug760076.xul \
 		test_bug760109.xul \

js/xpconnect/tests/chrome/test_bug738244.xul

@@ -0,0 +1,50 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=533596
+-->
+<window title="Mozilla Bug 533596"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+
+  <iframe src="http://example.org/tests/js/xpconnect/tests/mochitest/file_bug738244.html"
+          onload="xrayTest(this)">
+  </iframe>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+
+      SimpleTest.waitForExplicitFinish();
+
+      function xrayTest(ifr) {
+        var win = ifr.contentWindow;
+        var doc = ifr.contentDocument;
+
+        doc.getElementById = 42;
+        is(doc.getElementById, 42,
+           "Native property cannot be shadowed on the xray");
+
+        is(doc.form1.name, "form1",
+           "Form elements cannot be found by name on the document through xray");
+
+        is(doc.form1.input1.name, "input1",
+           "Input element cannot be found by name on a form element through xray");
+
+        is(typeof doc.form1.appendChild, "function",
+           "Input element shadows native property with its name through xray");
+
+        is(win.frame1.name, "frame1",
+           "IFrames cannot be found by name on the window through xray");
+
+        SimpleTest.finish();
+      }
+
+  ]]></script>
+</window>

js/xpconnect/tests/mochitest/Makefile.in

@@ -72,6 +72,7 @@ MOCHITEST_FILES =	bug500931_helper.html \
 		file_empty.html \
 		file_documentdomain.html \
 		test_lookupMethod.html \
+		file_bug738244.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES	= \

js/xpconnect/tests/mochitest/file_bug738244.html

@@ -0,0 +1,10 @@
+<html>
+<body>
+<form name="form1">
+  <input name="input1" />
+  <input name="appendChild" />
+</form>
+<iframe name="frame1">
+</iframe>
+</body>
+</html>

js/xpconnect/wrappers/WrapperFactory.cpp

@@ -625,6 +625,14 @@ WrapperFactory::WrapForSameCompartmentXray(JSContext *cx, JSObject *obj)
     return wrapperObj;
 }
 
+
+bool
+WrapperFactory::XrayWrapperNotShadowing(JSObject *wrapper, jsid id)
+{
+    ResolvingId *rid = ResolvingId::getResolvingIdFromWrapper(wrapper);
+    return rid->isXrayShadowing(id);
+}
+
 /*
  * Calls to JS_TransplantObject* should go through these helpers here so that
  * waivers get fixed up properly.

js/xpconnect/wrappers/WrapperFactory.h

@@ -93,6 +93,9 @@ class WrapperFactory {
 
     // Wrap a same-compartment object for Xray inspection.
     static JSObject *WrapForSameCompartmentXray(JSContext *cx, JSObject *obj);
+
+    // Returns true if the wrapper is in not shadowing mode for the id.
+    static bool XrayWrapperNotShadowing(JSObject *wrapper, jsid id);
 };
 
 extern js::DirectWrapper XrayWaiver;

js/xpconnect/wrappers/XrayWrapper.cpp

@@ -279,6 +279,68 @@ createHolder(JSContext *cx, JSObject *wrappedNative, JSObject *parent)
 
 using namespace XrayUtils;
 
+ResolvingId::ResolvingId(JSObject *wrapper, jsid id)
+    : mId(id),
+    mHolder(getHolderObject(wrapper)),
+    mPrev(getResolvingId(mHolder)),
+    mXrayShadowing(false)
+{
+    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, js::PrivateValue(this));
+}
+
+ResolvingId::~ResolvingId()
+{
+    MOZ_ASSERT(getResolvingId(mHolder) == this, "unbalanced ResolvingIds");
+    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, js::PrivateValue(mPrev));
+}
+
+bool
+ResolvingId::isXrayShadowing(jsid id)
+{
+    if (!mXrayShadowing)
+        return false;
+
+    return mId == id;
+}
+
+bool
+ResolvingId::isResolving(jsid id)
+{
+    for (ResolvingId *cur = this; cur; cur = cur->mPrev) {
+        if (cur->mId == id)
+            return true;
+    }
+
+    return false;
+}
+
+ResolvingId *
+ResolvingId::getResolvingId(JSObject *holder)
+{
+    MOZ_ASSERT(strcmp(JS_GetClass(holder)->name, "NativePropertyHolder") == 0);
+    return (ResolvingId *)js::GetReservedSlot(holder, JSSLOT_RESOLVING).toPrivate();
+}
+
+JSObject *
+ResolvingId::getHolderObject(JSObject *wrapper)
+{
+    return &js::GetProxyExtra(wrapper, 0).toObject();
+}
+
+ResolvingId *
+ResolvingId::getResolvingIdFromWrapper(JSObject *wrapper)
+{
+    return getResolvingId(getHolderObject(wrapper));
+}
+
+class ResolvingIdDummy
+{
+public:
+    ResolvingIdDummy(JSObject *wrapper, jsid id)
+    {
+    }
+};
+
 class XPCWrappedNativeXrayTraits
 {
 public:
@@ -298,30 +360,18 @@ public:
     }
     static JSObject* getInnerObject(JSObject *wrapper);
 
-    class ResolvingId
-    {
-    public:
-        ResolvingId(JSObject *holder, jsid id);
-        ~ResolvingId();
-
-    private:
-        friend class XPCWrappedNativeXrayTraits;
-
-        jsid mId;
-        JSObject *mHolder;
-        ResolvingId *mPrev;
-    };
     static bool isResolving(JSContext *cx, JSObject *holder, jsid id);
 
+    static bool resolveDOMCollectionProperty(JSContext *cx, JSObject *wrapper, JSObject *holder,
+                                             jsid id, bool set, PropertyDescriptor *desc);
+
+    typedef ResolvingId ResolvingIdImpl;
+
 private:
     static JSObject* getHolderObject(JSObject *wrapper)
     {
         return &js::GetProxyExtra(wrapper, 0).toObject();
     }
-    static ResolvingId* getResolvingId(JSObject *holder)
-    {
-        return (ResolvingId *)js::GetReservedSlot(holder, JSSLOT_RESOLVING).toPrivate();
-    }
 };
 
 class ProxyXrayTraits
@@ -346,18 +396,13 @@ public:
         return &js::GetProxyPrivate(wrapper).toObject();
     }
 
-    class ResolvingId
-    {
-    public:
-        ResolvingId(JSObject *holder, jsid id)
-        {
-        }
-    };
     static bool isResolving(JSContext *cx, JSObject *holder, jsid id)
     {
-      return false;
+        return false;
     }
 
+    typedef ResolvingIdDummy ResolvingIdImpl;
+
 private:
     static JSObject* getHolderObject(JSContext *cx, JSObject *wrapper,
                                      bool createHolder)
@@ -395,18 +440,13 @@ public:
         return &js::GetProxyPrivate(wrapper).toObject();
     }
 
-    class ResolvingId
-    {
-    public:
-        ResolvingId(JSObject *holder, jsid id)
-        {
-        }
-    };
     static bool isResolving(JSContext *cx, JSObject *holder, jsid id)
     {
-      return false;
+        return false;
     }
 
+    typedef ResolvingIdDummy ResolvingIdImpl;
+
 private:
     static JSObject* getHolderObject(JSContext *cx, JSObject *wrapper,
                                      bool createHolder)
@@ -472,30 +512,11 @@ XPCWrappedNativeXrayTraits::getInnerObject(JSObject *wrapper)
     return GetWrappedNativeObjectFromHolder(getHolderObject(wrapper));
 }
 
-XPCWrappedNativeXrayTraits::ResolvingId::ResolvingId(JSObject *wrapper, jsid id)
-  : mId(id),
-    mHolder(getHolderObject(wrapper)),
-    mPrev(getResolvingId(mHolder))
-{
-    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, PrivateValue(this));
-}
-
-XPCWrappedNativeXrayTraits::ResolvingId::~ResolvingId()
-{
-    NS_ASSERTION(getResolvingId(mHolder) == this, "unbalanced ResolvingIds");
-    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, PrivateValue(mPrev));
-}
-
 bool
 XPCWrappedNativeXrayTraits::isResolving(JSContext *cx, JSObject *holder,
                                         jsid id)
 {
-    for (ResolvingId *cur = getResolvingId(holder); cur; cur = cur->mPrev) {
+    return ResolvingId::getResolvingId(holder)->isResolving(id);
-        if (cur->mId == id)
-            return true;
-    }
-
-    return false;
 }
 
 // Some DOM objects have shared properties that don't have an explicit
@@ -548,6 +569,71 @@ holder_set(JSContext *cx, JSHandleObject wrapper_, JSHandleId id, JSBool strict,
     return true;
 }
 
+class AutoSetWrapperNotShadowing
+{
+public:
+    AutoSetWrapperNotShadowing(JSObject *wrapper MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+    {
+        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+        MOZ_ASSERT(wrapper);
+        mResolvingId = ResolvingId::getResolvingIdFromWrapper(wrapper);
+        MOZ_ASSERT(mResolvingId);
+        mResolvingId->mXrayShadowing = true;
+    }
+
+    ~AutoSetWrapperNotShadowing()
+    {
+        mResolvingId->mXrayShadowing = false;
+    }
+
+private:
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+    ResolvingId *mResolvingId;
+};
+
+// This is called after the resolveNativeProperty could not find any property
+// with the given id. At this point we can check for DOM specific collections
+// like document["formName"] because we already know that it is not shadowing
+// any native property.
+bool
+XPCWrappedNativeXrayTraits::resolveDOMCollectionProperty(JSContext *cx, JSObject *wrapper,
+                                                         JSObject *holder, jsid id, bool set,
+                                                         PropertyDescriptor *desc)
+{
+    // If we are not currently resolving this id and resolveNative is called
+    // we don't do anything. (see defineProperty in case of shadowing is forbidden).
+    ResolvingId *rid = ResolvingId::getResolvingId(holder);
+    if (!rid || rid->mId != id)
+        return true;
+
+    XPCWrappedNative *wn = GetWrappedNativeFromHolder(holder);
+    if (!NATIVE_HAS_FLAG(wn, WantNewResolve))
+        return true;
+
+    // Setting the current ResolvingId in non-shadowing mode. So for this id
+    // Xray won't ignore DOM specific collection properties temporarily.
+    AutoSetWrapperNotShadowing asw(wrapper);
+
+    bool retval = true;
+    JSObject *pobj = NULL;
+    unsigned flags = (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED;
+    nsresult rv = wn->GetScriptableInfo()->GetCallback()->NewResolve(wn, cx, wrapper, id,
+                                                                     flags, &pobj, &retval);
+    if (NS_FAILED(rv)) {
+        if (retval)
+            XPCThrower::Throw(rv, cx);
+        return false;
+    }
+
+    if (pobj && !JS_GetPropertyDescriptorById(cx, holder, id,
+                                              JSRESOLVE_QUALIFIED, desc))
+    {
+        return false;
+    }
+
+    return true;
+}
+
 bool
 XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapper,
                                                   JSObject *holder, jsid id, bool set,
@@ -562,11 +648,13 @@ XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapp
     // This will do verification and the method lookup for us.
     XPCCallContext ccx(JS_CALLER, cx, wnObject, nullptr, id);
 
-    // There are no native numeric properties, so we can shortcut here. We will not
+    // There are no native numeric properties, so we can shortcut here. We will
-    // find the property.
+    // not find the property. However we want to support non shadowing dom
+    // specific collection properties like window.frames, so we still have to
+    // check for those.
     if (!JSID_IS_STRING(id)) {
         /* Not found */
-        return true;
+        return resolveDOMCollectionProperty(cx, wrapper, holder, id, set, desc);
     }
 
     XPCNativeInterface *iface;
@@ -576,7 +664,7 @@ XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapp
         !(iface = ccx.GetInterface()) ||
         !(member = ccx.GetMember())) {
         /* Not found */
-        return true;
+        return resolveDOMCollectionProperty(cx, wrapper, holder, id, set, desc);
     }
 
     desc->obj = holder;
@@ -1235,7 +1323,7 @@ XrayWrapper<Base, Traits>::getPropertyDescriptor(JSContext *cx, JSObject *wrappe
     if (!this->enter(cx, wrapper, id, action, &status))
         return status;
 
-    typename Traits::ResolvingId resolving(wrapper, id);
+    typename Traits::ResolvingIdImpl resolving(wrapper, id);
 
     // Redirect access straight to the wrapper if we should be transparent.
     if (XrayUtils::IsTransparent(cx, wrapper)) {
@@ -1334,7 +1422,7 @@ XrayWrapper<Base, Traits>::getOwnPropertyDescriptor(JSContext *cx, JSObject *wra
     if (!this->enter(cx, wrapper, id, action, &status))
         return status;
 
-    typename Traits::ResolvingId resolving(wrapper, id);
+    typename Traits::ResolvingIdImpl resolving(wrapper, id);
 
     // NB: Nothing we do here acts on the wrapped native itself, so we don't
     // enter our policy.

js/xpconnect/wrappers/XrayWrapper.h

@@ -7,6 +7,7 @@
 
 #include "jsapi.h"
 #include "jswrapper.h"
+#include "mozilla/GuardObjects.h"
 
 // Xray wrappers re-resolve the original native properties on the native
 // object and always directly access to those properties.
@@ -100,4 +101,30 @@ public:
 };
 
 extern SandboxProxyHandler sandboxProxyHandler;
+
+class AutoSetWrapperNotShadowing;
+class XPCWrappedNativeXrayTraits;
+
+class ResolvingId {
+public:
+    ResolvingId(JSObject *wrapper, jsid id);
+    ~ResolvingId();
+
+    bool isXrayShadowing(jsid id);
+    bool isResolving(jsid id);
+    static ResolvingId* getResolvingId(JSObject *holder);
+    static JSObject* getHolderObject(JSObject *wrapper);
+    static ResolvingId *getResolvingIdFromWrapper(JSObject *wrapper);
+
+private:
+    friend class AutoSetWrapperNotShadowing;
+    friend class XPCWrappedNativeXrayTraits;
+
+    jsid mId;
+    JSObject *mHolder;
+    ResolvingId *mPrev;
+    bool mXrayShadowing;
+};
+
 }
+