ONC: add support for non-utf-8 SSIDs

chromeos/network/onc/onc_validator.cc

Index: chromeos/network/onc/onc_validator.cc
diff --git a/chromeos/network/onc/onc_validator.cc b/chromeos/network/onc/onc_validator.cc
index d84d2556cb174bbc54e22ddff9c47c69a0027a11..b441d7e2f1009323fb02cd343611a02f558fc75d 100644
--- a/chromeos/network/onc/onc_validator.cc
+++ b/chromeos/network/onc/onc_validator.cc
@@ -20,6 +20,8 @@ namespace onc {
 
 namespace {
 
+const int kMaximumSSIDLengthInBytes = 32;

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: add a comment where this comes from (802.11 standard)

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+
 template <typename T, size_t N>
 std::vector<T> toVector(T const (&array)[N]) {
   return std::vector<T>(array, array + N);
@@ -396,6 +398,68 @@ bool Validator::FieldExistsAndIsEmpty(const base::DictionaryValue& object,
   return true;
 }
 
+bool Validator::IsSSIDOrHexSSIDValid(const base::DictionaryValue& object,
+                                     const std::string& ssid_field_name,

[pneubeck (no reviews), 2014/11/26 10:15:34]

as you always pass the same strings for the field names, you could inline kSSID
and kHexSSID

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+                                     const std::string& hex_ssid_field_name) {
+  // SSID validity

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: use whole sentences (ending with fullstop).

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+  std::string ssid_string;
+  const base::Value* value = NULL;
+  if (object.GetWithoutPathExpansion(ssid_field_name, &value)) {
+    if (!value->GetAsString(&ssid_string)) {

[pneubeck (no reviews), 2014/11/26 10:15:34]

type is verified already by the generic/default verification _before_ the custom
verification code runs.

you can change line 407 to 'GetStringWithoutPathExpansion'

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+      LOG(ERROR) << MessageHeader() << "Field " << ssid_field_name
+                 << " does not contain a string.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+    if (ssid_string.size() <= 0 ||
+        ssid_string.size() > kMaximumSSIDLengthInBytes) {
+      LOG(ERROR) << MessageHeader() << ssid_field_name
+                 << " has an invalid length.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+
+  // HexSSID validity
+  std::string hex_ssid_string;
+  if (object.GetWithoutPathExpansion(hex_ssid_field_name, &value)) {

[pneubeck (no reviews), 2014/11/26 10:15:34]

use GetStringWithoutPathExpansion

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+    if (!value->GetAsString(&hex_ssid_string)) {

[pneubeck (no reviews), 2014/11/26 10:15:34]

again not required

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+      LOG(ERROR) << MessageHeader() << "Field " << hex_ssid_field_name
+                 << " does not contain a string.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+    std::vector<uint8> bytes;
+    if (!base::HexStringToBytes(hex_ssid_string, &bytes)) {
+      LOG(ERROR) << MessageHeader() << "Field " << hex_ssid_field_name
+                 << " is not a valid hex representation: \"" << hex_ssid_string
+                 << "\"";
+      error_or_warning_found_ = true;
+      return false;
+    }
+    if (bytes.size() <= 0 || bytes.size() > kMaximumSSIDLengthInBytes) {
+      LOG(ERROR) << MessageHeader() << hex_ssid_field_name
+                 << " has an invalid length.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+
+  if (ssid_string.length() > 0 && hex_ssid_string.length() > 0) {
+    // both set

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: make the comment a bit more verbose:

"If both SSID and HexSSID are set, verify that they are consistent."

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+    std::string hexified =
+        base::HexEncode(ssid_string.c_str(), ssid_string.size());
+    if (hexified != hex_ssid_string) {
+      LOG(ERROR) << MessageHeader() << "Fields " << ssid_field_name << " and "
+                 << hex_ssid_field_name << " contain inconsistent values.";
+      error_or_warning_found_ = true;
+      return false;
+    }
+  }
+
+  return true;
+}
+
 bool Validator::RequireField(const base::DictionaryValue& dict,
                              const std::string& field_name) {
   if (dict.HasKey(field_name))
@@ -572,8 +636,15 @@ bool Validator::ValidateWiFi(base::DictionaryValue* result) {
   if (FieldExistsAndHasNoValidValue(*result, kSecurity, valid_securities))
     return false;
 
-  bool all_required_exist =
-      RequireField(*result, kSecurity) && RequireField(*result, kSSID);
+  // validate SSID and HexSSID fields

[pneubeck (no reviews), 2014/11/26 10:15:34]

nit: start with capital case and end with fullstop

append a ', if present.' or 'if existent'

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+  if (!IsSSIDOrHexSSIDValid(*result, kSSID, kHexSSID))
+    return false;
+
+  bool all_required_exist = RequireField(*result, kSecurity);
+  if (!result->HasKey(kSSID))

[pneubeck (no reviews), 2014/11/26 10:15:34]

prepend a comment that says that "One of {kSSID, kHexSSID} must be present".

[cschuet (SLOW), 2014/11/27 11:03:58]

Done.

+    all_required_exist &= RequireField(*result, kHexSSID);
+  if (!result->HasKey(kHexSSID))
+    all_required_exist &= RequireField(*result, kSSID);
 
   std::string security;
   result->GetStringWithoutPathExpansion(kSecurity, &security);