OpenSystemsDevelopment / qpdf

Commit 7b64f219a66de4803d37d1d3bd0ed43ec786e024

Authored by Jay Berkenbilt
Committed by GitHub
2 parents b4c36d9b f099246b

Merge pull request #901 from m-holger/jrrr 

Refactor removal of reserved objects in QPDF::JSONReactor
Inline Side-by-side
Showing 3 changed files with 51 additions and 57 deletions
include/qpdf/JSON.hh
  View file @7b64f21
... ... @@ -54,6 +54,8 @@ class JSON
54 54 {
55 55 public:
56 56 static int constexpr LATEST = 2;
  57 +
  58 + QPDF_DLL
57 59 JSON() = default;
58 60  
59 61 QPDF_DLL
... ... @@ -369,7 +371,7 @@ class JSON
369 371 }
370 372 virtual ~JSON_dictionary() = default;
371 373 virtual void write(Pipeline*, size_t depth) const;
372   - std::map<std::string, std::shared_ptr<JSON_value>> members;
  374 + std::map<std::string, JSON> members;
373 375 std::set<std::string> parsed_keys;
374 376 };
375 377 struct JSON_array: public JSON_value
... ... @@ -380,7 +382,7 @@ class JSON
380 382 }
381 383 virtual ~JSON_array() = default;
382 384 virtual void write(Pipeline*, size_t depth) const;
383   - std::vector<std::shared_ptr<JSON_value>> elements;
  385 + std::vector<JSON> elements;
384 386 };
385 387 struct JSON_string: public JSON_value
386 388 {
... ... @@ -423,7 +425,7 @@ class JSON
423 425 std::function<void(Pipeline*)> fn;
424 426 };
425 427  
426   - JSON(std::shared_ptr<JSON_value>);
  428 + JSON(std::unique_ptr<JSON_value>);
427 429  
428 430 static bool checkSchemaInternal(
429 431 JSON_value* this_v,
... ... @@ -441,13 +443,13 @@ class JSON
441 443 ~Members() = default;
442 444  
443 445 private:
444   - Members(std::shared_ptr<JSON_value>);
  446 + Members(std::unique_ptr<JSON_value>);
445 447 Members(Members const&) = delete;
446 448  
447   - std::shared_ptr<JSON_value> value;
  449 + std::unique_ptr<JSON_value> value;
448 450 // start and end are only populated for objects created by parse
449   - qpdf_offset_t start;
450   - qpdf_offset_t end;
  451 + qpdf_offset_t start{0};
  452 + qpdf_offset_t end{0};
451 453 };
452 454  
453 455 std::shared_ptr<Members> m;
... ...
libqpdf/JSON.cc
  View file @7b64f21
... ... @@ -9,15 +9,13 @@
9 9 #include <cstring>
10 10 #include <stdexcept>
11 11  
12   -JSON::Members::Members(std::shared_ptr<JSON_value> value) :
13   - value(value),
14   - start(0),
15   - end(0)
  12 +JSON::Members::Members(std::unique_ptr<JSON_value> value) :
  13 + value(std::move(value))
16 14 {
17 15 }
18 16  
19   -JSON::JSON(std::shared_ptr<JSON_value> value) :
20   - m(new Members(value))
  17 +JSON::JSON(std::unique_ptr<JSON_value> value) :
  18 + m(new Members(std::move(value)))
21 19 {
22 20 }
23 21  
... ... @@ -278,7 +276,7 @@ JSON::encode_string(std::string const&amp; str)
278 276 JSON
279 277 JSON::makeDictionary()
280 278 {
281   - return JSON(std::make_shared<JSON_dictionary>());
  279 + return JSON(std::make_unique<JSON_dictionary>());
282 280 }
283 281  
284 282 JSON
... ... @@ -286,7 +284,7 @@ JSON::addDictionaryMember(std::string const&amp; key, JSON const&amp; val)
286 284 {
287 285 if (auto* obj = dynamic_cast<JSON_dictionary*>(this->m->value.get())) {
288 286 return obj->members[encode_string(key)] =
289   - val.m->value ? val.m->value : std::make_shared<JSON_null>();
  287 + val.m->value ? val : makeNull();
290 288 } else {
291 289 throw std::runtime_error(
292 290 "JSON::addDictionaryMember called on non-dictionary");
... ... @@ -311,7 +309,7 @@ JSON::checkDictionaryKeySeen(std::string const&amp; key)
311 309 JSON
312 310 JSON::makeArray()
313 311 {
314   - return JSON(std::make_shared<JSON_array>());
  312 + return JSON(std::make_unique<JSON_array>());
315 313 }
316 314  
317 315 JSON
... ... @@ -322,9 +320,9 @@ JSON::addArrayElement(JSON const&amp; val)
322 320 throw std::runtime_error("JSON::addArrayElement called on non-array");
323 321 }
324 322 if (val.m->value.get()) {
325   - arr->elements.push_back(val.m->value);
  323 + arr->elements.push_back(val);
326 324 } else {
327   - arr->elements.push_back(std::make_shared<JSON_null>());
  325 + arr->elements.push_back(makeNull());
328 326 }
329 327 return arr->elements.back();
330 328 }
... ... @@ -332,43 +330,43 @@ JSON::addArrayElement(JSON const&amp; val)
332 330 JSON
333 331 JSON::makeString(std::string const& utf8)
334 332 {
335   - return JSON(std::make_shared<JSON_string>(utf8));
  333 + return JSON(std::make_unique<JSON_string>(utf8));
336 334 }
337 335  
338 336 JSON
339 337 JSON::makeInt(long long int value)
340 338 {
341   - return JSON(std::make_shared<JSON_number>(value));
  339 + return JSON(std::make_unique<JSON_number>(value));
342 340 }
343 341  
344 342 JSON
345 343 JSON::makeReal(double value)
346 344 {
347   - return JSON(std::make_shared<JSON_number>(value));
  345 + return JSON(std::make_unique<JSON_number>(value));
348 346 }
349 347  
350 348 JSON
351 349 JSON::makeNumber(std::string const& encoded)
352 350 {
353   - return JSON(std::make_shared<JSON_number>(encoded));
  351 + return JSON(std::make_unique<JSON_number>(encoded));
354 352 }
355 353  
356 354 JSON
357 355 JSON::makeBool(bool value)
358 356 {
359   - return JSON(std::make_shared<JSON_bool>(value));
  357 + return JSON(std::make_unique<JSON_bool>(value));
360 358 }
361 359  
362 360 JSON
363 361 JSON::makeNull()
364 362 {
365   - return JSON(std::make_shared<JSON_null>());
  363 + return JSON(std::make_unique<JSON_null>());
366 364 }
367 365  
368 366 JSON
369 367 JSON::makeBlob(std::function<void(Pipeline*)> fn)
370 368 {
371   - return JSON(std::make_shared<JSON_blob>(fn));
  369 + return JSON(std::make_unique<JSON_blob>(fn));
372 370 }
373 371  
374 372 bool
... ... @@ -504,11 +502,11 @@ JSON::checkSchemaInternal(
504 502 }
505 503  
506 504 if (sch_dict && (!pattern_key.empty())) {
507   - auto pattern_schema = sch_dict->members[pattern_key].get();
  505 + auto pattern_schema = sch_dict->members[pattern_key].m->value.get();
508 506 for (auto const& iter: this_dict->members) {
509 507 std::string const& key = iter.first;
510 508 checkSchemaInternal(
511   - this_dict->members[key].get(),
  509 + this_dict->members[key].m->value.get(),
512 510 pattern_schema,
513 511 flags,
514 512 errors,
... ... @@ -519,8 +517,8 @@ JSON::checkSchemaInternal(
519 517 std::string const& key = iter.first;
520 518 if (this_dict->members.count(key)) {
521 519 checkSchemaInternal(
522   - this_dict->members[key].get(),
523   - iter.second.get(),
  520 + this_dict->members[key].m->value.get(),
  521 + iter.second.m->value.get(),
524 522 flags,
525 523 errors,
526 524 prefix + "." + key);
... ... @@ -557,8 +555,8 @@ JSON::checkSchemaInternal(
557 555 int i = 0;
558 556 for (auto const& element: this_arr->elements) {
559 557 checkSchemaInternal(
560   - element.get(),
561   - sch_arr->elements.at(0).get(),
  558 + element.m->value.get(),
  559 + sch_arr->elements.at(0).m->value.get(),
562 560 flags,
563 561 errors,
564 562 prefix + "." + std::to_string(i));
... ... @@ -568,7 +566,7 @@ JSON::checkSchemaInternal(
568 566 QTC::TC("libtests", "JSON schema array for single item");
569 567 checkSchemaInternal(
570 568 this_v,
571   - sch_arr->elements.at(0).get(),
  569 + sch_arr->elements.at(0).m->value.get(),
572 570 flags,
573 571 errors,
574 572 prefix);
... ... @@ -587,8 +585,8 @@ JSON::checkSchemaInternal(
587 585 size_t i = 0;
588 586 for (auto const& element: this_arr->elements) {
589 587 checkSchemaInternal(
590   - element.get(),
591   - sch_arr->elements.at(i).get(),
  588 + element.m->value.get(),
  589 + sch_arr->elements.at(i).m->value.get(),
592 590 flags,
593 591 errors,
594 592 prefix + "." + std::to_string(i));
... ...
libqpdf/QPDF_json.cc
  View file @7b64f21
... ... @@ -6,6 +6,7 @@
6 6 #include <qpdf/QIntC.hh>
7 7 #include <qpdf/QPDFObject_private.hh>
8 8 #include <qpdf/QPDFValue.hh>
  9 +#include <qpdf/QPDF_Null.hh>
9 10 #include <qpdf/QTC.hh>
10 11 #include <qpdf/QUtil.hh>
11 12 #include <algorithm>
... ... @@ -234,6 +235,11 @@ class QPDF::JSONReactor: public JSON::Reactor
234 235 descr(std::make_shared<QPDFValue::Description>(QPDFValue::JSON_Descr(
235 236 std::make_shared<std::string>(is->getName()), "")))
236 237 {
  238 + for (auto& oc: pdf.m->obj_cache) {
  239 + if (oc.second.object->getTypeCode() == ::ot_reserved) {
  240 + reserved.insert(oc.first);
  241 + }
  242 + }
237 243 }
238 244 virtual ~JSONReactor() = default;
239 245 virtual void dictionaryStart() override;
... ... @@ -265,7 +271,6 @@ class QPDF::JSONReactor: public JSON::Reactor
265 271 void setObjectDescription(QPDFObjectHandle& oh, JSON const& value);
266 272 QPDFObjectHandle makeObject(JSON const& value);
267 273 void error(qpdf_offset_t offset, std::string const& message);
268   - QPDFObjectHandle reserveObject(int obj, int gen);
269 274 void replaceObject(
270 275 QPDFObjectHandle to_replace,
271 276 QPDFObjectHandle replacement,
... ... @@ -416,27 +421,17 @@ QPDF::JSONReactor::containerEnd(JSON const&amp; value)
416 421 object_stack.pop_back();
417 422 }
418 423 } else if ((state == st_top) && (from_state == st_qpdf)) {
419   - for (auto const& og: this->reserved) {
420   - // Handle dangling indirect object references which the
421   - // PDF spec says to treat as nulls. It's tempting to make
422   - // this an error, but that would be wrong since valid
423   - // input files may have these.
424   - QTC::TC("qpdf", "QPDF_json non-trivial null reserved");
425   - this->pdf.replaceObject(og, QPDFObjectHandle::newNull());
  424 + // Handle dangling indirect object references which the PDF spec says to
  425 + // treat as nulls. It's tempting to make this an error, but that would
  426 + // be wrong since valid input files may have these.
  427 + for (auto& oc: pdf.m->obj_cache) {
  428 + if (oc.second.object->getTypeCode() == ::ot_reserved &&
  429 + reserved.count(oc.first) == 0) {
  430 + QTC::TC("qpdf", "QPDF_json non-trivial null reserved");
  431 + pdf.updateCache(oc.first, QPDF_Null::create(), -1, -1);
  432 + }
426 433 }
427   - this->reserved.clear();
428   - }
429   -}
430   -
431   -QPDFObjectHandle
432   -QPDF::JSONReactor::reserveObject(int obj, int gen)
433   -{
434   - QPDFObjGen og(obj, gen);
435   - auto oh = pdf.reserveObjectIfNotExists(og);
436   - if (oh.isReserved()) {
437   - this->reserved.insert(og);
438 434 }
439   - return oh;
440 435 }
441 436  
442 437 void
... ... @@ -446,7 +441,6 @@ QPDF::JSONReactor::replaceObject(
446 441 JSON const& value)
447 442 {
448 443 auto og = to_replace.getObjGen();
449   - this->reserved.erase(og);
450 444 this->pdf.replaceObject(og, replacement);
451 445 auto oh = pdf.getObject(og);
452 446 setObjectDescription(oh, value);
... ... @@ -564,7 +558,7 @@ QPDF::JSONReactor::dictionaryItem(std::string const&amp; key, JSON const&amp; value)
564 558 this->cur_object = "trailer";
565 559 } else if (is_obj_key(key, obj, gen)) {
566 560 this->cur_object = key;
567   - auto oh = reserveObject(obj, gen);
  561 + auto oh = pdf.reserveObjectIfNotExists(QPDFObjGen(obj, gen));
568 562 object_stack.push_back(oh);
569 563 nestedState(key, value, st_object_top);
570 564 } else {
... ... @@ -763,7 +757,7 @@ QPDF::JSONReactor::makeObject(JSON const&amp; value)
763 757 int gen = 0;
764 758 std::string str;
765 759 if (is_indirect_object(str_v, obj, gen)) {
766   - result = reserveObject(obj, gen);
  760 + result = pdf.reserveObjectIfNotExists(QPDFObjGen(obj, gen));
767 761 } else if (is_unicode_string(str_v, str)) {
768 762 result = QPDFObjectHandle::newUnicodeString(str);
769 763 } else if (is_binary_string(str_v, str)) {
... ...