Commit d9293175 authored by rdevlin.cronin's avatar rdevlin.cronin Committed by Commit bot

[Extension Bindings] Don't allow arrays or functions as object params

In JavaScript (and v8), functions and arrays are both considered
objects. However, for the sake of argument matching in extension APIs,
we shouldn't consider them as such. This is to help in our ambiguous
argument matching, where we allow optional inner parameters.

For instance, given an API function that takes an int, an optional
object with no required properties, and an optional callback, the
system wouldn't necessarily be able to tell that, given a call of
foo(1, function() {}), it was passed the callback rather than the
object. This is exactly the scenario that arises with tabs.sendMessage.

While we could potentially make our matching more intelligent to
account for this case, it doesn't seem worth the extra complexity (and
speed cost). Instead, simply don't allow functions or arrays as
object parameters.

Add tests for the same.

BUG=653596

Review-Url: https://codereview.chromium.org/2621883002
Cr-Commit-Position: refs/heads/master@{#442671}
parent 21b1c08f
This diff is collapsed.
......@@ -152,9 +152,12 @@ bool ArgumentSpec::ParseArgument(v8::Local<v8::Context> context,
if (IsFundamentalType())
return ParseArgumentToFundamental(context, value, out_value, error);
if (type_ == ArgumentType::OBJECT) {
// TODO(devlin): Currently, this would accept an array (if that array had
// all the requisite properties). Is that the right thing to do?
if (!value->IsObject()) {
// Don't allow functions or arrays (even though they are technically
// objects). This is to make it easier to match otherwise-ambiguous
// signatures. For instance, if an API method has an optional object
// parameter and then an optional callback, we wouldn't necessarily be able
// to match the arguments if we allowed functions as objects.
if (!value->IsObject() || value->IsFunction() || value->IsArray()) {
*error = "Wrong type";
return false;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment