Add fuzzer test for preg_parser

components/policy/core/common/preg_parser.h

 // Copyright (c) 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.
 
 // This file provides a parser for PReg files which are used for storing group
 // policy settings in the file system. The file format is documented here:
 //
 // http://msdn.microsoft.com/en-us/library/windows/desktop/aa374407(v=vs.85).aspx
 
 #ifndef COMPONENTS_POLICY_CORE_COMMON_PREG_PARSER_H_
 #define COMPONENTS_POLICY_CORE_COMMON_PREG_PARSER_H_
 
 #include <memory>
 #include <vector>
 
 #include "base/strings/string16.h"
+#include "components/policy/core/common/policy_load_status.h"

[Thiemo Nagel, 2017/04/12 17:10:06]

Why have you removed the forward declaration of PolicyLoadStatusSample? -- I'd
have thought that should cover you.

[ljusten (tachyonic), 2017/04/13 11:17:15]

The internal method takes a PolicyLoadStatus, which is an enum in this .h file.
PolicyLoadStatusSample sends off a UMA sample at destruction, which fills up
memory in the fuzzer.

 #include "components/policy/policy_export.h"
 
 namespace base {
 class FilePath;
 }
 
 namespace policy {
 
-class PolicyLoadStatusSample;
 class RegistryDict;
 
 namespace preg_parser {
 
 // The magic header in PReg files: ASCII "PReg" + version (0x0001).
 POLICY_EXPORT extern const char kPRegFileHeader[8];
 
 // Reads the PReg file at |file_path| and writes the registry data to |dict|.
-// |root| specifies the registry subtree the caller is interested in,
-// everything else gets ignored.
+// |root| specifies the registry subtree the caller is interested in, everything
+// else gets ignored.
 POLICY_EXPORT bool ReadFile(const base::FilePath& file_path,
                             const base::string16& root,
                             RegistryDict* dict,
-                            PolicyLoadStatusSample* status);
+                            PolicyLoadStatusSample* status_sample);
+
+// Similar to ReadFile, but reads from |data| of length |data_size| instead of
+// a file, and writes status to the enum PolicyLoadStatus, which does not record
+// UMA stats, unlike |PolicyLoadStatusSample|. |debug_name| is printed out along
+// with error messages. Used internally only for testing. For production code,

[Thiemo Nagel, 2017/04/12 17:10:06]

Nit: "only for testing" seems not correct as it's used for the implementation of
ReadFile() as well.

[ljusten (tachyonic), 2017/04/13 11:17:15]

Clarified the wording.

+// use ReadFile instead.
+POLICY_EXPORT bool ReadDataInternal(const uint8_t* data,
+                                    size_t data_size,
+                                    const base::string16& root,
+                                    RegistryDict* dict,
+                                    PolicyLoadStatus* status,

[Thiemo Nagel, 2017/04/12 17:10:06]

Nit: Returning a bool *and* updating |status| seems redundant.

[ljusten (tachyonic), 2017/04/13 11:17:14]

PolicyLoadStatus is used as UMA stat and does not have an 'OK' status. Adding an
OK status at the end of the list or using POLICY_LOAD_STATUS_SIZE to signal 'OK'
would feel a bit hacky.

+                                    const std::string& debug_name);
 
 }  // namespace preg_parser
 }  // namespace policy
 
 #endif  // COMPONENTS_POLICY_CORE_COMMON_PREG_PARSER_H_