Bug 965898 - Implement proper behavior for [[Enumerate]] And [[OwnPropertyKeys]]. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Wed, 30 Jul 2014 12:23:02 -0700
changeset 196843 cf2bc60412d679ecd1bef1f6fe4bd977e09348c7
parent 196842 103615c82485bf1006a32df461206a5dd036a9f5
child 196844 4209175a1f0a7cbda7caf41fe41bdc8021d5fedd
push id46984
push userbobbyholley@gmail.com
push dateWed, 30 Jul 2014 19:24:00 +0000
treeherdermozilla-inbound@22e1b7b69877 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs965898
milestone34.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 965898 - Implement proper behavior for [[Enumerate]] And [[OwnPropertyKeys]]. r=gabor
js/xpconnect/tests/mochitest/test_bug862380.html
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/FilteringWrapper.cpp
js/xpconnect/wrappers/FilteringWrapper.h
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/js/xpconnect/tests/mochitest/test_bug862380.html
+++ b/js/xpconnect/tests/mochitest/test_bug862380.html
@@ -15,17 +15,16 @@ https://bugzilla.mozilla.org/show_bug.cg
   function go() {
     checkNotEnumerable($('ifr').contentWindow);
     checkNotEnumerable($('ifr').contentWindow.location);
     SimpleTest.finish();
   }
 
   function checkNotEnumerable(obj) {
     try {
-      is(Object.getOwnPropertyNames(obj).length, 0, "Object.getOwnPropertyNames gives empty array");
       is(Object.keys(obj).length, 0, "Object.keys gives empty array");
       for (var i in obj)
         ok(false, "Enumerated something: " + i);
     } catch (e) {
       ok(false, "threw: " + e);
     }
   }
 
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -149,25 +149,23 @@ IsWindow(const char *name)
 
 bool
 AccessCheck::isCrossOriginAccessPermitted(JSContext *cx, JSObject *wrapperArg, jsid idArg,
                                           Wrapper::Action act)
 {
     if (act == Wrapper::CALL)
         return false;
 
+    if (act == Wrapper::ENUMERATE)
+        return true;
+
     RootedId id(cx, idArg);
     RootedObject wrapper(cx, wrapperArg);
     RootedObject obj(cx, Wrapper::wrappedObject(wrapper));
 
-    // For XOWs, we generally want to deny enumerate-like operations, but fail
-    // silently (see CrossOriginAccessiblePropertiesOnly::deny).
-    if (act == Wrapper::ENUMERATE)
-        return false;
-
     const char *name;
     const js::Class *clasp = js::GetObjectClass(obj);
     MOZ_ASSERT(!XrayUtils::IsXPCWNHolderClass(Jsvalify(clasp)), "shouldn't have a holder here");
     if (clasp->ext.innerObject)
         name = "Window";
     else
         name = clasp->name;
 
--- a/js/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/xpconnect/wrappers/FilteringWrapper.cpp
@@ -214,16 +214,34 @@ bool
 CrossOriginXrayWrapper::getPrototypeOf(JSContext *cx, JS::HandleObject wrapper,
                                        JS::MutableHandleObject protop) const
 {
     // Cross-origin objects have null prototypes.
     protop.set(nullptr);
     return true;
 }
 
+bool
+CrossOriginXrayWrapper::getOwnPropertyNames(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                                            JS::AutoIdVector &props) const
+{
+    // All properties on cross-origin objects are supposed |own|, despite what
+    // the underlying native object may report. Override the inherited trap to
+    // avoid passing JSITER_OWNONLY as a flag.
+    return SecurityXrayDOM::enumerate(cx, wrapper, JSITER_HIDDEN, props);
+}
+
+bool
+CrossOriginXrayWrapper::enumerate(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                                  JS::AutoIdVector &props) const
+{
+    // Cross-origin properties are non-enumerable.
+    return true;
+}
+
 #define XOW FilteringWrapper<CrossOriginXrayWrapper, CrossOriginAccessiblePropertiesOnly>
 #define NNXOW FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>
 #define NNXOWC FilteringWrapper<CrossCompartmentSecurityWrapper, OpaqueWithCall>
 
 template<> const XOW XOW::singleton(0);
 template<> const NNXOW NNXOW::singleton(0);
 template<> const NNXOWC NNXOWC::singleton(0);
 
--- a/js/xpconnect/wrappers/FilteringWrapper.h
+++ b/js/xpconnect/wrappers/FilteringWrapper.h
@@ -64,15 +64,20 @@ class CrossOriginXrayWrapper : public Se
 
     virtual bool getPropertyDescriptor(JSContext *cx, JS::Handle<JSObject*> wrapper,
                                        JS::Handle<jsid> id,
                                        JS::MutableHandle<JSPropertyDescriptor> desc) const MOZ_OVERRIDE;
     virtual bool getOwnPropertyDescriptor(JSContext *cx, JS::Handle<JSObject*> wrapper,
                                           JS::Handle<jsid> id,
                                           JS::MutableHandle<JSPropertyDescriptor> desc) const MOZ_OVERRIDE;
 
+    virtual bool getOwnPropertyNames(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                                     JS::AutoIdVector &props) const MOZ_OVERRIDE;
+    virtual bool enumerate(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                           JS::AutoIdVector &props) const MOZ_OVERRIDE;
+
     virtual bool getPrototypeOf(JSContext *cx, JS::HandleObject wrapper,
                                 JS::MutableHandleObject protop) const MOZ_OVERRIDE;
 };
 
 }
 
 #endif /* __FilteringWrapper_h__ */
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1907,26 +1907,28 @@ XPCWrappedNativeXrayTraits::enumerateNam
     AutoIdVector wnProps(cx);
     {
         RootedObject target(cx, singleton.getTargetObject(wrapper));
         JSAutoCompartment ac(cx, target);
         if (!js::GetPropertyNames(cx, target, flags, &wnProps))
             return false;
     }
 
-    // Go through the properties we got and enumerate all native ones.
+    // Go through the properties we found on the underlying object and see if
+    // they appear on the XrayWrapper. If it throws (which may happen if the
+    // wrapper is a SecurityWrapper), just clear the exception and move on.
+    MOZ_ASSERT(!JS_IsExceptionPending(cx));
     if (!props.reserve(wnProps.length()))
         return false;
     for (size_t n = 0; n < wnProps.length(); ++n) {
         RootedId id(cx, wnProps[n]);
         bool hasProp;
-        if (!JS_HasPropertyById(cx, wrapper, id, &hasProp))
-            return false;
-        if (hasProp)
+        if (JS_HasPropertyById(cx, wrapper, id, &hasProp) && hasProp)
             props.infallibleAppend(id);
+        JS_ClearPendingException(cx);
     }
     return true;
 }
 
 JSObject *
 XPCWrappedNativeXrayTraits::createHolder(JSContext *cx, JSObject *wrapper)
 {
     RootedObject global(cx, JS_GetGlobalForObject(cx, wrapper));
@@ -2664,20 +2666,16 @@ XrayWrapper<Base, Traits>::delete_(JSCon
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::enumerate(JSContext *cx, HandleObject wrapper, unsigned flags,
                                      AutoIdVector &props) const
 {
     assertEnteredPolicy(cx, wrapper, JSID_VOID, BaseProxyHandler::ENUMERATE);
-    if (!AccessCheck::wrapperSubsumes(wrapper)) {
-        JS_ReportError(cx, "Not allowed to enumerate cross origin objects");
-        return false;
-    }
 
     // Enumerate expando properties first. Note that the expando object lives
     // in the target compartment.
     RootedObject target(cx, Traits::singleton.getTargetObject(wrapper));
     RootedObject expando(cx, Traits::singleton.getExpandoObject(cx, target, wrapper));
     if (expando) {
         JSAutoCompartment ac(cx, expando);
         if (!js::GetPropertyNames(cx, expando, flags, &props))
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -133,16 +133,17 @@ class XrayWrapper : public Base {
     }
     bool getPrototypeOfHelper(JSContext *cx, JS::HandleObject wrapper,
                               JS::HandleObject target, JS::MutableHandleObject protop) const
     {
         return getPrototypeOfHelper<Traits::HasPrototype>(cx, wrapper, target,
                                                           protop);
     }
 
+  protected:
     bool enumerate(JSContext *cx, JS::Handle<JSObject*> wrapper, unsigned flags,
                    JS::AutoIdVector &props) const;
 };
 
 #define PermissiveXrayXPCWN xpc::XrayWrapper<js::CrossCompartmentWrapper, xpc::XPCWrappedNativeXrayTraits>
 #define SecurityXrayXPCWN xpc::XrayWrapper<js::CrossCompartmentSecurityWrapper, xpc::XPCWrappedNativeXrayTraits>
 #define PermissiveXrayDOM xpc::XrayWrapper<js::CrossCompartmentWrapper, xpc::DOMXrayTraits>
 #define SecurityXrayDOM xpc::XrayWrapper<js::CrossCompartmentSecurityWrapper, xpc::DOMXrayTraits>