buffet: Don't crash if user is passing bad parameters via D-Bus.
With this change, 'buffet_client RegisterDevice' now returns useful
information and the buffet daemon doesn't crash:
# buffet_client RegisterDevice
[0212/144637:ERROR:logging.h(777)] Failed to call method: org.chromium.Buffet.Manager.RegisterDevice: object_path= /org/chromium/Buffet/Manager: org.freedesktop.DBus.Error.Failed: buffet/missing_parameter:Parameter ticket_id not specified
[0212/144637:ERROR:dbus_method_invoker.h(110)] CallMethodAndBlockWithTimeout(...): Domain=dbus, Code=org.freedesktop.DBus.Error.Failed, Message=buffet/missing_parameter:Parameter ticket_id not specified
Failed to receive a response: buffet/missing_parameter:Parameter ticket_id not specified
BUG=brillo:193
TEST=Manually tested.
Change-Id: I70a90293f5f8226cdaaf1f5bf0fbe46b8984b23a
Reviewed-on: https://chromium-review.googlesource.com/249340
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 187ec39..413dc49 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -350,33 +350,25 @@
return std::unique_ptr<base::Value>(json.release());
}
-bool CheckParam(const std::string& param_name,
- const std::string& param_value,
- chromeos::ErrorPtr* error) {
- if (!param_value.empty())
- return true;
-
- chromeos::Error::AddToPrintf(error, FROM_HERE, kErrorDomainBuffet,
- "missing_parameter",
- "Parameter %s not specified",
- param_name.c_str());
- return false;
-}
-
std::string DeviceRegistrationInfo::RegisterDevice(
const std::map<std::string, std::string>& params,
chromeos::ErrorPtr* error) {
- GetParamValue(params, "ticket_id", &ticket_id_);
- GetParamValue(params, storage_keys::kClientId, &client_id_);
- GetParamValue(params, storage_keys::kClientSecret, &client_secret_);
- GetParamValue(params, storage_keys::kApiKey, &api_key_);
- GetParamValue(params, storage_keys::kDeviceKind, &device_kind_);
- GetParamValue(params, storage_keys::kName, &name_);
- GetParamValue(params, storage_keys::kDisplayName, &display_name_);
- GetParamValue(params, storage_keys::kDescription, &description_);
- GetParamValue(params, storage_keys::kLocation, &location_);
- GetParamValue(params, storage_keys::kOAuthURL, &oauth_url_);
- GetParamValue(params, storage_keys::kServiceURL, &service_url_);
+ if (!GetParamValue(params, "ticket_id", &ticket_id_, error) ||
+ !GetParamValue(params, storage_keys::kClientId, &client_id_, error) ||
+ !GetParamValue(params, storage_keys::kClientSecret, &client_secret_,
+ error) ||
+ !GetParamValue(params, storage_keys::kApiKey, &api_key_, error) ||
+ !GetParamValue(params, storage_keys::kDeviceKind, &device_kind_,
+ error) ||
+ !GetParamValue(params, storage_keys::kName, &name_, error) ||
+ !GetParamValue(params, storage_keys::kDisplayName, &display_name_,
+ error) ||
+ !GetParamValue(params, storage_keys::kDescription, &description_,
+ error) ||
+ !GetParamValue(params, storage_keys::kLocation, &location_, error) ||
+ !GetParamValue(params, storage_keys::kOAuthURL, &oauth_url_, error) ||
+ !GetParamValue(params, storage_keys::kServiceURL, &service_url_, error))
+ return std::string();
std::unique_ptr<base::DictionaryValue> device_draft =
BuildDeviceResource(error);
@@ -804,18 +796,26 @@
base::Bind(&IgnoreCloudResult), base::Bind(&IgnoreCloudError));
}
-void DeviceRegistrationInfo::GetParamValue(
+bool DeviceRegistrationInfo::GetParamValue(
const std::map<std::string, std::string>& params,
const std::string& param_name,
- std::string* param_value) {
+ std::string* param_value,
+ chromeos::ErrorPtr* error) {
auto p = params.find(param_name);
if (p != params.end()) {
*param_value = p->second;
- return;
+ return true;
}
- bool present = config_store_->GetString(param_name, param_value);
- CHECK(present) << "No default for parameter " << param_name;
+ bool default_was_set = config_store_->GetString(param_name, param_value);
+ if (default_was_set)
+ return true;
+
+ chromeos::Error::AddToPrintf(error, FROM_HERE, kErrorDomainBuffet,
+ "missing_parameter",
+ "Parameter %s not specified",
+ param_name.c_str());
+ return false;
}
} // namespace buffet
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index dc3524a..4c9cfaa 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -157,10 +157,17 @@
void PublishStateUpdates();
- void GetParamValue(
+ // Looks up the value for parameter with name |param_name| in
+ // |params|, supplying a default value if one is available and
+ // |params| doesn't have a value for |param_name|. The value will be
+ // returned in |param_value|. Returns |true| if a value was set
+ // (either from |params| or a default), |false| otherwise and
+ // |error| will be set.
+ bool GetParamValue(
const std::map<std::string, std::string>& params,
const std::string& param_name,
- std::string* param_value);
+ std::string* param_value,
+ chromeos::ErrorPtr* error);
// Builds Cloud API devices collection REST resouce which matches
// current state of the device including command definitions
