Bug 1363886 - Part 3: Check async callback arguments against schema draft
authorTomislav Jovanovic <tomica@gmail.com>
Sun, 21 May 2017 04:19:46 +0200
changeset 582097 c1b7d358e7c3f86f1789d569865d781cf26a92f6
parent 582096 7fa1fdb6a39261ae6eac1ca4057cc2f92b0c8930
child 629671 d02da5c6ebf0ba36caa48562cd8d11ae948f191a
push id59972
push userbmo:tomica@gmail.com
push dateSun, 21 May 2017 11:45:33 +0000
bugs1363886
milestone55.0a1
Bug 1363886 - Part 3: Check async callback arguments against schema MozReview-Commit-ID: E0yp9SdJrv6
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/test/xpcshell/test_ext_schemas_async.js
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -2054,16 +2054,17 @@ class CallEntry extends Entry {
 // Represents a "function" defined in a schema namespace.
 FunctionEntry = class FunctionEntry extends CallEntry {
   static parseSchema(schema, path) {
     let returns = null;
     if ("returns" in schema) {
       returns = {
         type: Schemas.parseSchema(schema.returns, path, ["optional", "name"]),
         optional: schema.returns.optional || false,
+        name: "result",
       };
     }
 
     return new this(schema, path, schema.name,
                     Schemas.parseSchema(schema, path,
                       ["name", "unsupported", "returns",
                        "permissions",
                        "allowAmbiguousOptionalArguments"]),
@@ -2078,28 +2079,35 @@ FunctionEntry = class FunctionEntry exte
     this.unsupported = unsupported;
     this.returns = returns;
     this.permissions = permissions;
 
     this.isAsync = type.isAsync;
     this.hasAsyncCallback = type.hasAsyncCallback;
   }
 
-  checkValue({type, optional}, value, context) {
+  checkValue({type, optional, name}, value, context) {
     if (optional && value == null) {
       return;
     }
     if (type.reference === "ExtensionPanel" || type.reference === "Port") {
       // TODO: We currently treat objects with functions as SubModuleType,
       // which is just wrong, and a bigger yak.  Skipping for now.
       return;
     }
     const {error} = type.normalize(value, context);
     if (error) {
-      this.throwError(context, `Type error for result value (${error})`);
+      this.throwError(context, `Type error for ${name} value (${error})`);
+    }
+  }
+
+  checkCallback(args, context) {
+    const callback = this.parameters[this.parameters.length - 1];
+    for (const [i, param] of callback.type.parameters.entries()) {
+      this.checkValue(param, args[i], context);
     }
   }
 
   getDescriptor(path, context) {
     let apiImpl = context.getImplementation(path.join("."), this.name);
 
     let stub;
     if (this.isAsync) {
@@ -2111,17 +2119,31 @@ FunctionEntry = class FunctionEntry exte
           callback = actuals.pop();
         }
         if (callback === null && context.isChromeCompat) {
           // We pass an empty stub function as a default callback for
           // the `chrome` API, so promise objects are not returned,
           // and lastError values are reported immediately.
           callback = () => {};
         }
-        return apiImpl.callAsyncFunction(actuals, callback);
+        if (DEBUG && this.hasAsyncCallback && callback) {
+          let original = callback;
+          callback = (...args) => {
+            this.checkCallback(args, context);
+            original(...args);
+          };
+        }
+        let result = apiImpl.callAsyncFunction(actuals, callback);
+        if (DEBUG && this.hasAsyncCallback && !callback) {
+          return result.then(result => {
+            this.checkCallback([result], context);
+            return result;
+          });
+        }
+        return result;
       };
     } else if (!this.returns) {
       stub = (...args) => {
         this.checkDeprecated(context);
         let actuals = this.checkParameters(args, context);
         return apiImpl.callFunctionNoReturn(actuals);
       };
     } else {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas_async.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas_async.js
@@ -3,16 +3,24 @@
 Components.utils.import("resource://gre/modules/ExtensionCommon.jsm");
 Components.utils.import("resource://gre/modules/Schemas.jsm");
 
 let {BaseContext, LocalAPIImplementation} = ExtensionCommon;
 
 let schemaJson = [
   {
     namespace: "testnamespace",
+    types: [{
+      id: "Widget",
+      type: "object",
+      properties: {
+        size: {type: "integer"},
+        colour: {type: "string", optional: true},
+      },
+    }],
     functions: [{
       name: "one_required",
       type: "function",
       parameters: [{
         name: "first",
         type: "function",
         parameters: [],
       }],
@@ -39,16 +47,28 @@ let schemaJson = [
       type: "function",
       async: "first",
       parameters: [{
         name: "first",
         type: "function",
         parameters: [],
         optional: true,
       }],
+    }, {
+      name: "async_result",
+      type: "function",
+      async: "callback",
+      parameters: [{
+        name: "callback",
+        type: "function",
+        parameters: [{
+          name: "widget",
+          $ref: "Widget",
+        }],
+      }],
     }],
   },
 ];
 
 const global = this;
 class StubContext extends BaseContext {
   constructor() {
     let fakeExtension = {id: "test@web.extension"};
@@ -142,16 +162,46 @@ add_task(async function testParameterVal
     assertNoThrows("async_optional");
     assertNoThrows("async_optional", null);
     assertNoThrows("async_optional", cb);
     assertThrows("async_optional", cb, null);
     assertThrows("async_optional", cb, cb);
   }
 });
 
+add_task(async function testCheckAsyncResults() {
+  await Schemas.load("data:," + JSON.stringify(schemaJson));
+
+  const complete = generateAPIs({}, {
+    async_result: async () => ({size: 5, colour: "green"}),
+  });
+
+  const optional = generateAPIs({}, {
+    async_result: async () => ({size: 6}),
+  });
+
+  const invalid = generateAPIs({}, {
+    async_result: async () => ({}),
+  });
+
+  deepEqual(await complete.async_result(), {size: 5, colour: "green"});
+
+  deepEqual(await optional.async_result(), {size: 6},
+            "Missing optional properties is allowed");
+
+  if (AppConstants.DEBUG) {
+    await Assert.rejects(invalid.async_result(),
+          `Type error for widget value (Property "size" is required)`,
+          "Should throw for invalid callback argument in DEBUG builds");
+  } else {
+    deepEqual(await invalid.async_result(), {},
+              "Invalid callback argument doesn't throw in release builds");
+  }
+});
+
 add_task(async function testAsyncResults() {
   await Schemas.load("data:," + JSON.stringify(schemaJson));
   async function runWithCallback(func) {
     do_print(`Calling testnamespace.${func.name}, expecting callback with result`);
     return await new Promise(resolve => {
       let result = "uninitialized value";
       let returnValue = func(reply => {
         result = reply;