buffet: Fix crash when CommandInstance::SetResults() is invoked
The crash was a result of use-after-delete in
CloudCommandProxy::OnResultsChanged() where TypedValueToJson returned
std::unique_ptr while the JSON value was set to Dictionary by using
unique_ptr::get() on the temporary. Should have called release()
instead to transfer the ownership to the dictionary.
Added a unit test to buffet/device_registration_info_unittest.cc to
test this specific scenario.
BUG=brillo:770
TEST=`FEATURES=test emerge-link buffet`
Change-Id: I34a28ce610a06f2b77af1cc75e358b3fb3562a13
Reviewed-on: https://chromium-review.googlesource.com/264465
Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index 91dc8b1..6136eeb 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -5,6 +5,7 @@
#include "buffet/device_registration_info.h"
#include <base/json/json_reader.h>
+#include <base/json/json_writer.h>
#include <base/values.h>
#include <chromeos/bind_lambda.h>
#include <chromeos/http/http_request.h>
@@ -179,6 +180,10 @@
static bool Save(DeviceRegistrationInfo* info) {
return info->Save();
}
+ static void PublishCommands(DeviceRegistrationInfo* info,
+ const base::ListValue& commands) {
+ return info->PublishCommands(commands);
+ }
};
class DeviceRegistrationInfoTest : public ::testing::Test {
@@ -188,7 +193,7 @@
storage_ = std::make_shared<MemStorage>();
storage_->Save(&data_);
transport_ = std::make_shared<chromeos::http::fake::Transport>();
- command_manager_ = std::make_shared<CommandManager>();
+ command_manager_ = std::make_shared<CommandManager>(nullptr);
state_manager_ = std::make_shared<StateManager>(&mock_state_change_queue_);
chromeos::KeyValueStore config_store;
config_store.SetString("client_id", test_data::kClientId);
@@ -213,6 +218,8 @@
transport_, storage_,
true,
mock_callback));
+ EXPECT_CALL(*this, OnRegistrationStatusChange())
+ .Times(testing::AnyNumber());
}
MOCK_METHOD0(OnRegistrationStatusChange, void());
@@ -503,4 +510,52 @@
EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus());
}
+TEST_F(DeviceRegistrationInfoTest, UpdateCommand) {
+ EXPECT_TRUE(dev_reg_->Load());
+ auto json_cmds = buffet::unittests::CreateDictionaryValue(R"({
+ 'robot': {
+ '_jump': {
+ 'parameters': {'_height': 'integer'},
+ 'results': {'status': 'string'}
+ }
+ }
+ })");
+ EXPECT_TRUE(command_manager_->LoadCommands(*json_cmds, "", nullptr));
+
+ auto commands_json = buffet::unittests::CreateValue(R"([{
+ 'name':'robot._jump',
+ 'id':'1234',
+ 'parameters': {'_height': 100}
+ }])");
+ ASSERT_NE(nullptr, commands_json.get());
+ const base::ListValue* command_list = nullptr;
+ ASSERT_TRUE(commands_json->GetAsList(&command_list));
+ DeviceRegistrationInfo::TestHelper::PublishCommands(dev_reg_.get(),
+ *command_list);
+ auto command = command_manager_->FindCommand("1234");
+ ASSERT_NE(nullptr, command);
+ StringPropType string_type;
+ native_types::Object results{
+ {"status", string_type.CreateValue(std::string{"Ok"}, nullptr)}
+ };
+
+ auto update_command = [](const ServerRequest& request,
+ ServerResponse* response) {
+ // Make sure we serialize the JSON back without any pretty print so
+ // the string comparison works correctly.
+ auto json = request.GetDataAsJson();
+ EXPECT_NE(nullptr, json.get());
+ std::string value;
+ ASSERT_TRUE(base::JSONWriter::Write(json.get(), &value));
+ EXPECT_EQ(R"({"results":{"status":"Ok"}})", value);
+ };
+
+ transport_->AddHandler(dev_reg_->GetServiceURL("commands/1234"),
+ chromeos::http::request_type::kPatch,
+ base::Bind(update_command));
+
+ command->SetResults(results);
+}
+
+
} // namespace buffet
