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

chrome/browser/policy/policy_loader_win.cc

Index: chrome/browser/policy/policy_loader_win.cc
diff --git a/chrome/browser/policy/policy_loader_win.cc b/chrome/browser/policy/policy_loader_win.cc
index df5754eda04dc3be261e6b4daacb9f2a13303d80..262de182c0bab4dbddb6e01522dc920e352b2f7a 100644
--- a/chrome/browser/policy/policy_loader_win.cc
+++ b/chrome/browser/policy/policy_loader_win.cc
@@ -34,11 +34,23 @@ namespace policy {
 
 namespace {
 
+// Path separator for registry keys.
+const wchar_t kPathSep[] = L"\\";
+
 // Suffix of kRegistryMandatorySubKey where 3rd party policies are stored.
 const char k3rdPartyPolicySubKey[] = "\\3rdparty\\";
 
-// Path separator for registry keys.
-const wchar_t kPathSep[] = L"\\";
+// Registry value that contains a component's schema.
+const wchar_t kSchema[] = L"schema";
+
+// Entry in a JSON schema that contains the expected type of an entry.
+const char kType[] = "type";
+
+// Entry in a JSON schema that describes the expected properties of an object.
+const char kProperties[] = "properties";
+
+// Entry in a JSON schema that describes the expected type of array elements.
+const char kItems[] = "items";
 
 // Map of registry hives to their corresponding policy scope, in decreasing
 // order of priority.
@@ -133,9 +145,9 @@ bool ReadRegistryInteger(RegKey* key,
 
 // Returns the Value for a Chrome string policy named |name|, or NULL if
 // it wasn't found. The caller owns the returned value.
-base::Value* ReadStringValue(const string16& name,
-                             PolicyLevel* level,
-                             PolicyScope* scope) {
+base::Value* ReadChromeStringValue(const string16& name,
+                                   PolicyLevel* level,
+                                   PolicyScope* scope) {
   RegKey key;
   if (!LoadHighestPriorityKey(string16(), name, &key, level, scope))
     return NULL;
@@ -147,9 +159,9 @@ base::Value* ReadStringValue(const string16& name,
 
 // Returns the Value for a Chrome string list policy named |name|,
 // or NULL if it wasn't found. The caller owns the returned value.
-base::Value* ReadStringListValue(const string16& name,
-                                 PolicyLevel* level,
-                                 PolicyScope* scope) {
+base::Value* ReadChromeStringListValue(const string16& name,
+                                       PolicyLevel* level,
+                                       PolicyScope* scope) {
   RegKey key;
   if (!LoadHighestPriorityKey(name, string16(), &key, level, scope))
     return NULL;
@@ -163,9 +175,9 @@ base::Value* ReadStringListValue(const string16& name,
 
 // Returns the Value for a Chrome boolean policy named |name|,
 // or NULL if it wasn't found. The caller owns the returned value.
-base::Value* ReadBooleanValue(const string16& name,
-                              PolicyLevel* level,
-                              PolicyScope* scope) {
+base::Value* ReadChromeBooleanValue(const string16& name,
+                                    PolicyLevel* level,
+                                    PolicyScope* scope) {
   RegKey key;
   if (!LoadHighestPriorityKey(string16(), name, &key, level, scope))
     return NULL;
@@ -177,9 +189,9 @@ base::Value* ReadBooleanValue(const string16& name,
 
 // Returns the Value for a Chrome integer policy named |name|,
 // or NULL if it wasn't found. The caller owns the returned value.
-base::Value* ReadIntegerValue(const string16& name,
-                              PolicyLevel* level,
-                              PolicyScope* scope) {
+base::Value* ReadChromeIntegerValue(const string16& name,
+                                    PolicyLevel* level,
+                                    PolicyScope* scope) {
   RegKey key;
   if (!LoadHighestPriorityKey(string16(), name, &key, level, scope))
     return NULL;
@@ -191,9 +203,9 @@ base::Value* ReadIntegerValue(const string16& name,
 
 // Returns the Value for a Chrome dictionary policy named |name|,
 // or NULL if it wasn't found. The caller owns the returned value.
-base::Value* ReadDictionaryValue(const string16& name,
-                                 PolicyLevel* level,
-                                 PolicyScope* scope) {
+base::Value* ReadChromeDictionaryValue(const string16& name,
+                                       PolicyLevel* level,
+                                       PolicyScope* scope) {
   // Dictionaries are encoded as JSON strings on Windows.
   //
   // A dictionary could be stored as a subkey, with each of its entries
@@ -216,10 +228,183 @@ base::Value* ReadDictionaryValue(const string16& name,
   return base::JSONReader::Read(UTF16ToUTF8(value));
 }
 
-// Loads the dictionary at |path| in the given |hive|. Ownership is transferred
-// to the caller.
-base::DictionaryValue* ReadRegistryDictionaryValue(HKEY hive,
-                                                   const string16& path) {
+// Returns the Value type described in |schema|, or |default_type| if not found.
+base::Value::Type GetType(const base::DictionaryValue* schema,
+                          base::Value::Type default_type) {
+  // JSON-schema types to base::Value::Type mapping.
+  static const struct {
+    // JSON schema type.
+    const char* schema_type;
+    // Correspondent value type.
+    base::Value::Type value_type;
+  } kSchemaToValueTypeMap[] = {
+    { "array",        base::Value::TYPE_LIST        },
+    { "boolean",      base::Value::TYPE_BOOLEAN     },
+    { "integer",      base::Value::TYPE_INTEGER     },
+    { "null",         base::Value::TYPE_NULL        },
+    { "number",       base::Value::TYPE_DOUBLE      },
+    { "object",       base::Value::TYPE_DICTIONARY  },
+    { "string",       base::Value::TYPE_STRING      },
+  };
+
+  if (!schema)
+    return default_type;
+  std::string type;
+  if (!schema->GetString(kType, &type))
+    return default_type;
+  for (size_t i = 0; i < arraysize(kSchemaToValueTypeMap); ++i) {
+    if (type == kSchemaToValueTypeMap[i].schema_type)
+      return kSchemaToValueTypeMap[i].value_type;
+  }
+  return default_type;
+}
+
+// Returns the default type for registry entries of |reg_type|, when there is
+// no schema defined type for a policy.
+base::Value::Type GetDefaultFor(DWORD reg_type) {
+  return reg_type == REG_DWORD ? base::Value::TYPE_INTEGER :
+                                 base::Value::TYPE_STRING;
+}
+
+// Returns the entry with key |name| in |dictionary| (can be NULL), or NULL.
+base::DictionaryValue* GetEntry(const base::DictionaryValue* dictionary,
+                                const std::string& name) {
+  if (!dictionary)
+    return NULL;
+  base::DictionaryValue* entry = NULL;
+  dictionary->GetDictionary(name, &entry);
+  return entry;
+}
+
+// Converts string |value| to another |type|, if possible.
+base::Value* ConvertComponentStringValue(const string16& value,
+                                         base::Value::Type type) {
+  switch (type) {
+    case base::Value::TYPE_DICTIONARY:
+      return base::JSONReader::Read(UTF16ToUTF8(value));
+
+    case base::Value::TYPE_NULL:
+      return base::Value::CreateNullValue();
+
+    case base::Value::TYPE_DOUBLE: {
+      double double_value;
+      if (base::StringToDouble(UTF16ToUTF8(value), &double_value))
+        return base::Value::CreateDoubleValue(double_value);
+      DLOG(WARNING) << "Failed to read policy value as double: " << value;
+      return NULL;
+    }
+
+    case base::Value::TYPE_STRING:
+      return base::Value::CreateStringValue(value);
+

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

what about TYPE_LIST and TYPE_INTEGER? Should they have conversion handlers here
as well?

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

Sure, done. Also added a conversion to TYPE_BOOLEAN.

+    default:

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

I'd list the remaining enum constants here and remove the default branch in
favor of a NOTREACHED() below.

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

Done.

+      DLOG(WARNING) << "Cannot convert REG_SZ entry to type " << type;
+      return NULL;
+  }
+}
+
+// Converts an integer |value| to another |type|, if possible.
+base::Value* ConvertComponentIntegerValue(uint32 value,
+                                          base::Value::Type type) {
+  switch (type) {
+    case base::Value::TYPE_BOOLEAN:
+      return base::Value::CreateBooleanValue(value != 0);
+
+    case base::Value::TYPE_DOUBLE:
+      return base::Value::CreateDoubleValue(value);
+
+    case base::Value::TYPE_INTEGER:
+      return base::Value::CreateIntegerValue(value);
+
+    default:

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

same comment regarding remaining constants, default branch and NOTREACHED().

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

Done.

+      DLOG(WARNING) << "Cannot convert REG_DWORD entry to type " << type;
+      return NULL;
+  }
+}
+
+// Reads a simple (non-Dictionary, non-Array) value from the registry |key|
+// named |name| with registry type |reg_type| as a value of type |type|.
+// Returns NULL if the value could not be loaded or converted.
+base::Value* ReadComponentSimpleValue(RegKey* key,
+                                      const string16& name,
+                                      DWORD reg_type,
+                                      base::Value::Type type) {
+  switch (reg_type) {
+    case REG_SZ: {
+      string16 value;
+      if (ReadRegistryString(key, name, &value))
+        return ConvertComponentStringValue(value, type);
+      break;
+    }
+
+    case REG_DWORD: {
+      uint32 value;
+      if (ReadRegistryInteger(key, name, &value))
+        return ConvertComponentIntegerValue(value, type);
+      break;
+    }
+
+    default:
+      DLOG(WARNING) << "Registry type not supported for key " << name;
+      break;
+  }
+  NOTREACHED();
+  return NULL;
+}
+
+// Forward declaration for ReadComponentListValue().
+base::DictionaryValue* ReadComponentDictionaryValue(
+    HKEY hive,
+    const string16& path,
+    const base::DictionaryValue* schema);
+
+// Loads the list at |path| in the given |hive|. |schema| is a JSON schema
+// (http://json-schema.org/) that describes the expected type of the list's
+// elements. Ownership of the result is transferred to the caller.
+base::ListValue* ReadComponentListValue(HKEY hive,
+                                        const string16& path,
+                                        const base::DictionaryValue* schema) {
+  // The sub-elements are indexed from 1 to N. They can be represented as
+  // registry values or registry keys though; use |schema| first to try to
+  // determine the right type, and if that fails default to STRING.
+
+  RegKey key(hive, path.c_str(), KEY_READ);
+  if (!key.Valid())
+    return NULL;
+
+  base::Value::Type type = GetType(schema, base::Value::TYPE_STRING);
+  base::ListValue* list = new base::ListValue();
+  for (int i = 1; ; ++i) {
+    string16 name = base::IntToString16(i);
+    base::Value* value = NULL;
+    base::DictionaryValue* sub_schema = NULL;
+    if (type == base::Value::TYPE_DICTIONARY) {
+      value = ReadComponentDictionaryValue(hive, path + kPathSep + name,
+                                           GetEntry(schema, kProperties));
+    } else if (type == base::Value::TYPE_LIST) {
+      value = ReadComponentListValue(hive, path + kPathSep + name,
+                                     GetEntry(schema, kItems));
+    } else {
+      DWORD reg_type;
+      key.ReadValue(name.c_str(), NULL, NULL, &reg_type);
+      value = ReadComponentSimpleValue(&key, name, reg_type, type);
+    }
+    if (!value)
+      break;
+    list->Append(value);
+  }
+  return list;
+}
+
+// Loads the dictionary at |path| in the given |hive|. |schema| is a JSON
+// schema (http://json-schema.org/) that describes the expected types for the
+// dictionary entries. When the type for a certain entry isn't described in the
+// schema, a default conversion takes place. |schema| can be NULL.
+// Ownership of the result is transferred to the caller.
+base::DictionaryValue* ReadComponentDictionaryValue(
+    HKEY hive,
+    const string16& path,
+    const base::DictionaryValue* schema) {
   // A "value" in the registry is like a file in a filesystem, and a "key" is
   // like a directory, that contains other "values" and "keys".
   // Unfortunately it is possible to have a name both as a "value" and a "key".
@@ -228,43 +413,44 @@ base::DictionaryValue* ReadRegistryDictionaryValue(HKEY hive,
   // First iterate over all the "values" in |path| and convert them; then
   // recurse into each "key" in |path| and convert them as dictionaries.
 
-  base::DictionaryValue* dict = new base::DictionaryValue();
   RegKey key(hive, path.c_str(), KEY_READ);
+  if (!key.Valid())
+    return NULL;
 
+  base::DictionaryValue* dict = new base::DictionaryValue();
   for (RegistryValueIterator it(hive, path.c_str()); it.Valid(); ++it) {
-    string16 name(it.Name());
-    switch (it.Type()) {
-      case REG_SZ: {
-        string16 value;
-        if (ReadRegistryString(&key, name, &value))
-          dict->SetString(UTF16ToUTF8(name), value);
-        break;
-      }
-
-      case REG_DWORD: {
-        uint32 value;
-        if (ReadRegistryInteger(&key, name, &value))
-          dict->SetInteger(UTF16ToUTF8(name), value);
-        break;
-      }
-
-      default:
-        // TODO(joaodasilva): use a schema to determine the correct types.
-        LOG(WARNING) << "Ignoring registry value with unsupported type: "
-                     << path << kPathSep << name;
-    }
+    string16 name16(it.Name());
+    std::string name(UTF16ToUTF8(name16));
+    const base::DictionaryValue* sub_schema = GetEntry(schema, name);

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

What about the case of a dictionary with arbitrary keys but a common type for
all entries? Content settings could use that.

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

Great idea, and json-schema even has something for that case
(http://tools.ietf.org/html/draft-zyp-json-schema-03#section-5.4). Done.

+    base::Value::Type type = GetType(sub_schema, GetDefaultFor(it.Type()));
+    base::Value* value =
+        ReadComponentSimpleValue(&key, name16, it.Type(), type);

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

Why can't this be an encoded dictionary or list?

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

Because we're using a RegistryValueIterator, so this can only be a REG_SZ or
REG_DWORD, not a subkey. ReadComponentSimpleValue() will decode a REG_SZ as a
dictionary or list, if the schema says so.

+    if (value)
+      dict->Set(name, value);
   }
 
   for (RegistryKeyIterator it(hive, path.c_str()); it.Valid(); ++it) {
     string16 name16(it.Name());
     std::string name(UTF16ToUTF8(name16));
     if (dict->HasKey(name)) {
-      LOG(WARNING) << "Ignoring registry key because a value exists with the "
-                      "same name: " << path << kPathSep << name;
+      DLOG(WARNING) << "Ignoring registry key because a value exists with the "
+                       "same name: " << path << kPathSep << name;
       continue;
     }
-    base::DictionaryValue* value =
-        ReadRegistryDictionaryValue(hive, path + kPathSep + name16);
+
+    base::DictionaryValue* sub_schema = GetEntry(schema, name);
+    base::Value::Type type = GetType(sub_schema, base::Value::TYPE_DICTIONARY);
+    base::Value* value = NULL;
+    const string16 sub_path = path + kPathSep + name16;
+    if (type == base::Value::TYPE_DICTIONARY) {
+      sub_schema = GetEntry(sub_schema, kProperties);
+      value = ReadComponentDictionaryValue(hive, sub_path, sub_schema);
+    } else if (type == base::Value::TYPE_LIST) {
+      sub_schema = GetEntry(sub_schema, kItems);
+      value = ReadComponentListValue(hive, sub_path, sub_schema);
+    } else {
+      DLOG(WARNING) << "Can't read a simple type in registry key at " << path;
+    }
     if (value)
       dict->Set(name, value);
   }
@@ -272,6 +458,38 @@ base::DictionaryValue* ReadRegistryDictionaryValue(HKEY hive,
   return dict;
 }
 
+// Reads a JSON schema from the given |registry_value|, at the given
+// |registry_key| in |hive|. |registry_value| must be a string (REG_SZ), and
+// is decoded as JSON data. Returns NULL on failure. Ownership is transferred
+// to the caller.
+base::DictionaryValue* ReadRegistrySchema(HKEY hive,
+                                          const string16& registry_key,
+                                          const string16& registry_value) {
+  RegKey key(hive, registry_key.c_str(), KEY_READ);
+  string16 schema;
+  if (!ReadRegistryString(&key, registry_value, &schema))
+    return NULL;
+  // A JSON schema is represented in JSON too.
+  scoped_ptr<base::Value> value(base::JSONReader::Read(UTF16ToUTF8(schema)));
+  if (!value.get())
+    return NULL;
+  base::DictionaryValue* dict = NULL;
+  if (!value->GetAsDictionary(&dict))
+    return NULL;
+  // The top-level entry must be an object, and each of its properties maps
+  // a policy name to its schema.
+  if (GetType(dict, base::Value::TYPE_DICTIONARY) !=
+      base::Value::TYPE_DICTIONARY) {
+    DLOG(WARNING) << "schema top-level type isn't \"object\"";
+    return NULL;
+  }
+  base::DictionaryValue* properties = GetEntry(dict, kProperties);
+  if (!properties)
+    return NULL;
+  value.release();
+  return properties;
+}
+
 }  // namespace
 
 PolicyLoaderWin::PolicyLoaderWin(const PolicyDefinitionList* policy_list)
@@ -323,23 +541,23 @@ void PolicyLoaderWin::LoadChromePolicy(PolicyMap* chrome_policies) {
 
     switch (current->value_type) {
       case base::Value::TYPE_STRING:
-        value = ReadStringValue(name, &level, &scope);
+        value = ReadChromeStringValue(name, &level, &scope);
         break;
 
       case base::Value::TYPE_LIST:
-        value = ReadStringListValue(name, &level, &scope);
+        value = ReadChromeStringListValue(name, &level, &scope);
         break;
 
       case base::Value::TYPE_BOOLEAN:
-        value = ReadBooleanValue(name, &level, &scope);
+        value = ReadChromeBooleanValue(name, &level, &scope);
         break;
 
       case base::Value::TYPE_INTEGER:
-        value = ReadIntegerValue(name, &level, &scope);
+        value = ReadChromeIntegerValue(name, &level, &scope);
         break;
 
       case base::Value::TYPE_DICTIONARY:
-        value = ReadDictionaryValue(name, &level, &scope);
+        value = ReadChromeDictionaryValue(name, &level, &scope);
         break;
 
       default:
@@ -390,12 +608,16 @@ void PolicyLoaderWin::Load3rdPartyPolicies(PolicyBundle* bundle) {
         string16 component(domain_iterator.Name());
         string16 component_path = domain_path + kPathSep + component;
 
+        // Load the schema for this component's policy, if present.
+        scoped_ptr<base::DictionaryValue> schema(
+            ReadRegistrySchema(hkey, component_path, kSchema));
+
         for (size_t k = 0; k < arraysize(kKeyPaths); ++k) {
           string16 path =
               component_path + kPathSep + ASCIIToUTF16(kKeyPaths[k].path);
 
           scoped_ptr<base::DictionaryValue> dictionary(
-              ReadRegistryDictionaryValue(hkey, path));
+              ReadComponentDictionaryValue(hkey, path, schema.get()));
           if (dictionary.get()) {
             PolicyMap policies;
             policies.LoadFrom(