Use a schema to decode 3rd party policy on windows, when present.

chrome/browser/policy/policy_loader_win_unittest.cc

Index: chrome/browser/policy/policy_loader_win_unittest.cc
diff --git a/chrome/browser/policy/policy_loader_win_unittest.cc b/chrome/browser/policy/policy_loader_win_unittest.cc
index 888fa8a05580460948ba309f9bd1d02bbccd2077..4c4a19f0f4c918a81ddf5a03e406607f419dee88 100644
--- a/chrome/browser/policy/policy_loader_win_unittest.cc
+++ b/chrome/browser/policy/policy_loader_win_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/json/json_writer.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/registry.h"
 #include "chrome/browser/policy/async_policy_provider.h"
@@ -30,54 +31,151 @@ const wchar_t kUnitTestMachineOverrideSubKey[] =
 const wchar_t kUnitTestUserOverrideSubKey[] =
     L"SOFTWARE\\Chromium Unit Tests\\HKCU Override";
 
-// Installs |dict| at the given |path|, in the given |hive|. Currently only
-// string, int and dictionary types are converted; other types cause a failure.
-// Returns false if there was any failure, and true if |dict| was successfully
-// written.
-// TODO(joaodasilva): generate a schema for |dict| too, so that all types can
-// be retrieved.
-bool InstallDictionary(const base::DictionaryValue& dict,
-                       HKEY hive,
-                       const string16& path) {
+const wchar_t kPathSep[] = L"\\";
+
+// Registry key where 3rd party policy is stored.
+const wchar_t kThirdParty[] = L"3rdparty";
+
+// Keys for mandatory and recommended policy and schema under the registry key
+// for a component.
+const wchar_t kMandatory[] = L"policy";
+const wchar_t kRecommended[] = L"recommended";
+const wchar_t kSchema[] = L"schema";
+
+// Installs |value| in the given registry |path| and |hive|, under the key
+// |name|. Returns false on errors.
+// Some of the possible Value types are stored after a conversion (e.g. doubles
+// are stored as strings), and can only be retrieved if a corresponding schema
+// is written.
+bool InstallValue(const base::Value& value,
+                  HKEY hive,
+                  const string16& path,
+                  const string16& name) {
   // KEY_ALL_ACCESS causes the ctor to create the key if it does not exist yet.
   RegKey key(hive, path.c_str(), KEY_ALL_ACCESS);
-  const string16 kPathSep = ASCIIToUTF16("\\");
-
-  for (base::DictionaryValue::Iterator it(dict); it.HasNext(); it.Advance()) {
-    string16 name(UTF8ToUTF16(it.key()));
-    switch (it.value().GetType()) {
-      case base::Value::TYPE_STRING: {
-        string16 value;
-        if (!it.value().GetAsString(&value))
-          return false;
-        if (key.WriteValue(name.c_str(), value.c_str()) != ERROR_SUCCESS)
-          return false;
-        break;
-      }
+  switch (value.GetType()) {
+    case base::Value::TYPE_NULL:
+      return key.WriteValue(name.c_str(), L"") == ERROR_SUCCESS;
 
-      case base::Value::TYPE_INTEGER: {
-        int value;
-        if (!it.value().GetAsInteger(&value))
-          return false;
-        if (key.WriteValue(name.c_str(), value) != ERROR_SUCCESS)
+    case base::Value::TYPE_BOOLEAN: {
+      bool bool_value;
+      if (!value.GetAsBoolean(&bool_value))
+        return false;
+      return key.WriteValue(name.c_str(), bool_value ? 1 : 0) == ERROR_SUCCESS;
+    }
+
+    case base::Value::TYPE_INTEGER: {
+      int int_value;
+      if (!value.GetAsInteger(&int_value))
+        return false;
+      return key.WriteValue(name.c_str(), int_value) == ERROR_SUCCESS;
+    }
+
+    case base::Value::TYPE_DOUBLE: {
+      double double_value;
+      if (!value.GetAsDouble(&double_value))
+        return false;
+      string16 str_value = UTF8ToUTF16(base::DoubleToString(double_value));
+      return key.WriteValue(name.c_str(), str_value.c_str()) == ERROR_SUCCESS;
+    }
+
+    case base::Value::TYPE_STRING: {
+      string16 str_value;
+      if (!value.GetAsString(&str_value))
+        return false;
+      return key.WriteValue(name.c_str(), str_value.c_str()) == ERROR_SUCCESS;
+    }
+
+    case base::Value::TYPE_DICTIONARY: {
+      const base::DictionaryValue* sub_dict = NULL;
+      if (!value.GetAsDictionary(&sub_dict))
+        return false;
+      for (base::DictionaryValue::Iterator it(*sub_dict);
+           it.HasNext(); it.Advance()) {
+        if (!InstallValue(it.value(), hive, path + kPathSep + name,
+                          UTF8ToUTF16(it.key()))) {
           return false;
-        break;
+        }
       }
+      return true;
+    }
 
-      case base::Value::TYPE_DICTIONARY: {
-        const base::DictionaryValue* sub_dict = NULL;
-        if (!it.value().GetAsDictionary(&sub_dict))
+    case base::Value::TYPE_LIST: {
+      const base::ListValue* list = NULL;
+      if (!value.GetAsList(&list))
+        return false;
+      for (size_t i = 0; i < list->GetSize(); ++i) {
+        base::Value* item;
+        if (!list->Get(i, &item))
           return false;
-        if (!InstallDictionary(*sub_dict, hive, path + kPathSep + name))
+        if (!InstallValue(*item, hive, path + kPathSep + name,
+                          base::UintToString16(i + 1))) {
           return false;
-        break;
+        }
       }
+      return true;
+    }
 
-      default:
-        return false;
+    case base::Value::TYPE_BINARY:
+      return false;
+  }
+  NOTREACHED();
+  return false;
+}
+
+std::string BuildSchema(const base::Value& value) {
+  switch (value.GetType()) {
+    case base::Value::TYPE_NULL:
+      return "{ \"type\": \"null\" }";

[Mattias Nissler (ping if slow), 2012/06/27 12:07:17]

It might be more readable if you were to build this as a base::Value structure
and then serialize it. Not sure though.

[Joao da Silva, 2012/06/27 14:31:15]

Good idea, done.

+    case base::Value::TYPE_BOOLEAN:
+      return "{ \"type\": \"boolean\" }";
+    case base::Value::TYPE_INTEGER:
+      return "{ \"type\": \"integer\" }";
+    case base::Value::TYPE_DOUBLE:
+      return "{ \"type\": \"number\" }";
+    case base::Value::TYPE_STRING:
+      return "{ \"type\": \"string\" }";
+
+    case base::Value::TYPE_LIST: {
+      // Assumes every list element has the same type.
+      const base::ListValue* list = NULL;
+      if (!value.GetAsList(&list))
+        return EmptyString();
+      std::string item_type = "{}";
+      if (!list->empty())
+        item_type = BuildSchema(**list->begin());
+      return "{ \"type\": \"array\", \"items\": " + item_type + " }";
+    }
+
+    case base::Value::TYPE_DICTIONARY: {
+      const base::DictionaryValue* dict = NULL;
+      if (!value.GetAsDictionary(&dict))
+        return EmptyString();
+      std::string properties;
+      for (base::DictionaryValue::Iterator it(*dict);
+           it.HasNext(); it.Advance()) {
+        if (!properties.empty())
+          properties += ", ";
+        properties += "\"" + it.key() + "\": " + BuildSchema(it.value());
+      }
+      return "{ \"type\": \"object\", \"properties\": { " + properties + " } }";
     }
+
+    case base::Value::TYPE_BINARY:
+      return EmptyString();
   }
-  return true;
+  NOTREACHED();
+  return EmptyString();
+}
+
+bool InstallSchema(const base::Value& value,
+                   HKEY hive,
+                   const string16& path,
+                   const string16& name) {
+  string16 schema = UTF8ToUTF16(BuildSchema(value));
+  // KEY_ALL_ACCESS causes the ctor to create the key if it does not exist yet.
+  RegKey key(hive, path.c_str(), KEY_ALL_ACCESS);
+  return key.WriteValue(name.c_str(), schema.c_str()) == ERROR_SUCCESS;
 }
 
 // This class provides sandboxing and mocking for the parts of the Windows
@@ -130,6 +228,8 @@ class TestHarness : public PolicyProviderTestHarness {
   virtual void InstallDictionaryPolicy(
       const std::string& policy_name,
       const base::DictionaryValue* policy_value) OVERRIDE;
+  virtual void Install3rdPartyPolicy(
+      const base::DictionaryValue* policies) OVERRIDE;
 
   // Creates a harness instance that will install policy in HKCU or HKLM,
   // respectively.
@@ -247,6 +347,30 @@ void TestHarness::InstallDictionaryPolicy(
                  UTF8ToUTF16(json).c_str());
 }
 
+void TestHarness::Install3rdPartyPolicy(const base::DictionaryValue* policies) {
+  // The first level entries are domains, and the second level entries map
+  // components to their policy.
+  const string16 kPathPrefix = string16(kRegistryMandatorySubKey) + kPathSep +
+                               kThirdParty + kPathSep;
+  for (base::DictionaryValue::Iterator domain(*policies);
+       domain.HasNext(); domain.Advance()) {
+    const base::DictionaryValue* components = NULL;
+    if (!domain.value().GetAsDictionary(&components)) {
+      ADD_FAILURE();
+      continue;
+    }
+    for (base::DictionaryValue::Iterator component(*components);
+         component.HasNext(); component.Advance()) {
+      const string16 path = string16(kRegistryMandatorySubKey) + kPathSep +
+                            kThirdParty + kPathSep +
+                            UTF8ToUTF16(domain.key()) + kPathSep +
+                            UTF8ToUTF16(component.key());
+      InstallValue(component.value(), hive_, path, kMandatory);
+      EXPECT_TRUE(InstallSchema(component.value(), hive_, path, kSchema));
+    }
+  }
+}
+
 // static
 PolicyProviderTestHarness* TestHarness::CreateHKCU() {
   return new TestHarness(HKEY_CURRENT_USER, POLICY_SCOPE_USER);
@@ -265,6 +389,12 @@ INSTANTIATE_TEST_CASE_P(
     ConfigurationPolicyProviderTest,
     testing::Values(TestHarness::CreateHKCU, TestHarness::CreateHKLM));
 
+// Instantiate abstract test case for 3rd party policy reading tests.
+INSTANTIATE_TEST_CASE_P(
+    ThirdPartyPolicyProviderWinTest,
+    Configuration3rdPartyPolicyProviderTest,
+    testing::Values(TestHarness::CreateHKCU, TestHarness::CreateHKLM));
+
 // Test cases for windows policy provider specific functionality.
 class PolicyLoaderWinTest : public PolicyTestBase {
  protected:
@@ -294,9 +424,7 @@ TEST_F(PolicyLoaderWinTest, HKLMOverHKCU) {
   EXPECT_TRUE(bundle->Equals(expected_bundle));
 }
 
-// TODO(joaodasilva): share tests for 3rd party policy with
-// ConfigDirPolicyProvider once PolicyLoaderWin is able to load all types.
-TEST_F(PolicyLoaderWinTest, Load3rdParty) {
+TEST_F(PolicyLoaderWinTest, Load3rdPartyWithoutSchema) {
   base::DictionaryValue dict;
   dict.SetString("str", "string value");
   dict.SetInteger("int", 123);
@@ -305,12 +433,12 @@ TEST_F(PolicyLoaderWinTest, Load3rdParty) {
   dict.Set("subsubsubdict", dict.DeepCopy());
 
   base::DictionaryValue policy_dict;
-  policy_dict.Set("3rdparty.extensions.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.policy",
+  policy_dict.Set("extensions.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.policy",
                   dict.DeepCopy());
-  policy_dict.Set("3rdparty.extensions.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.policy",
+  policy_dict.Set("extensions.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.policy",
                   dict.DeepCopy());
-  EXPECT_TRUE(InstallDictionary(policy_dict, HKEY_LOCAL_MACHINE,
-                                kRegistryMandatorySubKey));
+  EXPECT_TRUE(InstallValue(policy_dict, HKEY_LOCAL_MACHINE,
+                           kRegistryMandatorySubKey, kThirdParty));
 
   PolicyBundle expected;
   expected.Get(POLICY_DOMAIN_EXTENSIONS, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
@@ -329,8 +457,6 @@ TEST_F(PolicyLoaderWinTest, Merge3rdPartyPolicies) {
 
   const string16 kPathSuffix =
       kRegistryMandatorySubKey + ASCIIToUTF16("\\3rdparty\\extensions\\merge");
-  const string16 kMandatoryPath = kPathSuffix + ASCIIToUTF16("\\policy");
-  const string16 kRecommendedPath = kPathSuffix + ASCIIToUTF16("\\recommended");
 
   const char kUserMandatory[] = "user-mandatory";
   const char kUserRecommended[] = "user-recommended";
@@ -339,19 +465,23 @@ TEST_F(PolicyLoaderWinTest, Merge3rdPartyPolicies) {
 
   base::DictionaryValue policy;
   policy.SetString("a", kMachineMandatory);
-  EXPECT_TRUE(InstallDictionary(policy, HKEY_LOCAL_MACHINE, kMandatoryPath));
+  EXPECT_TRUE(InstallValue(policy, HKEY_LOCAL_MACHINE,
+                           kPathSuffix, kMandatory));
   policy.SetString("a", kUserMandatory);
   policy.SetString("b", kUserMandatory);
-  EXPECT_TRUE(InstallDictionary(policy, HKEY_CURRENT_USER, kMandatoryPath));
+  EXPECT_TRUE(InstallValue(policy, HKEY_CURRENT_USER,
+                           kPathSuffix, kMandatory));
   policy.SetString("a", kMachineRecommended);
   policy.SetString("b", kMachineRecommended);
   policy.SetString("c", kMachineRecommended);
-  EXPECT_TRUE(InstallDictionary(policy, HKEY_LOCAL_MACHINE, kRecommendedPath));
+  EXPECT_TRUE(InstallValue(policy, HKEY_LOCAL_MACHINE,
+                           kPathSuffix, kRecommended));
   policy.SetString("a", kUserRecommended);
   policy.SetString("b", kUserRecommended);
   policy.SetString("c", kUserRecommended);
   policy.SetString("d", kUserRecommended);
-  EXPECT_TRUE(InstallDictionary(policy, HKEY_CURRENT_USER, kRecommendedPath));
+  EXPECT_TRUE(InstallValue(policy, HKEY_CURRENT_USER,
+                           kPathSuffix, kRecommended));
 
   PolicyBundle expected;
   PolicyMap& expected_policy = expected.Get(POLICY_DOMAIN_EXTENSIONS, "merge");