Implementation of ONC signature, validator and normalizer.

chrome/browser/chromeos/cros/onc_network_parser.cc

Index: chrome/browser/chromeos/cros/onc_network_parser.cc
diff --git a/chrome/browser/chromeos/cros/onc_network_parser.cc b/chrome/browser/chromeos/cros/onc_network_parser.cc
index 213b71623b52058fc5ff077db952e8ee0ea3e329..f54e7771b2482c76e80f0acf746012ccc7c63c04 100644
--- a/chrome/browser/chromeos/cros/onc_network_parser.cc
+++ b/chrome/browser/chromeos/cros/onc_network_parser.cc
@@ -19,6 +19,8 @@
 #include "chrome/browser/chromeos/cros/native_network_parser.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/cros/onc_constants.h"
+#include "chrome/browser/chromeos/network_settings/onc_signature.h"
+#include "chrome/browser/chromeos/network_settings/onc_validator.h"
 #include "chrome/browser/chromeos/proxy_config_service_impl.h"
 #include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/common/net/x509_certificate_model.h"
@@ -133,7 +135,7 @@ OncValueSignature vpn_signature[] = {
   { onc::vpn::kIPsec, PROPERTY_INDEX_ONC_IPSEC, TYPE_DICTIONARY },
   { onc::vpn::kL2TP, PROPERTY_INDEX_ONC_L2TP, TYPE_DICTIONARY },
   { onc::vpn::kOpenVPN, PROPERTY_INDEX_ONC_OPENVPN, TYPE_DICTIONARY },
-  { onc::vpn::kType, PROPERTY_INDEX_PROVIDER_TYPE, TYPE_STRING },

[Mattias Nissler (ping if slow), 2012/11/02 10:10:00]

Shouldn't this stay onc::vpn::kType? It's the type field for vpns after all.

[pneubeck (no reviews), 2012/11/05 12:04:48]

Done.

+  { onc::kType, PROPERTY_INDEX_PROVIDER_TYPE, TYPE_STRING },
   { NULL }
 };
 
@@ -203,7 +205,7 @@ OncValueSignature openvpn_signature[] = {
 };
 
 OncValueSignature proxy_settings_signature[] = {
-  { onc::proxy::kType, PROPERTY_INDEX_ONC_PROXY_TYPE, TYPE_STRING },

[Mattias Nissler (ping if slow), 2012/11/02 10:10:00]

ditto

[pneubeck (no reviews), 2012/11/05 12:04:48]

Done.

+  { onc::kType, PROPERTY_INDEX_ONC_PROXY_TYPE, TYPE_STRING },
   { onc::proxy::kPAC, PROPERTY_INDEX_ONC_PROXY_PAC, TYPE_STRING },
   { onc::proxy::kManual, PROPERTY_INDEX_ONC_PROXY_MANUAL, TYPE_DICTIONARY },
   { onc::proxy::kExcludeDomains, PROPERTY_INDEX_ONC_PROXY_EXCLUDE_DOMAINS,
@@ -304,6 +306,27 @@ OncNetworkParser::OncNetworkParser(const std::string& onc_blob,
     if (!root_dict_.get())
       return;
 
+    bool is_managed = onc_source == NetworkUIData::ONC_SOURCE_USER_POLICY ||
+        onc_source == NetworkUIData::ONC_SOURCE_DEVICE_POLICY;
+    // Validate the ONC dictionary. We are liberal and ignore unknown field
+    // names.

[Mattias Nissler (ping if slow), 2012/11/02 10:10:00]

I think this comment should go before the is_managed definition in line 309.

[pneubeck (no reviews), 2012/11/05 12:04:48]

Done.

+    bool error_on_unknown_field = false;
+    bool error_on_wrong_recommended = false;

[Mattias Nissler (ping if slow), 2012/11/02 10:10:00]

I don't understand what that means. Better name? Comment?

[pneubeck (no reviews), 2012/11/05 12:04:48]

Done.

+    bool error_on_missing_field = true;
+    scoped_ptr<onc::Validator> validator(
+        new onc::Validator(error_on_unknown_field, error_on_wrong_recommended,
+                           error_on_missing_field, is_managed));
+
+    // Unknown fields are removed from the result.
+    root_dict_ = validator->ValidateAndRepairObject(
+        &onc::kUnencryptedConfigurationSignature,
+        *root_dict_);
+
+    if (!root_dict_.get()) {
+      LOG(WARNING) << "Provided ONC is invalid and couldn't be repaired";
+      return;
+    }
+
     // At least one of NetworkConfigurations or Certificates is required.
     bool has_network_configurations =
         root_dict_->GetList("NetworkConfigurations", &network_configs_);
@@ -1074,7 +1097,7 @@ scoped_refptr<net::X509Certificate> OncNetworkParser::ParseClientCertificate(
 ClientCertType OncNetworkParser::ParseClientCertType(
     const std::string& type) {
   static EnumMapper<ClientCertType>::Pair table[] = {
-    { onc::certificate::kNone, CLIENT_CERT_TYPE_NONE },
+    { onc::kNone, CLIENT_CERT_TYPE_NONE },

[Mattias Nissler (ping if slow), 2012/11/02 10:10:00]

same here. I'd prefer clarity of where to find constants over sharing.

[pneubeck (no reviews), 2012/11/05 12:04:48]

Done.

     { onc::certificate::kRef, CLIENT_CERT_TYPE_REF },
     { onc::certificate::kPattern, CLIENT_CERT_TYPE_PATTERN },
   };
@@ -1164,7 +1187,7 @@ bool OncNetworkParser::ProcessProxySettings(OncNetworkParser* parser,
   const DictionaryValue* dict = NULL;
   CHECK(value.GetAsDictionary(&dict));
   std::string proxy_type_string;
-  if (!dict->GetString(onc::proxy::kType, &proxy_type_string)) {
+  if (!dict->GetString(onc::kType, &proxy_type_string)) {
     VLOG(1) << network->name() << ": ProxySettings.Type is missing";
     return false;
   }