Allow trailing comma in JSON policy files.

This is not really part of the JSON standard but a common practice and is
harmless in most of the cases so we can enable it for JSON policy files.

BUG=101141
TEST=none


Review URL: http://codereview.chromium.org/8423079

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@110503 0039d316-1c4b-4281-b951-d872f2087c98

base/base.gyp

@@ -150,6 +150,7 @@
         'i18n/string_search_unittest.cc',
         'i18n/time_formatting_unittest.cc',
         'json/json_reader_unittest.cc',
+        'json/json_value_serializer_unittest.cc',
         'json/json_writer_unittest.cc',
         'json/string_escape_unittest.cc',
         'lazy_instance_unittest.cc',

base/json/json_value_serializer.cc

@@ -104,5 +104,6 @@ Value* JSONFileValueSerializer::Deserialize(int* error_code,
   }
 
   JSONStringValueSerializer serializer(json_string);
+  serializer.set_allow_trailing_comma(allow_trailing_comma_);
   return serializer.Deserialize(error_code, error_str);
 }

base/json/json_value_serializer.h

@@ -75,7 +75,8 @@ class BASE_EXPORT JSONFileValueSerializer : public base::ValueSerializer {
   // When deserializing, the file should exist, but when serializing, the
   // serializer will attempt to create the file at the specified location.
   explicit JSONFileValueSerializer(const FilePath& json_file_path)
-    : json_file_path_(json_file_path) {}
+    : json_file_path_(json_file_path),
+      allow_trailing_comma_(false) {}
 
   virtual ~JSONFileValueSerializer() {}
 
@@ -118,8 +119,13 @@ class BASE_EXPORT JSONFileValueSerializer : public base::ValueSerializer {
   // be a JsonFileError.
   static const char* GetErrorMessageForCode(int error_code);
 
+  void set_allow_trailing_comma(bool new_value) {
+    allow_trailing_comma_ = new_value;
+  }
+
  private:
   FilePath json_file_path_;
+  bool allow_trailing_comma_;
 
   // A wrapper for file_util::ReadFileToString which returns a non-zero
   // JsonFileError if there were file errors.

base/json/json_value_serializer_unittest.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 "base/json/json_value_serializer.h"
+
+#include <string>
+
+#include "base/file_util.h"
+#include "base/json/json_reader.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/scoped_temp_dir.h"
+#include "base/string_util.h"
+#include "base/values.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+namespace {
+
+// Some proper JSON to test with:
+const char kProperJSON[] =
+    "{\n"
+    "   \"compound\": {\n"
+    "      \"a\": 1,\n"
+    "      \"b\": 2\n"
+    "   },\n"
+    "   \"some_String\": \"1337\",\n"
+    "   \"some_int\": 42,\n"
+    "   \"the_list\": [ \"val1\", \"val2\" ]\n"
+    "}\n";
+
+// Some proper JSON with trailing commas:
+const char kProperJSONWithCommas[] =
+    "{\n"
+    "\t\"some_int\": 42,\n"
+    "\t\"some_String\": \"1337\",\n"
+    "\t\"the_list\": [\"val1\", \"val2\", ],\n"
+    "\t\"compound\": { \"a\": 1, \"b\": 2, },\n"
+    "}\n";
+
+const char kWinLineEnds[] = "\r\n";
+const char kLinuxLineEnds[] = "\n";
+
+// Verifies the generated JSON against the expected output.
+void CheckJSONIsStillTheSame(Value& value) {
+  // Serialize back the output.
+  std::string serialized_json;
+  JSONStringValueSerializer str_serializer(&serialized_json);
+  str_serializer.set_pretty_print(true);
+  ASSERT_TRUE(str_serializer.Serialize(value));
+  // Unify line endings between platforms.
+  ReplaceSubstringsAfterOffset(&serialized_json, 0,
+                               kWinLineEnds, kLinuxLineEnds);
+  // Now compare the input with the output.
+  ASSERT_EQ(kProperJSON, serialized_json);
+}
+
+// Test proper JSON [de]serialization from string is working.
+TEST(JSONValueSerializerTest, ReadProperJSONFromString) {
+  // Try to deserialize it through the serializer.
+  std::string proper_json(kProperJSON);
+  JSONStringValueSerializer str_deserializer(proper_json);
+
+  int error_code = 0;
+  std::string error_message;
+  scoped_ptr<Value> value(
+      str_deserializer.Deserialize(&error_code, &error_message));
+  ASSERT_TRUE(value.get());
+  ASSERT_EQ(0, error_code);
+  ASSERT_TRUE(error_message.empty());
+  // Verify if the same JSON is still there.
+  CheckJSONIsStillTheSame(*value);
+}
+
+// Test that trialing commas are only properly deserialized from string when
+// the proper flag for that is set.
+TEST(JSONValueSerializerTest, ReadJSONWithTrailingCommasFromString) {
+  // Try to deserialize it through the serializer.
+  std::string proper_json(kProperJSONWithCommas);
+  JSONStringValueSerializer str_deserializer(proper_json);
+
+  int error_code = 0;
+  std::string error_message;
+  scoped_ptr<Value> value(
+      str_deserializer.Deserialize(&error_code, &error_message));
+  ASSERT_FALSE(value.get());
+  ASSERT_NE(0, error_code);
+  ASSERT_FALSE(error_message.empty());
+  // Now the flag is set and it must pass.
+  str_deserializer.set_allow_trailing_comma(true);
+  value.reset(str_deserializer.Deserialize(&error_code, &error_message));
+  ASSERT_TRUE(value.get());
+  ASSERT_EQ(JSONReader::JSON_TRAILING_COMMA, error_code);
+  // Verify if the same JSON is still there.
+  CheckJSONIsStillTheSame(*value);
+}
+
+// Test proper JSON [de]serialization from file is working.
+TEST(JSONValueSerializerTest, ReadProperJSONFromFile) {
+  ScopedTempDir tempdir;
+  ASSERT_TRUE(tempdir.CreateUniqueTempDir());
+  // Write it down in the file.
+  FilePath temp_file(tempdir.path().AppendASCII("test.json"));
+  ASSERT_EQ(static_cast<int>(strlen(kProperJSON)),
+            file_util::WriteFile(temp_file, kProperJSON, strlen(kProperJSON)));
+
+  // Try to deserialize it through the serializer.
+  JSONFileValueSerializer file_deserializer(temp_file);
+
+  int error_code = 0;
+  std::string error_message;
+  scoped_ptr<Value> value(
+      file_deserializer.Deserialize(&error_code, &error_message));
+  ASSERT_TRUE(value.get());
+  ASSERT_EQ(0, error_code);
+  ASSERT_TRUE(error_message.empty());
+  // Verify if the same JSON is still there.
+  CheckJSONIsStillTheSame(*value);
+}
+
+// Test that trialing commas are only properly deserialized from file when
+// the proper flag for that is set.
+TEST(JSONValueSerializerTest, ReadJSONWithCommasFromFile) {
+  ScopedTempDir tempdir;
+  ASSERT_TRUE(tempdir.CreateUniqueTempDir());
+  // Write it down in the file.
+  FilePath temp_file(tempdir.path().AppendASCII("test.json"));
+  ASSERT_EQ(static_cast<int>(strlen(kProperJSONWithCommas)),
+            file_util::WriteFile(temp_file,
+                                 kProperJSONWithCommas,
+                                 strlen(kProperJSONWithCommas)));
+
+  // Try to deserialize it through the serializer.
+  JSONFileValueSerializer file_deserializer(temp_file);
+  // This must fail without the proper flag.
+  int error_code = 0;
+  std::string error_message;
+  scoped_ptr<Value> value(
+      file_deserializer.Deserialize(&error_code, &error_message));
+  ASSERT_FALSE(value.get());
+  ASSERT_NE(0, error_code);
+  ASSERT_FALSE(error_message.empty());
+  // Now the flag is set and it must pass.
+  file_deserializer.set_allow_trailing_comma(true);
+  value.reset(file_deserializer.Deserialize(&error_code, &error_message));
+  ASSERT_TRUE(value.get());
+  ASSERT_EQ(JSONReader::JSON_TRAILING_COMMA, error_code);
+  // Verify if the same JSON is still there.
+  CheckJSONIsStillTheSame(*value);
+}
+
+}  // namespace
+
+}  // namespace base
+

chrome/browser/policy/config_dir_policy_provider.cc

@@ -34,6 +34,7 @@ DictionaryValue* ConfigDirPolicyProviderDelegate::Load() {
   for (std::set<FilePath>::iterator config_file_iter = files.begin();
        config_file_iter != files.end(); ++config_file_iter) {
     JSONFileValueSerializer deserializer(*config_file_iter);
+    deserializer.set_allow_trailing_comma(true);
     int error_code = 0;
     std::string error_msg;
     scoped_ptr<Value> value(deserializer.Deserialize(&error_code, &error_msg));