mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [mesos] cf-natali opened a new pull request #356: libprocess: check protobuf (de)serialisation success.
Date Tue, 07 Apr 2020 00:34:13 GMT
cf-natali opened a new pull request #356: libprocess: check protobuf (de)serialisation success.
URL: https://github.com/apache/mesos/pull/356
 
 
   Before the code didn't check at all the return value of
   `Message::SerializeToString`, which can fail for various reasons,
   e.g. out-of-memory, message too large, or invalid UTF-8 string.
   Also, the way deserialisation was checked for error using
   `Message::IsInitialized` doesn't detect errors such as the above,
   we need to check `Message::ParseFromString` return value.
   
   See example attached:
   ```
   $ # message too large
   $ make check 
   protoc test.proto --cpp_out=.
   g++ -o test test.cpp test.pb.cc  -lprotobuf -lpthread
   ./test
   [libprotobuf ERROR google/protobuf/message_lite.cc:289] Exceeded maximum protobuf size
of 2GB: 4294967298
   SerializeToString: false
   deserialising garbage
   ParseFromString: false
   IsInitialized: true
   ```
   
   ```
   $ # invalid utf-8 string
   $ make check 
   protoc test.proto --cpp_out=.
   g++ -o test test.cpp test.pb.cc  -lprotobuf -lpthread
   ./test
   [libprotobuf ERROR google/protobuf/wire_format_lite.cc:626] String field 'test.Test.payload'
contains invalid UTF-8 data when serializing a protocol buffer. Use the 'bytes' type if you
intend to send raw bytes. 
   SerializeToString: true
   [libprotobuf ERROR google/protobuf/wire_format_lite.cc:626] String field 'test.Test.payload'
contains invalid UTF-8 data when parsing a protocol buffer. Use the 'bytes' type if you intend
to send raw bytes. 
   ParseFromString: false
   IsInitialized: true
   deserialising garbage
   ParseFromString: false
   IsInitialized: true
   ```
   
   [test.zip](https://github.com/apache/mesos/files/4441407/test.zip)
   
   
   We noticed this at work because our custom executor had a bug causing it to send invalid/non-UTF8
mesos.TaskID, but it was successfully serialised by the executor (driver), and deserialised
by the framework, which was blowing it to blow up at later point far from the original source
of the problem.
   
   More generally we want to catch such invalid messages - which can happen for a variety
of reasons - as early as possible.
   
   @bmahler 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message