Bug 1363886 - Part 3: Check async callback arguments against schema
MozReview-Commit-ID: E0yp9SdJrv6
--- 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;