Index: extensions/renderer/argument_spec.cc |
diff --git a/extensions/renderer/argument_spec.cc b/extensions/renderer/argument_spec.cc |
index fd3e1a973a923855127aaa0f1ae9574b8ea9833f..3e5a527a1d0f978e6f6fb1f833dc65200f26b638 100644 |
--- a/extensions/renderer/argument_spec.cc |
+++ b/extensions/renderer/argument_spec.cc |
@@ -5,9 +5,11 @@ |
#include "extensions/renderer/argument_spec.h" |
#include "base/memory/ptr_util.h" |
+#include "base/strings/string_util.h" |
#include "base/strings/stringprintf.h" |
#include "base/values.h" |
#include "content/public/child/v8_value_converter.h" |
+#include "extensions/renderer/api_invocation_errors.h" |
#include "extensions/renderer/api_type_reference_map.h" |
#include "gin/converter.h" |
#include "gin/dictionary.h" |
@@ -16,21 +18,52 @@ namespace extensions { |
namespace { |
+// Returns a type string for the given |value|. |
+const char* GetV8ValueTypeString(v8::Local<v8::Value> value) { |
+ DCHECK(!value.IsEmpty()); |
+ |
+ if (value->IsNull()) |
+ return api_errors::kTypeNull; |
+ if (value->IsUndefined()) |
+ return api_errors::kTypeUndefined; |
+ if (value->IsInt32()) |
+ return api_errors::kTypeInteger; |
+ if (value->IsNumber()) |
+ return api_errors::kTypeDouble; |
+ if (value->IsBoolean()) |
+ return api_errors::kTypeBoolean; |
+ if (value->IsString()) |
+ return api_errors::kTypeString; |
+ |
+ // Note: check IsArray(), IsFunction(), and IsArrayBuffer[View]() before |
+ // IsObject() since arrays, functions, and array buffers are objects. |
+ if (value->IsArray()) |
+ return api_errors::kTypeList; |
+ if (value->IsFunction()) |
+ return api_errors::kTypeFunction; |
+ if (value->IsArrayBuffer() || value->IsArrayBufferView()) |
+ return api_errors::kTypeBinary; |
+ if (value->IsObject()) |
+ return api_errors::kTypeObject; |
+ |
+ return ""; |
+} |
+ |
+// Returns true if |value| is within the bounds specified by |minimum|, |
lazyboy
2017/04/25 18:23:12
within the bounds [|minimum|, |maximum|]
Devlin
2017/04/25 22:47:59
Done.
|
+// populating |error| otherwise. |
template <class T> |
-bool ParseFundamentalValueHelper(v8::Local<v8::Value> arg, |
- v8::Local<v8::Context> context, |
- const base::Optional<int>& minimum, |
- const base::Optional<int>& maximum, |
- std::unique_ptr<base::Value>* out_value) { |
- T val; |
- if (!gin::Converter<T>::FromV8(context->GetIsolate(), arg, &val)) |
- return false; |
- if (minimum && val < *minimum) |
+bool CheckFundamentalBounds(T value, |
+ const base::Optional<int>& minimum, |
+ const base::Optional<int>& maximum, |
+ std::string* error) { |
+ if (minimum && value < *minimum) { |
+ *error = api_errors::GetError(api_errors::kNumberTooSmall, *minimum); |
return false; |
- if (maximum && val > *maximum) |
+ } |
+ if (maximum && value > *maximum) { |
+ *error = api_errors::GetError(api_errors::kNumberTooLarge, *maximum); |
return false; |
- if (out_value) |
- *out_value = base::MakeUnique<base::Value>(val); |
+ } |
return true; |
} |
@@ -168,8 +201,10 @@ bool ArgumentSpec::ParseArgument(v8::Local<v8::Context> context, |
std::unique_ptr<base::Value>* out_value, |
std::string* error) const { |
if (type_ == ArgumentType::FUNCTION) { |
- if (!value->IsFunction()) |
+ if (!value->IsFunction()) { |
+ *error = GetInvalidTypeError(value); |
return false; |
+ } |
if (out_value) { |
// Certain APIs (contextMenus) have functions as parameters other than the |
@@ -194,7 +229,7 @@ bool ArgumentSpec::ParseArgument(v8::Local<v8::Context> context, |
if (choice->ParseArgument(context, value, refs, out_value, error)) |
return true; |
} |
- *error = "Did not match any of the choices"; |
+ *error = api_errors::kInvalidChoice; |
return false; |
} |
@@ -207,7 +242,7 @@ bool ArgumentSpec::ParseArgument(v8::Local<v8::Context> context, |
// 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"; |
+ *error = GetInvalidTypeError(value); |
return false; |
} |
v8::Local<v8::Object> object = value.As<v8::Object>(); |
@@ -215,7 +250,7 @@ bool ArgumentSpec::ParseArgument(v8::Local<v8::Context> context, |
} |
if (type_ == ArgumentType::LIST) { |
if (!value->IsArray()) { |
- *error = "Wrong type"; |
+ *error = GetInvalidTypeError(value); |
return false; |
} |
v8::Local<v8::Array> array = value.As<v8::Array>(); |
@@ -223,7 +258,7 @@ bool ArgumentSpec::ParseArgument(v8::Local<v8::Context> context, |
} |
if (type_ == ArgumentType::BINARY) { |
if (!value->IsArrayBuffer() && !value->IsArrayBufferView()) { |
- *error = "Wrong type"; |
+ *error = GetInvalidTypeError(value); |
return false; |
} |
return ParseArgumentToAny(context, value, out_value, error); |
@@ -245,26 +280,49 @@ bool ArgumentSpec::ParseArgumentToFundamental( |
std::unique_ptr<base::Value>* out_value, |
std::string* error) const { |
DCHECK(IsFundamentalType()); |
+ |
switch (type_) { |
- case ArgumentType::INTEGER: |
- return ParseFundamentalValueHelper<int32_t>(value, context, minimum_, |
- maximum_, out_value); |
- case ArgumentType::DOUBLE: |
- return ParseFundamentalValueHelper<double>(value, context, minimum_, |
- maximum_, out_value); |
+ case ArgumentType::INTEGER: { |
+ if (!value->IsInt32()) { |
+ *error = GetInvalidTypeError(value); |
+ return false; |
+ } |
+ int int_val = value.As<v8::Int32>()->Value(); |
+ if (!CheckFundamentalBounds(int_val, minimum_, maximum_, error)) |
+ return false; |
+ if (out_value) |
+ *out_value = base::MakeUnique<base::Value>(int_val); |
+ return true; |
+ } |
+ case ArgumentType::DOUBLE: { |
+ if (!value->IsNumber()) { |
+ *error = GetInvalidTypeError(value); |
+ return false; |
+ } |
+ double double_val = value.As<v8::Number>()->Value(); |
+ if (!CheckFundamentalBounds(double_val, minimum_, maximum_, error)) |
+ return false; |
+ if (out_value) |
+ *out_value = base::MakeUnique<base::Value>(double_val); |
+ return true; |
+ } |
case ArgumentType::STRING: { |
- if (!value->IsString()) |
+ if (!value->IsString()) { |
+ *error = GetInvalidTypeError(value); |
return false; |
+ } |
v8::Local<v8::String> v8_string = value.As<v8::String>(); |
size_t length = static_cast<size_t>(v8_string->Length()); |
if (min_length_ && length < *min_length_) { |
- *error = "Less than min length"; |
+ *error = |
+ api_errors::GetError(api_errors::kTooFewStringChars, *min_length_); |
return false; |
} |
if (max_length_ && length > *max_length_) { |
- *error = "Greater than max length"; |
+ *error = |
+ api_errors::GetError(api_errors::kTooManyStringChars, *max_length_); |
return false; |
} |
@@ -277,8 +335,14 @@ bool ArgumentSpec::ParseArgumentToFundamental( |
// We already checked that this is a string, so this should never fail. |
CHECK(gin::Converter<std::string>::FromV8(context->GetIsolate(), value, |
&s)); |
- if (!enum_values_.empty() && enum_values_.count(s) == 0) |
+ if (!enum_values_.empty() && enum_values_.count(s) == 0) { |
+ std::vector<base::StringPiece> options(enum_values_.begin(), |
+ enum_values_.end()); |
+ std::string options_str = base::JoinString(options, ", "); |
+ *error = api_errors::GetError(api_errors::kInvalidEnumValue, |
+ options_str.c_str()); |
return false; |
+ } |
if (out_value) { |
// TODO(devlin): If base::Value ever takes a std::string&&, we |
// could use std::move to construct. |
@@ -287,8 +351,10 @@ bool ArgumentSpec::ParseArgumentToFundamental( |
return true; |
} |
case ArgumentType::BOOLEAN: { |
- if (!value->IsBoolean()) |
+ if (!value->IsBoolean()) { |
+ *error = GetInvalidTypeError(value); |
return false; |
+ } |
if (out_value) { |
*out_value = |
base::MakeUnique<base::Value>(value.As<v8::Boolean>()->Value()); |
@@ -314,17 +380,22 @@ bool ArgumentSpec::ParseArgumentToObject( |
result = base::MakeUnique<base::DictionaryValue>(); |
v8::Local<v8::Array> own_property_names; |
- if (!object->GetOwnPropertyNames(context).ToLocal(&own_property_names)) |
+ if (!object->GetOwnPropertyNames(context).ToLocal(&own_property_names)) { |
+ *error = api_errors::kScriptThrewError; |
return false; |
+ } |
// Track all properties we see from |properties_| to check if any are missing. |
// Use ArgumentSpec* instead of std::string for comparison + copy efficiency. |
std::set<const ArgumentSpec*> seen_properties; |
uint32_t length = own_property_names->Length(); |
+ std::string property_error; |
for (uint32_t i = 0; i < length; ++i) { |
v8::Local<v8::Value> key; |
- if (!own_property_names->Get(context, i).ToLocal(&key)) |
+ if (!own_property_names->Get(context, i).ToLocal(&key)) { |
+ *error = api_errors::kScriptThrewError; |
return false; |
+ } |
// In JS, all keys are strings or numbers (or symbols, but those are |
// excluded by GetOwnPropertyNames()). If you try to set anything else |
// (e.g. an object), it is converted to a string. |
@@ -339,7 +410,7 @@ bool ArgumentSpec::ParseArgumentToObject( |
} else if (additional_properties_) { |
property_spec = additional_properties_.get(); |
} else { |
- *error = base::StringPrintf("Unknown property: %s", *utf8_key); |
+ *error = api_errors::GetError(api_errors::kUnexpectedProperty, *utf8_key); |
return false; |
} |
@@ -359,7 +430,8 @@ bool ArgumentSpec::ParseArgumentToObject( |
// TODO(devlin): This matches current behavior, but it is correct? |
if (prop_value->IsUndefined() || prop_value->IsNull()) { |
if (!property_spec->optional_) { |
- *error = base::StringPrintf("Missing key: %s", *utf8_key); |
+ *error = api_errors::GetError(api_errors::kMissingRequiredProperty, |
+ *utf8_key); |
return false; |
} |
continue; |
@@ -367,7 +439,9 @@ bool ArgumentSpec::ParseArgumentToObject( |
std::unique_ptr<base::Value> property; |
if (!property_spec->ParseArgument(context, prop_value, refs, |
- result ? &property : nullptr, error)) { |
+ result ? &property : nullptr, |
+ &property_error)) { |
+ *error = api_errors::GetPropertyError(*utf8_key, property_error); |
return false; |
} |
if (result) |
@@ -377,7 +451,8 @@ bool ArgumentSpec::ParseArgumentToObject( |
for (const auto& pair : properties_) { |
const ArgumentSpec* spec = pair.second.get(); |
if (!spec->optional_ && seen_properties.count(spec) == 0) { |
- *error = "Missing key: " + pair.first; |
+ *error = api_errors::GetError(api_errors::kMissingRequiredProperty, |
+ pair.first.c_str()); |
return false; |
} |
} |
@@ -410,7 +485,8 @@ bool ArgumentSpec::ParseArgumentToObject( |
} while (next_check->IsObject()); |
if (!found) { |
- *error = "Object is not of correct instance"; |
+ *error = api_errors::GetError(api_errors::kNotAnInstance, |
+ instance_of_->c_str()); |
return false; |
} |
} |
@@ -429,12 +505,12 @@ bool ArgumentSpec::ParseArgumentToArray(v8::Local<v8::Context> context, |
uint32_t length = value->Length(); |
if (min_length_ && length < *min_length_) { |
- *error = "Less than min length"; |
+ *error = api_errors::GetError(api_errors::kTooFewArrayItems, *min_length_); |
return false; |
} |
if (max_length_ && length > *max_length_) { |
- *error = "Greater than max length"; |
+ *error = api_errors::GetError(api_errors::kTooManyArrayItems, *max_length_); |
return false; |
} |
@@ -443,6 +519,7 @@ bool ArgumentSpec::ParseArgumentToArray(v8::Local<v8::Context> context, |
if (out_value) |
result = base::MakeUnique<base::ListValue>(); |
+ std::string item_error; |
for (uint32_t i = 0; i < length; ++i) { |
v8::MaybeLocal<v8::Value> maybe_subvalue = value->Get(context, i); |
v8::Local<v8::Value> subvalue; |
@@ -456,8 +533,9 @@ bool ArgumentSpec::ParseArgumentToArray(v8::Local<v8::Context> context, |
if (!maybe_subvalue.ToLocal(&subvalue)) |
return false; |
std::unique_ptr<base::Value> item; |
- if (!list_element_type_->ParseArgument(context, subvalue, refs, |
- result ? &item : nullptr, error)) { |
+ if (!list_element_type_->ParseArgument( |
+ context, subvalue, refs, result ? &item : nullptr, &item_error)) { |
+ *error = api_errors::GetIndexError(i, item_error); |
return false; |
} |
if (result) |
@@ -479,7 +557,7 @@ bool ArgumentSpec::ParseArgumentToAny(v8::Local<v8::Context> context, |
std::unique_ptr<base::Value> converted( |
converter->FromV8Value(value, context)); |
if (!converted) { |
- *error = "Could not convert to 'any'."; |
+ *error = api_errors::kUnserializableValue; |
return false; |
} |
if (type_ == ArgumentType::BINARY) |
@@ -489,4 +567,44 @@ bool ArgumentSpec::ParseArgumentToAny(v8::Local<v8::Context> context, |
return true; |
} |
+std::string ArgumentSpec::GetInvalidTypeError( |
+ v8::Local<v8::Value> value) const { |
+ base::StringPiece expected_type; |
+ switch (type_) { |
+ case ArgumentType::INTEGER: |
+ expected_type = api_errors::kTypeInteger; |
+ break; |
+ case ArgumentType::DOUBLE: |
+ expected_type = api_errors::kTypeDouble; |
+ break; |
+ case ArgumentType::BOOLEAN: |
+ expected_type = api_errors::kTypeBoolean; |
+ break; |
+ case ArgumentType::STRING: |
+ expected_type = api_errors::kTypeString; |
+ break; |
+ case ArgumentType::OBJECT: |
+ expected_type = instance_of_ ? *instance_of_ : api_errors::kTypeObject; |
+ break; |
+ case ArgumentType::LIST: |
+ expected_type = api_errors::kTypeList; |
+ break; |
+ case ArgumentType::BINARY: |
+ expected_type = api_errors::kTypeBinary; |
+ break; |
+ case ArgumentType::FUNCTION: |
+ expected_type = api_errors::kTypeFunction; |
+ break; |
+ case ArgumentType::REF: |
+ expected_type = *ref_; |
+ break; |
+ case ArgumentType::CHOICES: |
+ case ArgumentType::ANY: |
+ NOTREACHED(); |
+ } |
+ |
+ return api_errors::GetError(api_errors::kInvalidType, expected_type.data(), |
+ GetV8ValueTypeString(value)); |
+} |
+ |
} // namespace extensions |