Show parse errors in the UI when loading ONC files.

chrome/browser/chromeos/cros/onc_network_parser.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/cros/onc_network_parser.h"
 
 #include "base/base64.h"
 #include "base/json/json_value_serializer.h"
 #include "base/json/json_writer.h"  // for debug output only.
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/cros/native_network_constants.h"
 #include "chrome/browser/chromeos/cros/native_network_parser.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
+#include "grit/generated_resources.h"
 #include "net/base/cert_database.h"
 #include "net/base/crypto_module.h"
 #include "net/base/net_errors.h"
 #include "net/base/x509_certificate.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace chromeos {
 
 // Local constants.
 namespace {
 
 const base::Value::Type TYPE_BOOLEAN = base::Value::TYPE_BOOLEAN;
 const base::Value::Type TYPE_DICTIONARY = base::Value::TYPE_DICTIONARY;
 const base::Value::Type TYPE_INTEGER = base::Value::TYPE_INTEGER;
 const base::Value::Type TYPE_LIST = base::Value::TYPE_LIST;
@@ skipping 194 matching lines @@
   return network_configs_ ? network_configs_->GetSize() : 0;
 }
 
 int OncNetworkParser::GetCertificatesSize() const {
   return certificates_ ? certificates_->GetSize() : 0;
 }
 
 bool OncNetworkParser::ParseCertificate(int cert_index) {
   CHECK(certificates_);
   CHECK(static_cast<size_t>(cert_index) < certificates_->GetSize());
-  CHECK(cert_index >= 0);
+  CHECK_GE(cert_index, 0);
   base::DictionaryValue* certificate = NULL;
-  certificates_->GetDictionary(cert_index, &certificate);
-  CHECK(certificate);
+  if (!certificates_->GetDictionary(cert_index, &certificate)) {
+    parse_error_ = l10n_util::GetStringUTF8(

[kmixter1, 2011/12/10 15:13:09]

Currently almost all of the unit tests correctly parse, but we should adds ones
that purposefully don't and test parse_error_.

+        IDS_NETWORK_CONFIG_ERROR_CERT_DATA_MALFORMED);
+    return NULL;
+  }
 
   if (VLOG_IS_ON(2)) {
     std::string certificate_json;
     base::JSONWriter::Write(static_cast<base::Value*>(certificate),
                             true, &certificate_json);
     VLOG(2) << "Parsing certificate at index " << cert_index
             << ": " << certificate_json;
   }
 
   // Get out the attributes of the given cert.
   std::string guid;
   bool remove = false;
   if (!certificate->GetString("GUID", &guid) || guid.empty()) {
     LOG(WARNING) << "ONC File: certificate missing identifier at index"
                  << cert_index;
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_GUID_MISSING);
     return false;
   }
 
   if (!certificate->GetBoolean("Remove", &remove))
     remove = false;
 
   net::CertDatabase cert_database;
-  if (remove)
-    return cert_database.DeleteCertAndKeyByLabel(guid);
+  if (remove) {
+    if (!cert_database.DeleteCertAndKeyByLabel(guid)) {
+      parse_error_ = l10n_util::GetStringUTF8(
+          IDS_NETWORK_CONFIG_ERROR_CERT_DELETE);
+      return false;
+    }
+    return true;
+  }
 
   // Not removing, so let's get the data we need to add this cert.
   std::string cert_type;
   certificate->GetString("Type", &cert_type);
   if (cert_type == "Server" || cert_type == "Authority") {
     return ParseServerOrCaCertificate(cert_index, cert_type, certificate);
   }
   if (cert_type == "Client") {
     return ParseClientCertificate(cert_index, certificate);
   }
 
   LOG(WARNING) << "ONC File: certificate of unknown type: " << cert_type
                << " at index " << cert_index;
+  parse_error_ = l10n_util::GetStringUTF8(
+      IDS_NETWORK_CONFIG_ERROR_CERT_TYPE_MISSING);
   return false;
 }
 
 Network* OncNetworkParser::ParseNetwork(int n) {
-  if (!network_configs_)
+  CHECK(network_configs_);
+  CHECK(static_cast<size_t>(n) < network_configs_->GetSize());
+  CHECK_GE(n, 0);
+  DictionaryValue* info = NULL;
+  if (!network_configs_->GetDictionary(n, &info)) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_NETWORK_PROP_DICT_MALFORMED);

[kmixter1, 2011/12/10 15:13:09]

Please use JSON and ONC terminology in error messages (dictionary should be
object, network should be NetworkConfiguration).

[Charlie Lee, 2011/12/14 17:02:20]

I can't fix this for R17. Will remember to fix it for R18.

     return NULL;
-  DictionaryValue* info = NULL;
-  if (!network_configs_->GetDictionary(n, &info))
-    return NULL;
+  }
+
   if (VLOG_IS_ON(2)) {
     std::string network_json;
     base::JSONWriter::Write(static_cast<base::Value*>(info),
                             true, &network_json);
     VLOG(2) << "Parsing network at index " << n
             << ": " << network_json;
   }
 
   return CreateNetworkFromInfo(std::string(), *info);
 }
 
 Network* OncNetworkParser::CreateNetworkFromInfo(
     const std::string& service_path,
     const DictionaryValue& info) {
   ConnectionType type = ParseTypeFromDictionary(info);
-  if (type == TYPE_UNKNOWN)  // Return NULL if cannot parse network type.
+  if (type == TYPE_UNKNOWN) {  // Return NULL if cannot parse network type.
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_NETWORK_TYPE_MISSING);
     return NULL;
+  }
   scoped_ptr<Network> network(CreateNewNetwork(type, service_path));
   if (!ParseNestedObject(network.get(),
                          "NetworkConfiguration",
                          static_cast<const base::Value&>(info),
                          network_configuration_signature,
                          ParseNetworkConfigurationValue)) {
     LOG(WARNING) << "Network " << network->name() << " failed to parse.";
+    if (parse_error_.empty())
+      parse_error_ = l10n_util::GetStringUTF8(
+          IDS_NETWORK_CONFIG_ERROR_NETWORK_PROP_DICT_MALFORMED);
     return NULL;
   }
   if (VLOG_IS_ON(2)) {
     VLOG(2) << "Created Network '" << network->name()
             << "' from info. Path:" << service_path
             << " Type:" << ConnectionTypeToString(type);
   }
   return network.release();
 }
 
@@ skipping 38 matching lines @@
     base::DictionaryValue* certificate) {
   net::CertDatabase cert_database;
   bool web_trust = false;
   base::ListValue* trust_list = NULL;
   if (certificate->GetList("Trust", &trust_list)) {
     for (size_t i = 0; i < trust_list->GetSize(); ++i) {
       std::string trust_type;
       if (!trust_list->GetString(i, &trust_type)) {
         LOG(WARNING) << "ONC File: certificate trust is invalid at index "
                      << cert_index;
+        parse_error_ = l10n_util::GetStringUTF8(
+            IDS_NETWORK_CONFIG_ERROR_CERT_TRUST_INVALID);
         return false;
       }
       if (trust_type == "Web") {
         web_trust = true;
       } else {
         LOG(WARNING) << "ONC File: certificate contains unknown "
                      << "trust type: " << trust_type
                      << " at index " << cert_index;
+        parse_error_ = l10n_util::GetStringUTF8(
+            IDS_NETWORK_CONFIG_ERROR_CERT_TRUST_UNKNOWN);
         return false;
       }
     }
   }
 
   std::string x509_data;
   if (!certificate->GetString("X509", &x509_data) || x509_data.empty()) {
     LOG(WARNING) << "ONC File: certificate missing appropriate "
                  << "certificate data for type: " << cert_type
                  << " at index " << cert_index;
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_DATA_MISSING);
     return false;
   }
 
   std::string decoded_x509;
   if (!base::Base64Decode(x509_data, &decoded_x509)) {
     LOG(WARNING) << "Unable to base64 decode X509 data: \""
                  << x509_data << "\".";
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_DATA_MALFORMED);
     return false;
   }
 
   scoped_refptr<net::X509Certificate> x509_cert(
       net::X509Certificate::CreateFromBytes(decoded_x509.c_str(),
                                             decoded_x509.size()));
   if (!x509_cert.get()) {
     LOG(WARNING) << "Unable to create X509 certificate from bytes.";
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_DATA_MALFORMED);
     return false;
   }
   net::CertificateList cert_list;
   cert_list.push_back(x509_cert);
   net::CertDatabase::ImportCertFailureList failures;
   bool success = false;
   if (cert_type == "Server") {
     success = cert_database.ImportServerCert(cert_list, &failures);
-  } else { // Authority cert
+  } else {  // Authority cert
     net::CertDatabase::TrustBits trust = web_trust ?
                                          net::CertDatabase::TRUSTED_SSL :
                                          net::CertDatabase::UNTRUSTED;
     success = cert_database.ImportCACerts(cert_list, trust, &failures);
   }
   if (!failures.empty()) {
     LOG(WARNING) << "ONC File: Error ("
                  << net::ErrorToString(failures[0].net_error)
                  << ") importing " << cert_type << " certificate at index "
                  << cert_index;
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_IMPORT);
     return false;
   }
   if (!success) {
     LOG(WARNING) << "ONC File: Unknown error importing " << cert_type
                  << " certificate at index " << cert_index;
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_UNKNOWN);
     return false;
   }
   VLOG(2) << "Successfully imported server/ca certificate at index "
           << cert_index;
   return true;
 }
 
 bool OncNetworkParser::ParseClientCertificate(
     int cert_index,
     base::DictionaryValue* certificate) {
   net::CertDatabase cert_database;
   std::string pkcs12_data;
   if (!certificate->GetString("PKCS12", &pkcs12_data) ||
       pkcs12_data.empty()) {
     LOG(WARNING) << "ONC File: PKCS12 data is missing for Client "
                  << "certificate at index " << cert_index;
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_DATA_MISSING);
     return false;
   }
 
   std::string decoded_pkcs12;
   if (!base::Base64Decode(pkcs12_data, &decoded_pkcs12)) {
     LOG(WARNING) << "Unable to base64 decode PKCS#12 data: \""
                  << pkcs12_data << "\".";
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_DATA_MALFORMED);
     return false;
   }
 
   // Since this has a private key, always use the private module.
   scoped_refptr<net::CryptoModule> module(cert_database.GetPrivateModule());
   int result = cert_database.ImportFromPKCS12(
       module.get(), decoded_pkcs12, string16(), false);
   if (result != net::OK) {
     LOG(WARNING) << "ONC File: Unable to import Client certificate at index "
                  << cert_index
                  << " (error " << net::ErrorToString(result) << ").";
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_CERT_IMPORT);
     return false;
   }
   VLOG(2) << "Successfully imported client certificate at index "
           << cert_index;
   return true;
 }
 
 bool OncNetworkParser::ParseNestedObject(Network* network,
                                          const std::string& onc_type,
                                          const base::Value& value,
                                          OncValueSignature* signature,
                                          ParserPointer parser) {
   bool any_errors = false;
   if (!value.IsType(base::Value::TYPE_DICTIONARY)) {
     VLOG(1) << network->name() << ": expected object of type " << onc_type;
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_NETWORK_PROP_DICT_MALFORMED);
     return false;
   }
   VLOG(2) << "Parsing nested object of type " << onc_type;
   const DictionaryValue* dict = NULL;
   value.GetAsDictionary(&dict);
   for (DictionaryValue::key_iterator iter = dict->begin_keys();
        iter != dict->end_keys(); ++iter) {
     const std::string& key = *iter;
     base::Value* inner_value = NULL;
     dict->GetWithoutPathExpansion(key, &inner_value);
@@ skipping 25 matching lines @@
       any_errors = true;
       continue;
     }
     if (VLOG_IS_ON(2)) {
       std::string value_json;
       base::JSONWriter::Write(inner_value, true, &value_json);
       VLOG(2) << network->name() << ": Successfully parsed [" << key
               << "(" << index << ")] = " << value_json;
     }
   }
+  if (any_errors)
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_NETWORK_PROP_DICT_MALFORMED);
   return !any_errors;
 }
 
 // static
 bool OncNetworkParser::CheckNetworkType(Network* network,
                                         ConnectionType expected,
                                         const std::string& onc_type) {
   if (expected != network->type()) {
     LOG(WARNING) << network->name() << ": "
                  << onc_type << " field unexpected for this type network";
@@ skipping 518 matching lines @@
     // on the value of AuthenticationType.
     { "L2TP-IPsec", PROVIDER_TYPE_L2TP_IPSEC_PSK },
     { "OpenVPN", PROVIDER_TYPE_OPEN_VPN },
   };
   CR_DEFINE_STATIC_LOCAL(EnumMapper<ProviderType>, parser,
       (table, arraysize(table), PROVIDER_TYPE_MAX));
   return parser.Get(type);
 }
 
 }  // namespace chromeos