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>

buffet/device_registration_info.cc

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

buffet/device_registration_info.h

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