Enable non-sequential entries when loading registry lists.

components/policy/core/common/registry_dict_win.cc

 // Copyright 2013 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 "components/policy/core/common/registry_dict_win.h"
 
 #include <utility>
 
 #include "base/json/json_reader.h"
 #include "base/memory/ptr_util.h"
@@ skipping 85 matching lines @@
         return std::unique_ptr<base::Value>(
             new base::FundamentalValue(double_value));
       }
       break;
     }
     case base::Value::TYPE_LIST: {
       // Lists are encoded as subkeys with numbered value in the registry.
       const base::DictionaryValue* dict = nullptr;
       if (value.GetAsDictionary(&dict)) {
         std::unique_ptr<base::ListValue> result(new base::ListValue());
         for (int i = 1; ; ++i) {

[pastarmovj, 2016/11/03 16:27:47]

Don't you need to fix this part too?

[Georges Khalil, 2016/11/03 18:16:16]

Oops, will fix it.

           const base::Value* entry = nullptr;
           if (!dict->Get(base::IntToString(i), &entry))
             break;
           std::unique_ptr<base::Value> converted =
               ConvertValue(*entry, schema.GetItems());
           if (converted)
             result->Append(converted.release());
         }
         return std::move(result);
       }
@@ skipping 198 matching lines @@
         std::unique_ptr<base::Value> converted =
             entry->second->ConvertToJSON(subschema);
         if (converted)
           result->SetWithoutPathExpansion(entry->first, converted.release());
       }
       return std::move(result);
     }
     case base::Value::TYPE_LIST: {
       std::unique_ptr<base::ListValue> result(new base::ListValue());
       Schema item_schema = schema.valid() ? schema.GetItems() : Schema();
-      for (int i = 1; ; ++i) {
-        const std::string name(base::IntToString(i));
-        const RegistryDict* key = GetKey(name);
-        if (key) {
-          std::unique_ptr<base::Value> converted =
-              key->ConvertToJSON(item_schema);
-          if (converted)
-            result->Append(converted.release());
-          continue;
-        }
-        const base::Value* value = GetValue(name);
-        if (value) {
-          std::unique_ptr<base::Value> converted =
-              ConvertValue(*value, item_schema);
-          if (converted)
-            result->Append(converted.release());
-          continue;
-        }
-        break;
+      for (RegistryDict::KeyMap::const_iterator entry(keys_.begin());
+           entry != keys_.end(); ++entry) {

[pastarmovj, 2016/11/03 16:27:47]

I think it still makes sense to only parse numbered keys. The code as it is now
will parse a key with arbitrary names. Sometimes while testing polivies admins
tend to rename them to switch them off e.g. "HomePageUrl" -> "_HomePageUrl" one
can imagine they would do this to the indexed entries too so skipping anything
that is not a number sounds like a good idea to me. Wdyt?

If you agree about this change please augment the test to capture this too.

[Georges Khalil, 2016/11/03 18:16:16]

Skipping over non-integer names makes sense.

We won't enforce that integers have to be increasing, however. I don't see any
issues with that, WDYT? ie. 1, 5, 7, 3, 6 would be valid.

[Georges Khalil, 2016/11/03 20:24:54]

I just realized that a dictionary uses a map instead of an unordered_map, so
that's irrelevant, the number will always be increasing.

I'm adding just the numerical check.

+        std::unique_ptr<base::Value> converted =
+            entry->second->ConvertToJSON(item_schema);
+        if (converted)
+          result->Append(converted.release());
+      }
+      for (RegistryDict::ValueMap::const_iterator entry(values_.begin());
+           entry != values_.end(); ++entry) {
+        std::unique_ptr<base::Value> converted =
+            ConvertValue(*entry->second, item_schema);
+        if (converted)
+          result->Append(converted.release());
       }
       return std::move(result);
     }
     default:
       LOG(WARNING) << "Can't convert registry key to schema type " << type;
   }
 
   return nullptr;
 }
 
 }  // namespace policy