Bug 1066846 - Explicitly verify that incoming dipper parameters are objects. r=bz

js/xpconnect/src/XPCWrappedNative.cpp

@@ -1845,8 +1845,8 @@ CallMethodHelper::GetOutParamSource(uint8_t paramIndex, MutableHandleValue srcp)
 {
     const nsXPTParamInfo& paramInfo = mMethodInfo->GetParam(paramIndex);
 
-    if ((paramInfo.IsOut() || paramInfo.IsDipper()) &&
-        !paramInfo.IsRetval()) {
+    MOZ_ASSERT(!paramInfo.IsDipper(), "Dipper params are handled separately");
+    if (paramInfo.IsOut() && !paramInfo.IsRetval()) {
         MOZ_ASSERT(paramIndex < mArgc || paramInfo.IsOptional(),
                    "Expected either enough arguments or an optional argument");
         jsval arg = paramIndex < mArgc ? mArgv[paramIndex] : JSVAL_NULL;
@@ -2099,8 +2099,17 @@ CallMethodHelper::ConvertIndependentParam(uint8_t i)
     if (paramInfo.IsStringClass()) {
         if (!AllocateStringClass(dp, paramInfo))
             return false;
-        if (paramInfo.IsDipper())
+        if (paramInfo.IsDipper()) {
+            // We've allocated our string class explicitly, so we don't need
+            // to do any conversions on the incoming argument. However, we still
+            // need to verify that it's an object, so that we don't get surprised
+            // later on when trying to assign the result to .value.
+            if (i < mArgc && !mArgv[i].isObject()) {
+                ThrowBadParam(NS_ERROR_XPC_NEED_OUT_OBJECT, i, mCallContext);
+                return false;
+            }
             return true;
+        }
     }
 
     // Specify the correct storage/calling semantics.

js/xpconnect/tests/components/js/xpctest_params.js

@@ -73,6 +73,7 @@ TestParams.prototype = {
   testSizedWstring: f_is,
   testInterfaceIs: f_is,
   testInterfaceIsArray: f_size_and_iid,
+  testOutAString: function(o) { o.value = "out"; }
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([TestParams]);

js/xpconnect/tests/components/native/xpctest_params.cpp

@@ -339,3 +339,10 @@ NS_IMETHODIMP nsXPCTestParams::TestInterfaceIsArray(uint32_t aLength, const nsII
     // Do this second, since the macro returns.
     BUFFER_METHOD_IMPL(void*, 0, TAKE_OWNERSHIP_INTERFACE);
 }
+
+/* void testOutAString (out AString o); */
+NS_IMETHODIMP nsXPCTestParams::TestOutAString(nsAString & o)
+{
+    o.AssignLiteral("out");
+    return NS_OK;
+}

js/xpconnect/tests/idl/xpctest_params.idl

@@ -81,4 +81,7 @@ interface nsIXPCTestParams : nsISupports {
                                              [array, size_is(bLength), iid_is(bIID)] inout nsQIResult b,
                                              out unsigned long rvLength, out nsIIDPtr rvIID,
                                              [retval, array, size_is(rvLength), iid_is(rvIID)] out nsQIResult rv);
+
+  // Test for out dipper parameters
+  void                 testOutAString(out AString o);
 };

js/xpconnect/tests/unit/test_params.js

@@ -133,6 +133,15 @@ function test_component(contractid) {
   doTestWorkaround("testACString", "Just a regular C string.");
   doTest("testJsval", {aprop: 12, bprop: "str"}, 4.22);
 
+  // Test out dipper parameters, since they're special and we can't really test
+  // inouts.
+  let outAString = {};
+  o.testOutAString(outAString);
+  do_check_eq(outAString.value, "out");
+  try { o.testOutAString(undefined); } catch (e) {} // Don't crash
+  try { o.testOutAString(null); } catch (e) {} // Don't crash
+  try { o.testOutAString("string"); } catch (e) {} // Don't crash
+
   // Helpers to instantiate various test XPCOM objects.
   var numAsMade = 0;
   function makeA() {