diff --git a/core/include/gz/msgs/PointCloudPackedUtils.hh b/core/include/gz/msgs/PointCloudPackedUtils.hh index f746f8c0..37074aaf 100644 --- a/core/include/gz/msgs/PointCloudPackedUtils.hh +++ b/core/include/gz/msgs/PointCloudPackedUtils.hh @@ -24,6 +24,7 @@ #include #include +#include #include #include #include diff --git a/core/src/DynamicFactory.hh b/core/src/DynamicFactory.hh index 342f3edb..afe4c4c4 100644 --- a/core/src/DynamicFactory.hh +++ b/core/src/DynamicFactory.hh @@ -30,6 +30,7 @@ #pragma warning(pop) #endif +#include #include #include #include diff --git a/core/src/MessageFactory.cc b/core/src/MessageFactory.cc index 827205dc..45743dc1 100644 --- a/core/src/MessageFactory.cc +++ b/core/src/MessageFactory.cc @@ -115,7 +115,12 @@ MessageFactory::MessagePtr MessageFactory::New( std::unique_ptr msg = New(_msgType); if (msg) { - google::protobuf::TextFormat::ParseFromString(_args, msg.get()); + if (!google::protobuf::TextFormat::ParseFromString(_args, msg.get())) + { + // The user-provided string was invalid, + // return nullptr rather than an empty message. + msg.reset(); + } } return msg; } diff --git a/test/integration/Factory_TEST.cc b/test/integration/Factory_TEST.cc index 96d9433d..7f8a08fe 100644 --- a/test/integration/Factory_TEST.cc +++ b/test/integration/Factory_TEST.cc @@ -53,23 +53,10 @@ TEST(FactoryTest, Type) ///////////////////////////////////////////////// TEST(FactoryTest, New) { - // Correct call, using periods, fully qualified + // Preferred call, using periods, fully qualified auto msg = Factory::New("gz.msgs.Vector3d"); - ASSERT_TRUE(msg.get() != nullptr); - msg->set_x(1.0); - msg->set_y(2.0); - msg->set_z(3.0); - - auto msgFilled = Factory::New( - "gz_msgs.Vector3d", "x: 1.0, y: 2.0, z: 3.0"); - ASSERT_TRUE(msgFilled.get() != nullptr); - - EXPECT_DOUBLE_EQ(msg->x(), msgFilled->x()); - EXPECT_DOUBLE_EQ(msg->y(), msgFilled->y()); - EXPECT_DOUBLE_EQ(msg->z(), msgFilled->z()); - // Various other supported ways of specifying gz.msgs msg = Factory::New("gz_msgs.Vector3d"); EXPECT_TRUE(msg.get() != nullptr); @@ -81,6 +68,33 @@ TEST(FactoryTest, New) EXPECT_TRUE(msg.get() != nullptr); } +///////////////////////////////////////////////// +TEST(FactoryTest, NewWithWellFormedData) +{ + gz::msgs::Vector3d msg; + msg.set_x(1.0); + msg.set_y(2.0); + msg.set_z(3.0); + + auto msgFilled = Factory::New( + "gz.msgs.Vector3d", "x: 1.0, y: 2.0, z: 3.0"); + ASSERT_TRUE(nullptr != msgFilled); + + EXPECT_DOUBLE_EQ(msg.x(), msgFilled->x()); + EXPECT_DOUBLE_EQ(msg.y(), msgFilled->y()); + EXPECT_DOUBLE_EQ(msg.z(), msgFilled->z()); +} + + +///////////////////////////////////////////////// +TEST(FactoryTest, NewWithMalformedData) +{ + /// Passing bad data to New results in a nullptr + auto msgFilled = Factory::New( + "gz.msgs.Vector3d", "x: 1.0, y: asdf, z: 3.0"); + ASSERT_TRUE(nullptr == msgFilled); +} + ///////////////////////////////////////////////// TEST(FactoryTest, DeprecatedNonFullyQualified) {