Bug 1069518 - XPTCall should refuse to implement interfaces with [notxpcom] methods, r=froydnj/bholley
authorBenjamin Smedberg <benjamin@smedbergs.us>
Mon, 22 Sep 2014 15:10:31 -0400
changeset 206585 b415215720263664bf4b86f42b1f400821fce53a
parent 206584 eb5bd78a635f49e34f210f6e58a6e0ece2bf12bc
child 206586 cfa50eefce84e971a2f330d05025a14f08429019
push idunknown
push userunknown
push dateunknown
reviewersfroydnj, bholley
bugs1069518
milestone35.0a1
Bug 1069518 - XPTCall should refuse to implement interfaces with [notxpcom] methods, r=froydnj/bholley
js/xpconnect/src/XPCWrappedJS.cpp
js/xpconnect/src/xpcprivate.h
xpcom/reflect/xptcall/xptcall.cpp
xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
xpcom/reflect/xptinfo/xptiprivate.h
xpcom/tests/Makefile.in
xpcom/tests/NotXPCOMTest.idl
xpcom/tests/TestScriptable.idl
xpcom/tests/moz.build
xpcom/tests/unit/test_notxpcom_scriptable.js
xpcom/tests/unit/xpcomtest.manifest
xpcom/tests/unit/xpcshell.ini
--- a/js/xpconnect/src/XPCWrappedJS.cpp
+++ b/js/xpconnect/src/XPCWrappedJS.cpp
@@ -342,50 +342,60 @@ nsXPCWrappedJS::GetNewOrUsed(JS::HandleO
                                                                             allowNonScriptable);
     if (!clasp)
         return NS_ERROR_FAILURE;
 
     JS::RootedObject rootJSObj(cx, clasp->GetRootJSObject(cx, jsObj));
     if (!rootJSObj)
         return NS_ERROR_FAILURE;
 
+    nsresult rv = NS_ERROR_FAILURE;
     nsRefPtr<nsXPCWrappedJS> root = map->Find(rootJSObj);
     if (root) {
         nsRefPtr<nsXPCWrappedJS> wrapper = root->FindOrFindInherited(aIID);
         if (wrapper) {
             wrapper.forget(wrapperResult);
             return NS_OK;
         }
     } else if (rootJSObj != jsObj) {
 
         // Make a new root wrapper, because there is no existing
         // root wrapper, and the wrapper we are trying to make isn't
         // a root.
         nsRefPtr<nsXPCWrappedJSClass> rootClasp = nsXPCWrappedJSClass::GetNewOrUsed(cx, NS_GET_IID(nsISupports));
         if (!rootClasp)
             return NS_ERROR_FAILURE;
 
-        root = new nsXPCWrappedJS(cx, rootJSObj, rootClasp, nullptr);
+        root = new nsXPCWrappedJS(cx, rootJSObj, rootClasp, nullptr, &rv);
+        if (NS_FAILED(rv)) {
+            return rv;
+        }
     }
 
-    nsRefPtr<nsXPCWrappedJS> wrapper = new nsXPCWrappedJS(cx, jsObj, clasp, root);
+    nsRefPtr<nsXPCWrappedJS> wrapper = new nsXPCWrappedJS(cx, jsObj, clasp, root, &rv);
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
     wrapper.forget(wrapperResult);
     return NS_OK;
 }
 
 nsXPCWrappedJS::nsXPCWrappedJS(JSContext* cx,
                                JSObject* aJSObj,
                                nsXPCWrappedJSClass* aClass,
-                               nsXPCWrappedJS* root)
+                               nsXPCWrappedJS* root,
+                               nsresult *rv)
     : mJSObj(aJSObj),
       mClass(aClass),
       mRoot(root ? root : MOZ_THIS_IN_INITIALIZER_LIST()),
       mNext(nullptr)
 {
-    InitStub(GetClass()->GetIID());
+    *rv = InitStub(GetClass()->GetIID());
+    // Continue even in the failure case, so that our refcounting/Destroy
+    // behavior works correctly.
 
     // There is an extra AddRef to support weak references to wrappers
     // that are subject to finalization. See the top of the file for more
     // details.
     NS_ADDREF_THIS();
 
     if (IsRootWrapper()) {
         nsXPConnect::GetRuntimeInstance()->GetWrappedJSMap()->Add(cx, this);
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -2488,17 +2488,18 @@ public:
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
     virtual ~nsXPCWrappedJS();
 protected:
     nsXPCWrappedJS();   // not implemented
     nsXPCWrappedJS(JSContext* cx,
                    JSObject* aJSObj,
                    nsXPCWrappedJSClass* aClass,
-                   nsXPCWrappedJS* root);
+                   nsXPCWrappedJS* root,
+                   nsresult* rv);
 
     bool CanSkip();
     void Destroy();
     void Unlink();
 
 private:
     JS::Heap<JSObject*> mJSObj;
     nsRefPtr<nsXPCWrappedJSClass> mClass;
--- a/xpcom/reflect/xptcall/xptcall.cpp
+++ b/xpcom/reflect/xptcall/xptcall.cpp
@@ -3,16 +3,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* entry point wrappers. */
 
 #include "xptcprivate.h"
 #include "xptiprivate.h"
 #include "mozilla/XPTInterfaceInfoManager.h"
+#include "nsPrintfCString.h"
 
 using namespace mozilla;
 
 NS_IMETHODIMP
 nsXPTCStubBase::QueryInterface(REFNSIID aIID,
                                void **aInstancePtr)
 {
     if (aIID.Equals(mEntry->IID())) {
@@ -47,16 +48,24 @@ NS_GetXPTCallStub(REFNSIID aIID, nsIXPTC
         XPTInterfaceInfoManager::GetSingleton();
     if (NS_WARN_IF(!iim))
         return NS_ERROR_NOT_INITIALIZED;
 
     xptiInterfaceEntry *iie = iim->GetInterfaceEntryForIID(&aIID);
     if (!iie || !iie->EnsureResolved() || iie->GetBuiltinClassFlag())
         return NS_ERROR_FAILURE;
 
+    if (iie->GetHasNotXPCOMFlag()) {
+#ifdef DEBUG
+        nsPrintfCString msg("XPTCall will not implement interface %s because of [notxpcom] members.", iie->GetTheName());
+        NS_WARNING(msg.get());
+#endif
+        return NS_ERROR_FAILURE;
+    }
+
     nsXPTCStubBase* newbase = new nsXPTCStubBase(aOuter, iie);
     if (!newbase)
         return NS_ERROR_OUT_OF_MEMORY;
 
     *aResult = newbase;
     return NS_OK;
 }
 
--- a/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
+++ b/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
@@ -102,16 +102,29 @@ xptiInterfaceEntry::ResolveLocked()
         
         if(!parent || !parent->EnsureResolvedLocked())
         {
             SetResolvedState(RESOLVE_FAILED);
             return false;
         }
 
         mParent = parent;
+        if (parent->GetHasNotXPCOMFlag()) {
+            SetHasNotXPCOMFlag();
+        } else {
+            for (uint16_t idx = 0; idx < mDescriptor->num_methods; ++idx) {
+                nsXPTMethodInfo* method = reinterpret_cast<nsXPTMethodInfo*>(
+                    mDescriptor->method_descriptors + idx);
+                if (method->IsNotXPCOM()) {
+                    SetHasNotXPCOMFlag();
+                    break;
+                }
+            }
+        }
+
 
         mMethodBaseIndex =
             parent->mMethodBaseIndex + 
             parent->mDescriptor->num_methods;
         
         mConstantBaseIndex =
             parent->mConstantBaseIndex + 
             parent->mDescriptor->num_constants;
--- a/xpcom/reflect/xptinfo/xptiprivate.h
+++ b/xpcom/reflect/xptinfo/xptiprivate.h
@@ -175,32 +175,43 @@ public:
 
     enum {
         PARTIALLY_RESOLVED    = 1,
         FULLY_RESOLVED        = 2,
         RESOLVE_FAILED        = 3
     };
     
     // Additional bit flags...
-    enum {SCRIPTABLE = 4, BUILTINCLASS = 8};
+    enum {SCRIPTABLE = 4, BUILTINCLASS = 8, HASNOTXPCOM = 16};
 
     uint8_t GetResolveState() const {return mFlags.GetState();}
     
     bool IsFullyResolved() const 
         {return GetResolveState() == (uint8_t) FULLY_RESOLVED;}
 
     void   SetScriptableFlag(bool on)
                 {mFlags.SetFlagBit(uint8_t(SCRIPTABLE),on);}
     bool GetScriptableFlag() const
                 {return mFlags.GetFlagBit(uint8_t(SCRIPTABLE));}
     void   SetBuiltinClassFlag(bool on)
                 {mFlags.SetFlagBit(uint8_t(BUILTINCLASS),on);}
     bool GetBuiltinClassFlag() const
                 {return mFlags.GetFlagBit(uint8_t(BUILTINCLASS));}
 
+
+    // AddRef/Release are special and are not considered for the NOTXPCOM flag.
+    void SetHasNotXPCOMFlag()
+    {
+        mFlags.SetFlagBit(HASNOTXPCOM, true);
+    }
+    bool GetHasNotXPCOMFlag() const
+    {
+        return mFlags.GetFlagBit(HASNOTXPCOM);
+    }
+
     const nsID* GetTheIID()  const {return &mIID;}
     const char* GetTheName() const {return mName;}
 
     bool EnsureResolved()
         {return IsFullyResolved() ? true : Resolve();}
 
     already_AddRefed<xptiInterfaceInfo> InterfaceInfo();
     bool     InterfaceInfoEquals(const xptiInterfaceInfo* info) const 
--- a/xpcom/tests/Makefile.in
+++ b/xpcom/tests/Makefile.in
@@ -1,22 +1,28 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 # Make sure we have symbols in case we need to debug these.
 MOZ_DEBUG_SYMBOLS = 1
 
+# Don't add our test-only .xpt files to the normal manifests
+NO_INTERFACES_MANIFEST = 1
+
 include $(topsrcdir)/config/rules.mk
 
 ifneq (,$(SIMPLE_PROGRAMS))
 libs::
 	$(INSTALL) $(SIMPLE_PROGRAMS) $(DEPTH)/_tests/xpcshell/$(relativesrcdir)/unit
 endif
 
+libs::
+	$(INSTALL) $(DIST)/bin/components/xpcomtest.xpt $(DEPTH)/_tests/xpcshell/$(relativesrcdir)/unit
+
 ifeq (,$(filter-out WINNT, $(HOST_OS_ARCH)))
 getnativepath = $(call normalizepath,$(1))
 else
 getnativepath = $(1)
 endif
 
 abs_srcdir = $(abspath $(srcdir))
 
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/NotXPCOMTest.idl
@@ -0,0 +1,23 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "nsISupports.idl"
+
+[scriptable, uuid(93142a4f-e4cf-424a-b833-e638f87d2607)]
+interface ScriptableOK : nsISupports
+{
+  void method1();
+};
+
+[scriptable, uuid(237d01a3-771e-4c6e-adf9-c97f9aab2950)]
+interface ScriptableWithNotXPCOM : nsISupports
+{
+  [notxpcom] void method2();
+};
+
+[scriptable, uuid(4f06ec60-3bb3-4712-ab18-b2b595285558)]
+interface ScriptableWithNotXPCOMBase : ScriptableWithNotXPCOM
+{
+  void method3();
+};
deleted file mode 100644
--- a/xpcom/tests/TestScriptable.idl
+++ /dev/null
@@ -1,21 +0,0 @@
-
-[scriptable, uuid(76d74662-0eae-404c-9d1f-697c0e321c0a)]
-interface testScriptableInterface {
-  readonly attribute long scriptable_attr1;
-  attribute long scriptable_attr2;
-  [noscript] readonly attribute long notscriptable_attr1;
-  [noscript] attribute long notscriptable_attr2;
-
-  void scriptable_method1();
-  [noscript] void notscriptable_method1();
-  [notxpcom] void notscriptable_method2();
-  [noscript, notxpcom] void notscriptable_method3();
-};
-
-[uuid(76d74662-0eae-404c-9d1f-697c0e321c0a)]
-interface testNotscriptableInterface {
-  readonly attribute long notscriptable_attr1;
-  attribute long notscriptable_attr2;
-
-  void notscriptable_method1();
-};
--- a/xpcom/tests/moz.build
+++ b/xpcom/tests/moz.build
@@ -105,16 +105,21 @@ if CONFIG['MOZ_MEMORY']:
 if CONFIG['MOZ_DEBUG'] and CONFIG['OS_ARCH'] not in ('WINNT'):
     # FIXME bug 523392: TestDeadlockDetector doesn't like Windows
     # FIXME bug 523378: also fails on OS X
     CppUnitTests([
         'TestDeadlockDetector',
         'TestDeadlockDetectorScalability',
     ])
 
+XPIDL_MODULE = 'xpcomtest'
+XPIDL_SOURCES += [
+    'NotXPCOMTest.idl',
+]
+
 LOCAL_INCLUDES += [
     '../ds',
 ]
 
 RESOURCE_FILES += [
     'test.properties',
 ]
 
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/unit/test_notxpcom_scriptable.js
@@ -0,0 +1,86 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cu = Components.utils;
+const Cr = Components.results;
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+const kCID = Components.ID("{1f9f7181-e6c5-4f4c-8f71-08005cec8468}");
+const kContract = "@testing/notxpcomtest";
+
+function run_test()
+{
+  let manifest = do_get_file("xpcomtest.manifest");
+  let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
+  registrar.autoRegister(manifest);
+
+  ok(Ci.ScriptableWithNotXPCOM);
+
+  let method1Called = false;
+
+  let testObject = {
+    QueryInterface: XPCOMUtils.generateQI([Ci.ScriptableOK,
+                                           Ci.ScriptableWithNotXPCOM,
+                                           Ci.ScriptableWithNotXPCOMBase]),
+
+    method1: function() {
+      method1Called = true;
+    },
+
+    method2: function() {
+      ok(false, "method2 should not have been called!");
+    },
+
+    method3: function() {
+      ok(false, "mehod3 should not have been called!");
+    },
+
+    jsonly: true,
+  };
+
+  let factory = {
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsIFactory]),
+
+    createInstance: function(outer, iid) {
+      if (outer) {
+        throw Cr.NS_ERROR_NO_AGGREGATION;
+      }
+      return testObject.QueryInterface(iid);
+    },
+  };
+
+  registrar.registerFactory(kCID, null, kContract, factory);
+
+  let xpcomObject = Cc[kContract].createInstance();
+  ok(xpcomObject);
+  strictEqual(xpcomObject.jsonly, undefined);
+
+  xpcomObject.QueryInterface(Ci.ScriptableOK);
+
+  xpcomObject.method1();
+  ok(method1Called);
+
+  try {
+    xpcomObject.QueryInterface(Ci.ScriptableWithNotXPCOM);
+    ok(false, "Should not have implemented ScriptableWithNotXPCOM");
+  }
+  catch(e) {
+    ok(true, "Should not have implemented ScriptableWithNotXPCOM. Correctly threw error: " + e);
+  }
+  strictEqual(xpcomObject.method2, undefined);
+
+  try {
+    xpcomObject.QueryInterface(Ci.ScriptableWithNotXPCOMBase);
+    ok(false, "Should not have implemented ScriptableWithNotXPCOMBase");
+  }
+  catch (e) {
+    ok(true, "Should not have implemented ScriptableWithNotXPCOMBase. Correctly threw error: " + e);
+  }
+  strictEqual(xpcomObject.method3, undefined);
+}
+
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/unit/xpcomtest.manifest
@@ -0,0 +1,1 @@
+interfaces xpcomtest.xpt
--- a/xpcom/tests/unit/xpcshell.ini
+++ b/xpcom/tests/unit/xpcshell.ini
@@ -1,15 +1,19 @@
 [DEFAULT]
 head = head_xpcom.js
 tail =
 support-files =
   bug725015.manifest
   compmgr_warnings.manifest
   data/**
+  xpcomtest.xpt
+  xpcomtest.manifest
+generated-files =
+  xpcomtest.xpt
 
 [test_bug121341.js]
 [test_bug325418.js]
 [test_bug332389.js]
 [test_bug333505.js]
 [test_bug364285-1.js]
 # Bug 902073: test fails consistently on Android x86
 skip-if = os == "android"
@@ -59,8 +63,9 @@ skip-if = os == "android"
 [test_comp_no_aslr.js]
 skip-if = os != "win"
 [test_windows_shortcut.js]
 [test_bug745466.js]
 skip-if = os == "win"
 # Bug 676998: test fails consistently on Android
 fail-if = os == "android"
 [test_file_renameTo.js]
+[test_notxpcom_scriptable.js]