Merge pull request #1132 from m-holger/jdk

In JSON::parse allow duplicate dictionary keys

Merge pull request #1132 from m-holger/jdk
In JSON::parse allow duplicate dictionary keys
m-holger · GitHub
2 parents 183ff8b9 78012927
Showing 9 changed files with 41 additions and 35 deletions
include/qpdf/JSON.hh
libqpdf/JSON.cc
libtests/json.cc
libtests/libtests.testcov
libtests/qtest/json_parse.test
libtests/qtest/json_parse/good-12-react.out
libtests/qtest/json_parse/good-12.json
libtests/qtest/json_parse/save-12.json
manual/release-notes.rst
@@ -143,12 +143,6 @@ class JSON
     QPDF_DLL
     bool isDictionary() const;
-    // If the key is already in the dictionary, return true. Otherwise, mark it as seen and return
-    // false. This is primarily intended to be used by the parser to detect duplicate keys when the
-    // reactor blocks them from being added to the final dictionary.
-    QPDF_DLL
-    bool checkDictionaryKeySeen(std::string const& key);
-
     // Accessors. Accessor behavior:
     //
     // - If argument is wrong type, including null, return false
@@ -327,7 +321,6 @@ class JSON
         ~JSON_dictionary() override = default;
         void write(Pipeline*, size_t depth) const override;
         std::map<std::string, JSON> members;
-        std::set<std::string> parsed_keys;
     };
     struct JSON_array;
     struct JSON_string: public JSON_value
@@ -287,16 +287,6 @@ JSON::addDictionaryMember(std::string const&amp; key, JSON const&amp; val)
     }
 }
-bool
-JSON::checkDictionaryKeySeen(std::string const& key)
-{
-    if (auto* obj = m ? dynamic_cast<JSON_dictionary*>(m->value.get()) : nullptr) {
-        return !obj->parsed_keys.insert(key).second;
-    }
-    throw std::logic_error("JSON::checkDictionaryKey called on non-dictionary");
-    return false; // unreachable
-}
-
 JSON
 JSON::makeArray()
 {
@@ -1266,11 +1256,6 @@ JSONParser::handleToken()
         break;
     case ps_dict_after_colon:
-        if (tos.checkDictionaryKeySeen(dict_key)) {
-            QTC::TC("libtests", "JSON parse duplicate key");
-            throw std::runtime_error(
-                "JSON: offset " + std::to_string(dict_key_offset) + ": duplicated dictionary key");
-        }
         if (!reactor || !reactor->dictionaryItem(dict_key, item)) {
             tos.addDictionaryMember(dict_key, item);
         }
@@ -162,11 +162,6 @@ test_main()
     assert(jarr.addArrayElement(uninitialized).isNull());
     assert(!uninitialized.isArray());
     assert(!uninitialized.isDictionary());
-    try {
-        uninitialized.checkDictionaryKeySeen("key");
-        assert(false);
-    } catch (std::logic_error&) {
-    }
     std::string st_out = "unchanged";
     assert(!uninitialized.getString(st_out));
     assert(!uninitialized.getNumber(st_out));
@@ -90,6 +90,5 @@ JSON optional key 0
 JSON 16 high high 0
 JSON 16 low not after high 0
 JSON 16 dangling high 0
-JSON parse duplicate key 0
 JSON schema array for single item 0
 JSON schema array length mismatch 0
@@ -32,7 +32,7 @@ if ($^O ne &#39;msys&#39;)
 cleanup();
-my $good = 11;
+my $good = 12;
 for (my $i = 1; $i <= $good; ++$i)
 {
@@ -120,7 +120,7 @@ my @bad = (
     "stray low surrogate",      # 37
     "high high surrogate",      # 38
     "dangling high surrogate",  # 39
-    "duplicate dictionary key", # 40
+    undef,                      # 40, removed
     "decimal point after minus",# 41
     "e after minus",            # 42
     "missing digit after e",    # 43
@@ -136,10 +136,22 @@ foreach my $d (@bad)
 {
     ++$i;
     my $n = sprintf("%02d", $i);
-    $td->runtest("$n: $d",
-                 {$td->COMMAND => "json_parse bad-$n.json"},
-                 {$td->FILE => "bad-$n.out", $td->EXIT_STATUS => 2},
-                 $td->NORMALIZE_NEWLINES);
+    if (defined $d)
+    {
+        $td->runtest("$n: $d",
+                     {$td->COMMAND => "json_parse bad-$n.json"},
+                     {$td->FILE => "bad-$n.out", $td->EXIT_STATUS => 2},
+                     $td->NORMALIZE_NEWLINES);
+    }
+    else
+    {
+        # We used to disallow duplicated keys but no longer do. Add
+        # this hack to ignore a test number rather than renaming
+        # tests.
+        $td->runtest("$n: no longer used",
+                     {$td->STRING => ""},
+                     {$td->STRING => ""});
+    }
 }
 cleanup();
+dictionary start
+dictionary item: one -> [11, 12): 1
+dictionary item: two -> [23, 24): 2
+dictionary item: one -> [35, 36): 3
+dictionary item: four -> [48, 49): 4
+container end: [0, 51): {}
+{}
+{
+  "one": 1,
+  "two": 2,
+  "one": 3,
+  "four": 4
+}
+{
+  "four": 4,
+  "one": 3,
+  "two": 2
+}
@@ -11,6 +11,10 @@ For a detailed list of changes, please see the file
 12.0.0: not yet released
   - API: breaking changes
+    - ``JSON::checkDictionaryKeySeen`` has been removed. If ``JSON::parse``
+      encounters duplicate keys the last value is silently accepted instead
+      of throwing a runtime error.
+
     - Deprecated versionless overload of ``QPDFObjectHandle::getJSON``
       has been removed.
@@ -20,7 +24,7 @@ For a detailed list of changes, please see the file
       or ``Buffer buffer2{buffer1.copy()};`` to make it explicit that
       copying is intended.
-    - ``QIntC.hh`` contained the typ0 ``substract`` in function names,
+    - ``QIntC.hh`` contained the typo ``substract`` in function names,
       which has been fixed to ``subtract``.
   - Library Enhancements