Commit 90017b5f authored by yzshen's avatar yzshen Committed by Commit bot

Mojo C++ bindings: better log messages for some validation errors at the receiver side.

The CL provides more information in the log messages for the following errors:
- Fixed-size array has wrong number of elements.
- non-nullable field is set to null/invalid handle.

The reason to have better log messages for these errors is that they are
triggered when users (of the bindings) provide invalid arguments. The users
probably want to find out what they have done wrong. The other validation errors
are rarer, caused by bugs in the bindings or malicious senders. Providing better
log messages for them is of slightly lower priority.

BUG=324170
TEST=None

Review URL: https://codereview.chromium.org/494943004

Cr-Commit-Position: refs/heads/master@{#291704}
parent 895539fa
...@@ -216,7 +216,11 @@ struct ArraySerializationHelper<Handle, true> { ...@@ -216,7 +216,11 @@ struct ArraySerializationHelper<Handle, true> {
for (uint32_t i = 0; i < header->num_elements; ++i) { for (uint32_t i = 0; i < header->num_elements; ++i) {
if (!element_is_nullable && if (!element_is_nullable &&
elements[i].value() == kEncodedInvalidHandleValue) { elements[i].value() == kEncodedInvalidHandleValue) {
ReportValidationError(VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE); ReportValidationError(
VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE,
MakeMessageWithArrayIndex(
"invalid handle in array expecting valid handles",
header->num_elements, i).c_str());
return false; return false;
} }
if (!bounds_checker->ClaimHandle(elements[i])) { if (!bounds_checker->ClaimHandle(elements[i])) {
...@@ -280,7 +284,11 @@ struct ArraySerializationHelper<P*, false> { ...@@ -280,7 +284,11 @@ struct ArraySerializationHelper<P*, false> {
BoundsChecker* bounds_checker) { BoundsChecker* bounds_checker) {
for (uint32_t i = 0; i < header->num_elements; ++i) { for (uint32_t i = 0; i < header->num_elements; ++i) {
if (!element_is_nullable && !elements[i].offset) { if (!element_is_nullable && !elements[i].offset) {
ReportValidationError(VALIDATION_ERROR_UNEXPECTED_NULL_POINTER); ReportValidationError(
VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
MakeMessageWithArrayIndex(
"null in array expecting valid pointers",
header->num_elements, i).c_str());
return false; return false;
} }
if (!ValidateEncodedPointer(&elements[i].offset)) { if (!ValidateEncodedPointer(&elements[i].offset)) {
...@@ -356,7 +364,11 @@ class Array_Data { ...@@ -356,7 +364,11 @@ class Array_Data {
} }
if (Params::expected_num_elements != 0 && if (Params::expected_num_elements != 0 &&
header->num_elements != Params::expected_num_elements) { header->num_elements != Params::expected_num_elements) {
ReportValidationError(VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER); ReportValidationError(
VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER,
MakeMessageWithExpectedArraySize(
"fixed-size array has wrong number of elements",
header->num_elements, Params::expected_num_elements).c_str());
return false; return false;
} }
if (!bounds_checker->ClaimMemory(data, header->num_bytes)) { if (!bounds_checker->ClaimMemory(data, header->num_bytes)) {
......
...@@ -44,11 +44,15 @@ const char* ValidationErrorToString(ValidationError error) { ...@@ -44,11 +44,15 @@ const char* ValidationErrorToString(ValidationError error) {
return "Unknown error"; return "Unknown error";
} }
void ReportValidationError(ValidationError error) { void ReportValidationError(ValidationError error, const char* description) {
if (g_validation_error_observer) if (g_validation_error_observer) {
g_validation_error_observer->set_last_error(error); g_validation_error_observer->set_last_error(error);
else } else if (description) {
MOJO_LOG(ERROR) << "Invalid message: " << ValidationErrorToString(error)
<< " (" << description << ")";
} else {
MOJO_LOG(ERROR) << "Invalid message: " << ValidationErrorToString(error); MOJO_LOG(ERROR) << "Invalid message: " << ValidationErrorToString(error);
}
} }
ValidationErrorObserverForTesting::ValidationErrorObserverForTesting() ValidationErrorObserverForTesting::ValidationErrorObserverForTesting()
......
...@@ -48,7 +48,8 @@ enum ValidationError { ...@@ -48,7 +48,8 @@ enum ValidationError {
const char* ValidationErrorToString(ValidationError error); const char* ValidationErrorToString(ValidationError error);
void ReportValidationError(ValidationError error); void ReportValidationError(ValidationError error,
const char* description = NULL);
// Only used by validation tests and when there is only one thread doing message // Only used by validation tests and when there is only one thread doing message
// validation. // validation.
......
...@@ -19,7 +19,8 @@ ...@@ -19,7 +19,8 @@
{%- if not kind|is_nullable_kind %} {%- if not kind|is_nullable_kind %}
if (!object->{{name}}.offset) { if (!object->{{name}}.offset) {
ReportValidationError( ReportValidationError(
mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER); mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
"null {{name}} field in {{struct.name}} struct");
return false; return false;
} }
{%- endif %} {%- endif %}
...@@ -43,7 +44,8 @@ ...@@ -43,7 +44,8 @@
{%- if not kind|is_nullable_kind %} {%- if not kind|is_nullable_kind %}
if (object->{{name}}.value() == mojo::internal::kEncodedInvalidHandleValue) { if (object->{{name}}.value() == mojo::internal::kEncodedInvalidHandleValue) {
ReportValidationError( ReportValidationError(
mojo::internal::VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE); mojo::internal::VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE,
"invalid {{name}} field in {{struct.name}} struct");
return false; return false;
} }
{%- endif %} {%- endif %}
......
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