Bug 1363886 - Part 1: Check API function results against schema
MozReview-Commit-ID: E2mGR03zUSf
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -2097,37 +2097,61 @@ class CallEntry extends Entry {
return fixedArgs;
}
}
// Represents a "function" defined in a schema namespace.
FunctionEntry = class FunctionEntry extends CallEntry {
static parseSchema(schema, path) {
+ // When not in DEBUG mode, we just need to know *if* this returns.
+ let returns = !!schema.returns;
+ if (DEBUG && "returns" in schema) {
+ returns = {
+ type: Schemas.parseSchema(schema.returns, path, ["optional", "name"]),
+ optional: schema.returns.optional || false,
+ };
+ }
+
return new this(schema, path, schema.name,
Schemas.parseSchema(schema, path,
["name", "unsupported", "returns",
"permissions",
"allowAmbiguousOptionalArguments"]),
schema.unsupported || false,
schema.allowAmbiguousOptionalArguments || false,
- schema.returns || null,
+ returns,
schema.permissions || null);
}
constructor(schema, path, name, type, unsupported, allowAmbiguousOptionalArguments, returns, permissions) {
super(schema, path, name, type.parameters, allowAmbiguousOptionalArguments);
this.unsupported = unsupported;
this.returns = returns;
this.permissions = permissions;
this.isAsync = type.isAsync;
this.hasAsyncCallback = type.hasAsyncCallback;
}
+ checkValue({type, optional}, 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})`);
+ }
+ }
+
getDescriptor(path, context) {
let apiImpl = context.getImplementation(path.join("."), this.name);
let stub;
if (this.isAsync) {
stub = (...args) => {
this.checkDeprecated(context);
let actuals = this.checkParameters(args, context);
@@ -2148,17 +2172,21 @@ FunctionEntry = class FunctionEntry exte
this.checkDeprecated(context);
let actuals = this.checkParameters(args, context);
return apiImpl.callFunctionNoReturn(actuals);
};
} else {
stub = (...args) => {
this.checkDeprecated(context);
let actuals = this.checkParameters(args, context);
- return apiImpl.callFunction(actuals);
+ let result = apiImpl.callFunction(actuals);
+ if (DEBUG && this.returns) {
+ this.checkValue(this.returns, result, context);
+ }
+ return result;
};
}
return {
descriptor: {value: Cu.exportFunction(stub, context.cloneScope)},
revoke() {
apiImpl.revoke();
apiImpl = null;
--- a/toolkit/components/extensions/schemas/extension.json
+++ b/toolkit/components/extensions/schemas/extension.json
@@ -80,32 +80,30 @@
}
}
}
],
"returns": {
"type": "array",
"description": "Array of global objects",
"items": {
- "name": "viewGlobals",
"type": "object",
"isInstanceOf": "Window",
"additionalProperties": { "type": "any" }
}
}
},
{
"name": "getBackgroundPage",
"type": "function",
"description": "Returns the JavaScript 'window' object for the background page running inside the current extension. Returns null if the extension has no background page.",
"parameters": [],
"returns": {
"type": "object",
"optional": true,
- "name": "backgroundPageGlobal",
"isInstanceOf": "Window",
"additionalProperties": { "type": "any" }
}
},
{
"name": "isAllowedIncognitoAccess",
"type": "function",
"description": "Retrieves the state of the extension's access to Incognito-mode (as determined by the user-controlled 'Allowed in Incognito' checkbox.",
--- a/toolkit/components/extensions/schemas/identity.json
+++ b/toolkit/components/extensions/schemas/identity.json
@@ -188,17 +188,17 @@
"name": " path",
"type": "string",
"default": "",
"optional": true,
"description": "The path appended to the end of the generated URL. "
}
],
"returns": {
- "string": "path"
+ "type": "string"
}
}
],
"events": [
{
"name": "onSignInChanged",
"unsupported": true,
"type": "function",
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
@@ -71,17 +71,17 @@ let json = [
{
id: "submodule",
type: "object",
functions: [
{
name: "sub_foo",
type: "function",
parameters: [],
- returns: "integer",
+ returns: {type: "integer"},
},
],
},
],
functions: [
{
name: "foo",
@@ -399,16 +399,19 @@ class TallyingAPIImplementation extends
constructor(namespace, name) {
super();
this.namespace = namespace;
this.name = name;
}
callFunction(args) {
tally("call", this.namespace, this.name, args);
+ if (this.name === "sub_foo") {
+ return 13;
+ }
}
callFunctionNoReturn(args) {
tally("call", this.namespace, this.name, args);
}
getProperty() {
tally("get", this.namespace, this.name);
@@ -1495,16 +1498,17 @@ let defaultsJson = [
type: "function",
parameters: [
{name: "arg", type: "object", optional: true, properties: {
prop1: {type: "integer", optional: true},
}, default: {prop1: 1}},
],
returns: {
type: "object",
+ additionalProperties: true,
},
},
]},
];
add_task(async function testDefaults() {
let url = "data:," + JSON.stringify(defaultsJson);
await Schemas.load(url);
@@ -1531,8 +1535,85 @@ add_task(async function testDefaults() {
let root = {};
Schemas.inject(root, localWrapper);
deepEqual(root.defaultsJson.defaultFoo(), {prop1: 1, newProp: 1});
deepEqual(root.defaultsJson.defaultFoo({prop1: 2}), {prop1: 2, newProp: 1});
deepEqual(root.defaultsJson.defaultFoo(), {prop1: 1, newProp: 1});
});
+
+let returnsJson = [{
+ namespace: "returns",
+ types: [
+ {
+ id: "Widget",
+ type: "object",
+ properties: {
+ size: {type: "integer"},
+ colour: {type: "string", optional: true},
+ },
+ },
+ ],
+ functions: [
+ {
+ name: "complete",
+ type: "function",
+ returns: {$ref: "Widget"},
+ parameters: [],
+ },
+ {
+ name: "optional",
+ type: "function",
+ returns: {$ref: "Widget"},
+ parameters: [],
+ },
+ {
+ name: "invalid",
+ type: "function",
+ returns: {$ref: "Widget"},
+ parameters: [],
+ },
+ ],
+}];
+
+add_task(async function testReturns() {
+ const url = "data:," + JSON.stringify(returnsJson);
+ await Schemas.load(url);
+
+ const apiObject = {
+ complete() {
+ return {size: 3, colour: "orange"};
+ },
+ optional() {
+ return {size: 4};
+ },
+ invalid() {
+ return {};
+ },
+ };
+
+ const localWrapper = {
+ cloneScope: global,
+ shouldInject(ns) {
+ return true;
+ },
+ getImplementation(ns, name) {
+ return new LocalAPIImplementation(apiObject, name, null);
+ },
+ };
+
+ const root = {};
+ Schemas.inject(root, localWrapper);
+
+ deepEqual(root.returns.complete(), {size: 3, colour: "orange"});
+ deepEqual(root.returns.optional(), {size: 4},
+ "Missing optional properties is allowed");
+
+ if (AppConstants.DEBUG) {
+ Assert.throws(() => root.returns.invalid(),
+ `Type error for result value (Property "size" is required)`,
+ "Should throw for invalid result in DEBUG builds");
+ } else {
+ deepEqual(root.returns.invalid(), {},
+ "Doesn't throw for invalid result value in release builds");
+ }
+});
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas_revoke.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas_revoke.js
@@ -41,17 +41,17 @@ let json = [
{
id: "submodule",
type: "object",
functions: [
{
name: "sub_foo",
type: "function",
parameters: [],
- returns: "integer",
+ returns: {type: "integer"},
},
],
},
],
functions: [
{
name: "func",
@@ -116,16 +116,19 @@ class APIImplementation extends SchemaAP
}
revoke(...args) {
this.record("revoke", args);
}
callFunction(...args) {
this.record("callFunction", args);
+ if (this.name === "sub_foo") {
+ return 13;
+ }
}
callFunctionNoReturn(...args) {
this.record("callFunctionNoReturn", args);
}
getProperty(...args) {
this.record("getProperty", args);