This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new e6b15b1e Fixed protobuf (de)serialization error handling in libprocess.
e6b15b1e is described below
commit e6b15b1e968649c794d67bb97961e8f0216e2ed8
Author: Charles-Francois Natali <[email protected]>
AuthorDate: Tue Apr 7 18:33:12 2020 -0400
Fixed protobuf (de)serialization error handling in libprocess.
`MessageLite::SerializeToString` can return false for messages that
are too large (i.e. >2GB). For serializing invalid UTF-8:
"proto3": serialization will succeed but invalid UTF-8 will be logged.
"proto2": serialization will succeed. Invalid UTF-8 will only be
logged in debug mode (NDEBUG not defined).
`MessageLite::ParseFromString` can return false for missing required
fields, excess data unconsumed in the string, invalid varint, or
a string/submessage size being out of bounds. For invalid UTF-8:
"proto3": de-serialization will fail and it will be logged.
"proto2": de-serialization will succeed. Invalid UTF-8 will only be
logged in debug mode (NDEBUG not defined).
The existing code misses the false cases, and only happens to check
the missing required fields case in the de-serialization code.
This closes #356
---
3rdparty/libprocess/include/process/protobuf.hpp | 72 +++++++++++++-----------
1 file changed, 39 insertions(+), 33 deletions(-)
diff --git a/3rdparty/libprocess/include/process/protobuf.hpp
b/3rdparty/libprocess/include/process/protobuf.hpp
index 45ac3aa..87c40cd 100644
--- a/3rdparty/libprocess/include/process/protobuf.hpp
+++ b/3rdparty/libprocess/include/process/protobuf.hpp
@@ -39,8 +39,12 @@ inline void post(const process::UPID& to,
const google::protobuf::Message& message)
{
std::string data;
- message.SerializeToString(&data);
- post(to, message.GetTypeName(), data.data(), data.size());
+ if (message.SerializeToString(&data)) {
+ post(to, message.GetTypeName(), data.data(), data.size());
+ } else {
+ LOG(ERROR) << "Failed to post '" << message.GetTypeName() << "' to "
+ << to << ": Failed to serialize";
+ }
}
@@ -49,8 +53,12 @@ inline void post(const process::UPID& from,
const google::protobuf::Message& message)
{
std::string data;
- message.SerializeToString(&data);
- post(from, to, message.GetTypeName(), data.data(), data.size());
+ if (message.SerializeToString(&data)) {
+ post(from, to, message.GetTypeName(), data.data(), data.size());
+ } else {
+ LOG(ERROR) << "Failed to post '" << message.GetTypeName() << "' to "
+ << to << ": Failed to serialize";
+ }
}
} // namespace process {
@@ -119,8 +127,12 @@ protected:
const google::protobuf::Message& message)
{
std::string data;
- message.SerializeToString(&data);
- process::Process<T>::send(to, message.GetTypeName(), std::move(data));
+ if (message.SerializeToString(&data)) {
+ process::Process<T>::send(to, message.GetTypeName(), std::move(data));
+ } else {
+ LOG(ERROR) << "Failed to send '" << message.GetTypeName() << "' to "
+ << to << ": Failed to serialize";
+ }
}
using process::Process<T>::send;
@@ -261,13 +273,12 @@ private:
{
google::protobuf::Arena arena;
M* m = CHECK_NOTNULL(google::protobuf::Arena::CreateMessage<M>(&arena));
- m->ParseFromString(data);
- if (m->IsInitialized()) {
+ if (m->ParseFromString(data)) {
(t->*method)(sender, *m);
} else {
- LOG(WARNING) << "Initialization errors: "
- << m->InitializationErrorString();
+ LOG(ERROR) << "Failed to deserialize '" << m->GetTypeName()
+ << "' from " << sender;
}
}
@@ -279,13 +290,12 @@ private:
const std::string& data)
{
M m;
- m.ParseFromString(data);
- if (m.IsInitialized()) {
+ if (m.ParseFromString(data)) {
(t->*method)(sender, std::move(m));
} else {
- LOG(WARNING) << "Initialization errors: "
- << m.InitializationErrorString();
+ LOG(ERROR) << "Failed to deserialize '" << m.GetTypeName()
+ << "' from " << sender;
}
}
@@ -309,13 +319,12 @@ private:
{
google::protobuf::Arena arena;
M* m = CHECK_NOTNULL(google::protobuf::Arena::CreateMessage<M>(&arena));
- m->ParseFromString(data);
- if (m->IsInitialized()) {
+ if (m->ParseFromString(data)) {
(t->*method)(sender, google::protobuf::convert((m->*p)())...);
} else {
- LOG(WARNING) << "Initialization errors: "
- << m->InitializationErrorString();
+ LOG(ERROR) << "Failed to deserialize '" << m->GetTypeName()
+ << "' from " << sender;
}
}
@@ -324,18 +333,17 @@ private:
static void _handlerM(
T* t,
void (T::*method)(const M&),
- const process::UPID&,
+ const process::UPID& sender,
const std::string& data)
{
google::protobuf::Arena arena;
M* m = CHECK_NOTNULL(google::protobuf::Arena::CreateMessage<M>(&arena));
- m->ParseFromString(data);
- if (m->IsInitialized()) {
+ if (m->ParseFromString(data)) {
(t->*method)(*m);
} else {
- LOG(WARNING) << "Initialization errors: "
- << m->InitializationErrorString();
+ LOG(ERROR) << "Failed to deserialize '" << m->GetTypeName()
+ << "' from " << sender;
}
}
@@ -343,17 +351,16 @@ private:
static void _handlerMutM(
T* t,
void (T::*method)(M&&),
- const process::UPID&,
+ const process::UPID& sender,
const std::string& data)
{
M m;
- m.ParseFromString(data);
- if (m.IsInitialized()) {
+ if (m.ParseFromString(data)) {
(t->*method)(std::move(m));
} else {
- LOG(WARNING) << "Initialization errors: "
- << m.InitializationErrorString();
+ LOG(ERROR) << "Failed to deserialize '" << m.GetTypeName()
+ << "' from " << sender;
}
}
@@ -371,19 +378,18 @@ private:
static void _handlerN(
T* t,
void (T::*method)(PC...),
- const process::UPID&,
+ const process::UPID& sender,
const std::string& data,
MessageProperty<M, P>... p)
{
google::protobuf::Arena arena;
M* m = CHECK_NOTNULL(google::protobuf::Arena::CreateMessage<M>(&arena));
- m->ParseFromString(data);
- if (m->IsInitialized()) {
+ if (m->ParseFromString(data)) {
(t->*method)(google::protobuf::convert((m->*p)())...);
} else {
- LOG(WARNING) << "Initialization errors: "
- << m->InitializationErrorString();
+ LOG(ERROR) << "Failed to deserialize '" << m->GetTypeName()
+ << "' from " << sender;
}
}
