Merge pull request open-source-parsers#1098 from cdunn2001/allow-trailing-commas

cdunn2001 · web-flow · commit 781eec4da804 · 2019-11-14T00:21:18.000-06:00
Allow trailing comma in objects and arrays
diff --git a/include/json/json_features.h b/include/json/json_features.h
@@ -23,6 +23,7 @@ class JSON_API Features {
   /** \brief A configuration that allows all features and assumes all strings
    * are UTF-8.
    * - C & C++ comments are allowed
+   * - Trailing commas in objects and arrays are allowed.
    * - Root object can be any JSON value
    * - Assumes Value strings are encoded in UTF-8
    */
@@ -31,6 +32,7 @@ class JSON_API Features {
   /** \brief A configuration that is strictly compatible with the JSON
    * specification.
    * - Comments are forbidden.
+   * - Trailing commas in objects and arrays are forbidden.
    * - Root object must be either an array or an object value.
    * - Assumes Value strings are encoded in UTF-8
    */
@@ -43,6 +45,10 @@ class JSON_API Features {
   /// \c true if comments are allowed. Default: \c true.
   bool allowComments_{true};
 
+  /// \c true if trailing commas in objects and arrays are allowed. Default \c
+  /// true.
+  bool allowTrailingCommas_{true};
+
   /// \c true if root must be either an array or an object value. Default: \c
   /// false.
   bool strictRoot_{false};
diff --git a/include/json/reader.h b/include/json/reader.h
@@ -299,6 +299,8 @@ class JSON_API CharReaderBuilder : public CharReader::Factory {
    *     if allowComments is false.
    * - `"allowComments": false or true`
    *   - true if comments are allowed.
+   * - `"allowTrailingCommas": false or true`
+   *   - true if trailing commas in objects and arrays are allowed.
    * - `"strictRoot": false or true`
    *   - true if root must be either an array or an object value
    * - `"allowDroppedNullPlaceholders": false or true`
diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp
@@ -67,6 +67,7 @@ Features Features::all() { return {}; }
 Features Features::strictMode() {
   Features features;
   features.allowComments_ = false;
+  features.allowTrailingCommas_ = false;
   features.strictRoot_ = true;
   features.allowDroppedNullPlaceholders_ = false;
   features.allowNumericKeys_ = false;
@@ -454,7 +455,9 @@ bool Reader::readObject(Token& token) {
       initialTokenOk = readToken(tokenName);
     if (!initialTokenOk)
       break;
-    if (tokenName.type_ == tokenObjectEnd && name.empty()) // empty object
+    if (tokenName.type_ == tokenObjectEnd &&
+        (name.empty() ||
+         features_.allowTrailingCommas_)) // empty object or trailing comma
       return true;
     name.clear();
     if (tokenName.type_ == tokenString) {
@@ -502,15 +505,20 @@ bool Reader::readArray(Token& token) {
   Value init(arrayValue);
   currentValue().swapPayload(init);
   currentValue().setOffsetStart(token.start_ - begin_);
-  skipSpaces();
-  if (current_ != end_ && *current_ == ']') // empty array
-  {
-    Token endArray;
-    readToken(endArray);
-    return true;
-  }
   int index = 0;
   for (;;) {
+    skipSpaces();
+    if (current_ != end_ && *current_ == ']' &&
+        (index == 0 ||
+         (features_.allowTrailingCommas_ &&
+          !features_.allowDroppedNullPlaceholders_))) // empty array or trailing
+                                                      // comma
+    {
+      Token endArray;
+      readToken(endArray);
+      return true;
+    }
+
     Value& value = currentValue()[index++];
     nodes_.push(&value);
     bool ok = readValue();
@@ -863,6 +871,7 @@ class OurFeatures {
 public:
   static OurFeatures all();
   bool allowComments_;
+  bool allowTrailingCommas_;
   bool strictRoot_;
   bool allowDroppedNullPlaceholders_;
   bool allowNumericKeys_;
@@ -1437,7 +1446,9 @@ bool OurReader::readObject(Token& token) {
       initialTokenOk = readToken(tokenName);
     if (!initialTokenOk)
       break;
-    if (tokenName.type_ == tokenObjectEnd && name.empty()) // empty object
+    if (tokenName.type_ == tokenObjectEnd &&
+        (name.empty() ||
+         features_.allowTrailingCommas_)) // empty object or trailing comma
       return true;
     name.clear();
     if (tokenName.type_ == tokenString) {
@@ -1491,15 +1502,19 @@ bool OurReader::readArray(Token& token) {
   Value init(arrayValue);
   currentValue().swapPayload(init);
   currentValue().setOffsetStart(token.start_ - begin_);
-  skipSpaces();
-  if (current_ != end_ && *current_ == ']') // empty array
-  {
-    Token endArray;
-    readToken(endArray);
-    return true;
-  }
   int index = 0;
   for (;;) {
+    skipSpaces();
+    if (current_ != end_ && *current_ == ']' &&
+        (index == 0 ||
+         (features_.allowTrailingCommas_ &&
+          !features_.allowDroppedNullPlaceholders_))) // empty array or trailing
+                                                      // comma
+    {
+      Token endArray;
+      readToken(endArray);
+      return true;
+    }
     Value& value = currentValue()[index++];
     nodes_.push(&value);
     bool ok = readValue();
@@ -1866,6 +1881,7 @@ CharReader* CharReaderBuilder::newCharReader() const {
   bool collectComments = settings_["collectComments"].asBool();
   OurFeatures features = OurFeatures::all();
   features.allowComments_ = settings_["allowComments"].asBool();
+  features.allowTrailingCommas_ = settings_["allowTrailingCommas"].asBool();
   features.strictRoot_ = settings_["strictRoot"].asBool();
   features.allowDroppedNullPlaceholders_ =
       settings_["allowDroppedNullPlaceholders"].asBool();
@@ -1884,6 +1900,7 @@ static void getValidReaderKeys(std::set<String>* valid_keys) {
   valid_keys->clear();
   valid_keys->insert("collectComments");
   valid_keys->insert("allowComments");
+  valid_keys->insert("allowTrailingCommas");
   valid_keys->insert("strictRoot");
   valid_keys->insert("allowDroppedNullPlaceholders");
   valid_keys->insert("allowNumericKeys");
@@ -1917,6 +1934,7 @@ Value& CharReaderBuilder::operator[](const String& key) {
 void CharReaderBuilder::strictMode(Json::Value* settings) {
   //! [CharReaderBuilderStrictMode]
   (*settings)["allowComments"] = false;
+  (*settings)["allowTrailingCommas"] = false;
   (*settings)["strictRoot"] = true;
   (*settings)["allowDroppedNullPlaceholders"] = false;
   (*settings)["allowNumericKeys"] = false;
@@ -1932,6 +1950,7 @@ void CharReaderBuilder::setDefaults(Json::Value* settings) {
   //! [CharReaderBuilderDefaults]
   (*settings)["collectComments"] = true;
   (*settings)["allowComments"] = true;
+  (*settings)["allowTrailingCommas"] = true;
   (*settings)["strictRoot"] = false;
   (*settings)["allowDroppedNullPlaceholders"] = false;
   (*settings)["allowNumericKeys"] = false;
diff --git a/src/test_lib_json/fuzz.cpp b/src/test_lib_json/fuzz.cpp
@@ -40,6 +40,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   builder.settings_["rejectDupKeys_"] = hash_settings & (1 << 7);
   builder.settings_["allowSpecialFloats_"] = hash_settings & (1 << 8);
   builder.settings_["collectComments"] = hash_settings & (1 << 9);
+  builder.settings_["allowTrailingCommas_"] = hash_settings & (1 << 10);
 
   std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
 
diff --git a/test/data/fail_test_array_02.json b/test/data/fail_test_array_02.json
@@ -0,0 +1 @@
+[1,,]
diff --git a/test/data/fail_test_object_01.json b/test/data/fail_test_object_01.json
@@ -0,0 +1 @@
+{ "count" : 1234,, }
diff --git a/test/data/test_array_08.expected b/test/data/test_array_08.expected
@@ -0,0 +1,2 @@
+.=[]
+.[0]=1
diff --git a/test/data/test_array_08.json b/test/data/test_array_08.json
@@ -0,0 +1 @@
+[1,]
diff --git a/test/data/test_object_05.expected b/test/data/test_object_05.expected
@@ -0,0 +1,2 @@
+.={}
+.count=1234
diff --git a/test/data/test_object_05.json b/test/data/test_object_05.json
@@ -0,0 +1 @@
+{ "count" : 1234, }
