Don't copy empty GUID in CopyIdentifyingProperties.

chromeos/network/shill_property_util.cc

Index: chromeos/network/shill_property_util.cc
diff --git a/chromeos/network/shill_property_util.cc b/chromeos/network/shill_property_util.cc
index 6f03eea8e48cea6a87e082a2179a65e7a67e86d4..b14cc2d63d272f674e38c19e0369a926d0d1af5a 100644
--- a/chromeos/network/shill_property_util.cc
+++ b/chromeos/network/shill_property_util.cc
@@ -43,12 +43,17 @@ std::string ValidateUTF8(const std::string& str) {
   return result;
 }
 
-void CopyStringFromDictionary(const base::DictionaryValue& source,
+// If existent and non-empty, copies the string at |key| from |source| to
+// |dest|. Returns true if the string was copied.
+bool CopyStringFromDictionary(const base::DictionaryValue& source,
                               const std::string& key,
                               base::DictionaryValue* dest) {
   std::string string_value;
-  if (source.GetStringWithoutPathExpansion(key, &string_value))
-    dest->SetStringWithoutPathExpansion(key, string_value);
+  if (!source.GetStringWithoutPathExpansion(key, &string_value) ||
+      string_value.empty())
+    return false;
+  dest->SetStringWithoutPathExpansion(key, string_value);
+  return true;
 }
 
 }  // namespace
@@ -175,22 +180,29 @@ void SetUIData(const NetworkUIData& ui_data,
 
 bool CopyIdentifyingProperties(const base::DictionaryValue& service_properties,
                                base::DictionaryValue* dest) {
+  bool success = true;
+
+  // GUID is optional.
   CopyStringFromDictionary(service_properties, flimflam::kGuidProperty, dest);
 
   std::string type;
   service_properties.GetStringWithoutPathExpansion(flimflam::kTypeProperty,
                                                    &type);
+  success &= !type.empty();
   dest->SetStringWithoutPathExpansion(flimflam::kTypeProperty, type);
   if (type == flimflam::kTypeWifi) {
-    CopyStringFromDictionary(
+    success &= CopyStringFromDictionary(
         service_properties, flimflam::kSecurityProperty, dest);
-    CopyStringFromDictionary(service_properties, flimflam::kSSIDProperty, dest);
-    CopyStringFromDictionary(service_properties, flimflam::kModeProperty, dest);
+    success &= CopyStringFromDictionary(
+        service_properties, flimflam::kSSIDProperty, dest);
+    success &= CopyStringFromDictionary(
+        service_properties, flimflam::kModeProperty, dest);
   } else if (type == flimflam::kTypeVPN) {
-    CopyStringFromDictionary(service_properties, flimflam::kNameProperty, dest);
+    success &= CopyStringFromDictionary(
+        service_properties, flimflam::kNameProperty, dest);
     // VPN Provider values are read from the "Provider" dictionary, but written
     // with the keys "Provider.Type" and "Provider.Host".
-    const base::DictionaryValue* provider_properties;
+    const base::DictionaryValue* provider_properties = NULL;
     if (!service_properties.GetDictionaryWithoutPathExpansion(
              flimflam::kProviderProperty, &provider_properties)) {
       LOG(ERROR) << "Incomplete Shill dictionary missing VPN provider dict.";
@@ -199,12 +211,14 @@ bool CopyIdentifyingProperties(const base::DictionaryValue& service_properties,
     std::string vpn_provider_type;
     provider_properties->GetStringWithoutPathExpansion(flimflam::kTypeProperty,
                                                        &vpn_provider_type);
+    success &= !vpn_provider_type.empty();
     dest->SetStringWithoutPathExpansion(flimflam::kProviderTypeProperty,
                                         vpn_provider_type);
 
     std::string vpn_provider_host;
     provider_properties->GetStringWithoutPathExpansion(flimflam::kHostProperty,
                                                        &vpn_provider_host);
+    success &= !vpn_provider_host.empty();
     dest->SetStringWithoutPathExpansion(flimflam::kProviderHostProperty,
                                         vpn_provider_host);
   } else if (type == flimflam::kTypeEthernet ||
@@ -213,8 +227,11 @@ bool CopyIdentifyingProperties(const base::DictionaryValue& service_properties,
     // properties.
   } else {
     NOTREACHED() << "Unsupported network type " << type;
+    success = false;
   }
-  return true;
+  if (!success)
+    LOG(ERROR) << "Missing required identifying properties.";

[stevenjb, 2013/09/05 17:00:41]

Seems worth using NET_LOG_ERROR here?

[pneubeck (no reviews), 2013/09/06 12:53:33]

Done.

+  return success;
 }
 
 }  // namespace shill_property_util