Commit 3eb77a700434ed6d9b51e326fa4d49c530fcd473
1 parent
6d4e3ba8
JSON: detect duplicate dictionary keys while parsing
Showing
6 changed files
with
44 additions
and
1 deletions
include/qpdf/JSON.hh
| @@ -42,6 +42,7 @@ | @@ -42,6 +42,7 @@ | ||
| 42 | #include <list> | 42 | #include <list> |
| 43 | #include <map> | 43 | #include <map> |
| 44 | #include <memory> | 44 | #include <memory> |
| 45 | +#include <set> | ||
| 45 | #include <string> | 46 | #include <string> |
| 46 | #include <vector> | 47 | #include <vector> |
| 47 | 48 | ||
| @@ -149,6 +150,14 @@ class JSON | @@ -149,6 +150,14 @@ class JSON | ||
| 149 | QPDF_DLL | 150 | QPDF_DLL |
| 150 | bool isDictionary() const; | 151 | bool isDictionary() const; |
| 151 | 152 | ||
| 153 | + // If the key is already in the dictionary, return true. | ||
| 154 | + // Otherwise, mark it has seen and return false. This is primarily | ||
| 155 | + // intended to used by the parser to detect duplicate keys when | ||
| 156 | + // the reactor blocks them from being added to the final | ||
| 157 | + // dictionary. | ||
| 158 | + QPDF_DLL | ||
| 159 | + bool checkDictionaryKeySeen(std::string const& key); | ||
| 160 | + | ||
| 152 | // Accessors. Accessor behavior: | 161 | // Accessors. Accessor behavior: |
| 153 | // | 162 | // |
| 154 | // - If argument is wrong type, including null, return false | 163 | // - If argument is wrong type, including null, return false |
| @@ -314,6 +323,7 @@ class JSON | @@ -314,6 +323,7 @@ class JSON | ||
| 314 | virtual ~JSON_dictionary() = default; | 323 | virtual ~JSON_dictionary() = default; |
| 315 | virtual void write(Pipeline*, size_t depth) const; | 324 | virtual void write(Pipeline*, size_t depth) const; |
| 316 | std::map<std::string, std::shared_ptr<JSON_value>> members; | 325 | std::map<std::string, std::shared_ptr<JSON_value>> members; |
| 326 | + std::set<std::string> parsed_keys; | ||
| 317 | }; | 327 | }; |
| 318 | struct JSON_array: public JSON_value | 328 | struct JSON_array: public JSON_value |
| 319 | { | 329 | { |
libqpdf/JSON.cc
| @@ -274,6 +274,21 @@ JSON::addDictionaryMember(std::string const& key, JSON const& val) | @@ -274,6 +274,21 @@ JSON::addDictionaryMember(std::string const& key, JSON const& val) | ||
| 274 | return obj->members[encode_string(key)]; | 274 | return obj->members[encode_string(key)]; |
| 275 | } | 275 | } |
| 276 | 276 | ||
| 277 | +bool | ||
| 278 | +JSON::checkDictionaryKeySeen(std::string const& key) | ||
| 279 | +{ | ||
| 280 | + JSON_dictionary* obj = dynamic_cast<JSON_dictionary*>(this->m->value.get()); | ||
| 281 | + if (0 == obj) { | ||
| 282 | + throw std::logic_error( | ||
| 283 | + "JSON::checkDictionaryKey called on non-dictionary"); | ||
| 284 | + } | ||
| 285 | + if (obj->parsed_keys.count(key)) { | ||
| 286 | + return true; | ||
| 287 | + } | ||
| 288 | + obj->parsed_keys.insert(key); | ||
| 289 | + return false; | ||
| 290 | +} | ||
| 291 | + | ||
| 277 | JSON | 292 | JSON |
| 278 | JSON::makeArray() | 293 | JSON::makeArray() |
| 279 | { | 294 | { |
| @@ -565,7 +580,8 @@ namespace | @@ -565,7 +580,8 @@ namespace | ||
| 565 | u_count(0), | 580 | u_count(0), |
| 566 | offset(0), | 581 | offset(0), |
| 567 | done(false), | 582 | done(false), |
| 568 | - parser_state(ps_top) | 583 | + parser_state(ps_top), |
| 584 | + dict_key_offset(0) | ||
| 569 | { | 585 | { |
| 570 | } | 586 | } |
| 571 | 587 | ||
| @@ -625,6 +641,7 @@ namespace | @@ -625,6 +641,7 @@ namespace | ||
| 625 | std::vector<std::shared_ptr<JSON>> stack; | 641 | std::vector<std::shared_ptr<JSON>> stack; |
| 626 | std::vector<parser_state_e> ps_stack; | 642 | std::vector<parser_state_e> ps_stack; |
| 627 | std::string dict_key; | 643 | std::string dict_key; |
| 644 | + size_t dict_key_offset; | ||
| 628 | }; | 645 | }; |
| 629 | } // namespace | 646 | } // namespace |
| 630 | 647 | ||
| @@ -1201,11 +1218,18 @@ JSONParser::handleToken() | @@ -1201,11 +1218,18 @@ JSONParser::handleToken() | ||
| 1201 | case ps_dict_begin: | 1218 | case ps_dict_begin: |
| 1202 | case ps_dict_after_comma: | 1219 | case ps_dict_after_comma: |
| 1203 | this->dict_key = s_value; | 1220 | this->dict_key = s_value; |
| 1221 | + this->dict_key_offset = item->getStart(); | ||
| 1204 | item = nullptr; | 1222 | item = nullptr; |
| 1205 | next_state = ps_dict_after_key; | 1223 | next_state = ps_dict_after_key; |
| 1206 | break; | 1224 | break; |
| 1207 | 1225 | ||
| 1208 | case ps_dict_after_colon: | 1226 | case ps_dict_after_colon: |
| 1227 | + if (tos->checkDictionaryKeySeen(dict_key)) { | ||
| 1228 | + QTC::TC("libtests", "JSON parse duplicate key"); | ||
| 1229 | + throw std::runtime_error( | ||
| 1230 | + "JSON: offset " + QUtil::uint_to_string(dict_key_offset) + | ||
| 1231 | + ": duplicated dictionary key"); | ||
| 1232 | + } | ||
| 1209 | if (!reactor || !reactor->dictionaryItem(dict_key, *item)) { | 1233 | if (!reactor || !reactor->dictionaryItem(dict_key, *item)) { |
| 1210 | tos->addDictionaryMember(dict_key, *item); | 1234 | tos->addDictionaryMember(dict_key, *item); |
| 1211 | } | 1235 | } |
libtests/libtests.testcov
libtests/qtest/json_parse.test
| @@ -120,6 +120,7 @@ my @bad = ( | @@ -120,6 +120,7 @@ my @bad = ( | ||
| 120 | "stray low surrogate", # 37 | 120 | "stray low surrogate", # 37 |
| 121 | "high high surrogate", # 38 | 121 | "high high surrogate", # 38 |
| 122 | "dangling high surrogate", # 39 | 122 | "dangling high surrogate", # 39 |
| 123 | + "duplicate dictionary key", # 40 | ||
| 123 | ); | 124 | ); |
| 124 | 125 | ||
| 125 | my $i = 0; | 126 | my $i = 0; |
libtests/qtest/json_parse/bad-40.json
0 → 100644
libtests/qtest/json_parse/bad-40.out
0 → 100644
| 1 | +exception: bad-40.json: JSON: offset 28: duplicated dictionary key |