Commit 5aa8225f493dc3c3171662fecc8a9ff5d0a16feb
Committed by
Jay Berkenbilt
1 parent
0c7c7e4b
Refactor QPDFObjectTypeAccessor and QPDFObjectHandle::dereference
Showing
2 changed files
with
56 additions
and
112 deletions
include/qpdf/QPDFObjectHandle.hh
| ... | ... | @@ -1464,7 +1464,10 @@ class QPDFObjectHandle |
| 1464 | 1464 | static std::shared_ptr<QPDFObject> |
| 1465 | 1465 | getObject(QPDFObjectHandle& o) |
| 1466 | 1466 | { |
| 1467 | - o.dereference(); | |
| 1467 | + if (!o.dereference()) { | |
| 1468 | + throw std::logic_error("attempted to dereference an" | |
| 1469 | + " uninitialized QPDFObjectHandle"); | |
| 1470 | + }; | |
| 1468 | 1471 | return o.obj; |
| 1469 | 1472 | } |
| 1470 | 1473 | }; |
| ... | ... | @@ -1573,7 +1576,7 @@ class QPDFObjectHandle |
| 1573 | 1576 | void typeWarning(char const* expected_type, std::string const& warning); |
| 1574 | 1577 | void objectWarning(std::string const& warning); |
| 1575 | 1578 | void assertType(char const* type_name, bool istype); |
| 1576 | - void dereference(); | |
| 1579 | + bool dereference(); | |
| 1577 | 1580 | void copyObject( |
| 1578 | 1581 | std::set<QPDFObjGen>& visited, |
| 1579 | 1582 | bool cross_indirect, | ... | ... |
libqpdf/QPDFObjectHandle.cc
| ... | ... | @@ -284,23 +284,14 @@ QPDFObjectHandle::isInitialized() const |
| 284 | 284 | QPDFObject::object_type_e |
| 285 | 285 | QPDFObjectHandle::getTypeCode() |
| 286 | 286 | { |
| 287 | - if (this->initialized) { | |
| 288 | - dereference(); | |
| 289 | - return this->obj->getTypeCode(); | |
| 290 | - } else { | |
| 291 | - return QPDFObject::ot_uninitialized; | |
| 292 | - } | |
| 287 | + return dereference() ? | |
| 288 | + this->obj->getTypeCode() : QPDFObject::ot_uninitialized; | |
| 293 | 289 | } |
| 294 | 290 | |
| 295 | 291 | char const* |
| 296 | 292 | QPDFObjectHandle::getTypeName() |
| 297 | 293 | { |
| 298 | - if (this->initialized) { | |
| 299 | - dereference(); | |
| 300 | - return this->obj->getTypeName(); | |
| 301 | - } else { | |
| 302 | - return "uninitialized"; | |
| 303 | - } | |
| 294 | + return dereference() ? this->obj->getTypeName() : "uninitialized"; | |
| 304 | 295 | } |
| 305 | 296 | |
| 306 | 297 | namespace |
| ... | ... | @@ -310,14 +301,9 @@ namespace |
| 310 | 301 | { |
| 311 | 302 | public: |
| 312 | 303 | static bool |
| 313 | - check(QPDFObject* o) | |
| 304 | + check(std::shared_ptr<QPDFObject> const& o) | |
| 314 | 305 | { |
| 315 | - return (o && dynamic_cast<T*>(o)); | |
| 316 | - } | |
| 317 | - static bool | |
| 318 | - check(QPDFObject const* o) | |
| 319 | - { | |
| 320 | - return (o && dynamic_cast<T const*>(o)); | |
| 306 | + return (o && dynamic_cast<T const*>(o.get())); | |
| 321 | 307 | } |
| 322 | 308 | }; |
| 323 | 309 | } // namespace |
| ... | ... | @@ -325,11 +311,7 @@ namespace |
| 325 | 311 | bool |
| 326 | 312 | QPDFObjectHandle::isBool() |
| 327 | 313 | { |
| 328 | - if (!this->initialized) { | |
| 329 | - return false; | |
| 330 | - } | |
| 331 | - dereference(); | |
| 332 | - return QPDFObjectTypeAccessor<QPDF_Bool>::check(obj.get()); | |
| 314 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Bool>::check(obj); | |
| 333 | 315 | } |
| 334 | 316 | |
| 335 | 317 | bool |
| ... | ... | @@ -339,37 +321,25 @@ QPDFObjectHandle::isDirectNull() const |
| 339 | 321 | // objid == 0, so there's nothing to resolve. |
| 340 | 322 | return ( |
| 341 | 323 | this->initialized && (getObjectID() == 0) && |
| 342 | - QPDFObjectTypeAccessor<QPDF_Null>::check(obj.get())); | |
| 324 | + QPDFObjectTypeAccessor<QPDF_Null>::check(obj)); | |
| 343 | 325 | } |
| 344 | 326 | |
| 345 | 327 | bool |
| 346 | 328 | QPDFObjectHandle::isNull() |
| 347 | 329 | { |
| 348 | - if (!this->initialized) { | |
| 349 | - return false; | |
| 350 | - } | |
| 351 | - dereference(); | |
| 352 | - return QPDFObjectTypeAccessor<QPDF_Null>::check(obj.get()); | |
| 330 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Null>::check(obj); | |
| 353 | 331 | } |
| 354 | 332 | |
| 355 | 333 | bool |
| 356 | 334 | QPDFObjectHandle::isInteger() |
| 357 | 335 | { |
| 358 | - if (!this->initialized) { | |
| 359 | - return false; | |
| 360 | - } | |
| 361 | - dereference(); | |
| 362 | - return QPDFObjectTypeAccessor<QPDF_Integer>::check(obj.get()); | |
| 336 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Integer>::check(obj); | |
| 363 | 337 | } |
| 364 | 338 | |
| 365 | 339 | bool |
| 366 | 340 | QPDFObjectHandle::isReal() |
| 367 | 341 | { |
| 368 | - if (!this->initialized) { | |
| 369 | - return false; | |
| 370 | - } | |
| 371 | - dereference(); | |
| 372 | - return QPDFObjectTypeAccessor<QPDF_Real>::check(obj.get()); | |
| 342 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Real>::check(obj); | |
| 373 | 343 | } |
| 374 | 344 | |
| 375 | 345 | bool |
| ... | ... | @@ -406,91 +376,58 @@ QPDFObjectHandle::getValueAsNumber(double& value) |
| 406 | 376 | bool |
| 407 | 377 | QPDFObjectHandle::isName() |
| 408 | 378 | { |
| 409 | - if (!this->initialized) { | |
| 410 | - return false; | |
| 411 | - } | |
| 412 | - dereference(); | |
| 413 | - return QPDFObjectTypeAccessor<QPDF_Name>::check(obj.get()); | |
| 379 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Name>::check(obj); | |
| 414 | 380 | } |
| 415 | 381 | |
| 416 | 382 | bool |
| 417 | 383 | QPDFObjectHandle::isString() |
| 418 | 384 | { |
| 419 | - if (!this->initialized) { | |
| 420 | - return false; | |
| 421 | - } | |
| 422 | - dereference(); | |
| 423 | - return QPDFObjectTypeAccessor<QPDF_String>::check(obj.get()); | |
| 385 | + return dereference() && QPDFObjectTypeAccessor<QPDF_String>::check(obj); | |
| 424 | 386 | } |
| 425 | 387 | |
| 426 | 388 | bool |
| 427 | 389 | QPDFObjectHandle::isOperator() |
| 428 | 390 | { |
| 429 | - if (!this->initialized) { | |
| 430 | - return false; | |
| 431 | - } | |
| 432 | - dereference(); | |
| 433 | - return QPDFObjectTypeAccessor<QPDF_Operator>::check(obj.get()); | |
| 391 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Operator>::check(obj); | |
| 434 | 392 | } |
| 435 | 393 | |
| 436 | 394 | bool |
| 437 | 395 | QPDFObjectHandle::isInlineImage() |
| 438 | 396 | { |
| 439 | - if (!this->initialized) { | |
| 440 | - return false; | |
| 441 | - } | |
| 442 | - dereference(); | |
| 443 | - return QPDFObjectTypeAccessor<QPDF_InlineImage>::check(obj.get()); | |
| 397 | + return dereference() && | |
| 398 | + QPDFObjectTypeAccessor<QPDF_InlineImage>::check(obj); | |
| 444 | 399 | } |
| 445 | 400 | |
| 446 | 401 | bool |
| 447 | 402 | QPDFObjectHandle::isArray() |
| 448 | 403 | { |
| 449 | - if (!this->initialized) { | |
| 450 | - return false; | |
| 451 | - } | |
| 452 | - dereference(); | |
| 453 | - return QPDFObjectTypeAccessor<QPDF_Array>::check(obj.get()); | |
| 404 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Array>::check(obj); | |
| 454 | 405 | } |
| 455 | 406 | |
| 456 | 407 | bool |
| 457 | 408 | QPDFObjectHandle::isDictionary() |
| 458 | 409 | { |
| 459 | - if (!this->initialized) { | |
| 460 | - return false; | |
| 461 | - } | |
| 462 | - dereference(); | |
| 463 | - return QPDFObjectTypeAccessor<QPDF_Dictionary>::check(obj.get()); | |
| 410 | + return dereference() && | |
| 411 | + QPDFObjectTypeAccessor<QPDF_Dictionary>::check(obj); | |
| 464 | 412 | } |
| 465 | 413 | |
| 466 | 414 | bool |
| 467 | 415 | QPDFObjectHandle::isStream() |
| 468 | 416 | { |
| 469 | - if (!this->initialized) { | |
| 470 | - return false; | |
| 471 | - } | |
| 472 | - dereference(); | |
| 473 | - return QPDFObjectTypeAccessor<QPDF_Stream>::check(obj.get()); | |
| 417 | + return dereference() && QPDFObjectTypeAccessor<QPDF_Stream>::check(obj); | |
| 474 | 418 | } |
| 475 | 419 | |
| 476 | 420 | bool |
| 477 | 421 | QPDFObjectHandle::isReserved() |
| 478 | 422 | { |
| 479 | - if (!this->initialized) { | |
| 480 | - return false; | |
| 481 | - } | |
| 482 | 423 | // dereference will clear reserved if this has been replaced |
| 483 | - dereference(); | |
| 484 | - return this->reserved; | |
| 424 | + return dereference() && this->reserved; | |
| 485 | 425 | } |
| 486 | 426 | |
| 487 | 427 | bool |
| 488 | 428 | QPDFObjectHandle::isIndirect() |
| 489 | 429 | { |
| 490 | - if (!this->initialized) { | |
| 491 | - return false; | |
| 492 | - } | |
| 493 | - return (getObjectID() != 0); | |
| 430 | + return this->initialized && (getObjectID() != 0); | |
| 494 | 431 | } |
| 495 | 432 | |
| 496 | 433 | bool |
| ... | ... | @@ -1748,8 +1685,10 @@ QPDFObjectHandle::unparse() |
| 1748 | 1685 | std::string |
| 1749 | 1686 | QPDFObjectHandle::unparseResolved() |
| 1750 | 1687 | { |
| 1751 | - dereference(); | |
| 1752 | - if (this->reserved) { | |
| 1688 | + if (!dereference()) { | |
| 1689 | + throw std::logic_error( | |
| 1690 | + "attempted to dereference an uninitialized QPDFObjectHandle"); | |
| 1691 | + } else if (this->reserved) { | |
| 1753 | 1692 | throw std::logic_error( |
| 1754 | 1693 | "QPDFObjectHandle: attempting to unparse a reserved object"); |
| 1755 | 1694 | } |
| ... | ... | @@ -1778,12 +1717,13 @@ QPDFObjectHandle::getJSON(int json_version, bool dereference_indirect) |
| 1778 | 1717 | { |
| 1779 | 1718 | if ((!dereference_indirect) && this->isIndirect()) { |
| 1780 | 1719 | return JSON::makeString(unparse()); |
| 1720 | + } else if (!dereference()) { | |
| 1721 | + throw std::logic_error( | |
| 1722 | + "attempted to dereference an uninitialized QPDFObjectHandle"); | |
| 1723 | + } else if (this->reserved) { | |
| 1724 | + throw std::logic_error( | |
| 1725 | + "QPDFObjectHandle: attempting to unparse a reserved object"); | |
| 1781 | 1726 | } else { |
| 1782 | - dereference(); | |
| 1783 | - if (this->reserved) { | |
| 1784 | - throw std::logic_error( | |
| 1785 | - "QPDFObjectHandle: attempting to unparse a reserved object"); | |
| 1786 | - } | |
| 1787 | 1727 | return this->obj->getJSON(json_version); |
| 1788 | 1728 | } |
| 1789 | 1729 | } |
| ... | ... | @@ -2524,8 +2464,11 @@ QPDFObjectHandle::parseInternal( |
| 2524 | 2464 | qpdf_offset_t |
| 2525 | 2465 | QPDFObjectHandle::getParsedOffset() |
| 2526 | 2466 | { |
| 2527 | - dereference(); | |
| 2528 | - return this->obj->getParsedOffset(); | |
| 2467 | + if (dereference()) { | |
| 2468 | + return this->obj->getParsedOffset(); | |
| 2469 | + } else { | |
| 2470 | + return -1; | |
| 2471 | + } | |
| 2529 | 2472 | } |
| 2530 | 2473 | |
| 2531 | 2474 | void |
| ... | ... | @@ -2723,6 +2666,7 @@ QPDFObjectHandle::newStream(QPDF* qpdf) |
| 2723 | 2666 | QPDFObjectHandle stream_dict = newDictionary(); |
| 2724 | 2667 | QPDFObjectHandle result = qpdf->makeIndirectObject( |
| 2725 | 2668 | QPDFObjectHandle(new QPDF_Stream(qpdf, 0, 0, stream_dict, 0, 0))); |
| 2669 | + // Indirect objects are guaranteed to be initialized | |
| 2726 | 2670 | result.dereference(); |
| 2727 | 2671 | QPDF_Stream* stream = dynamic_cast<QPDF_Stream*>(result.obj.get()); |
| 2728 | 2672 | stream->setObjGen(result.getObjectID(), result.getGeneration()); |
| ... | ... | @@ -2771,21 +2715,15 @@ QPDFObjectHandle::setObjectDescription( |
| 2771 | 2715 | { |
| 2772 | 2716 | // This is called during parsing on newly created direct objects, |
| 2773 | 2717 | // so we can't call dereference() here. |
| 2774 | - if (isInitialized() && this->obj.get()) { | |
| 2775 | - this->obj->setDescription(owning_qpdf, object_description); | |
| 2718 | + if (isInitialized() && obj.get()) { | |
| 2719 | + obj->setDescription(owning_qpdf, object_description); | |
| 2776 | 2720 | } |
| 2777 | 2721 | } |
| 2778 | 2722 | |
| 2779 | 2723 | bool |
| 2780 | 2724 | QPDFObjectHandle::hasObjectDescription() |
| 2781 | 2725 | { |
| 2782 | - if (isInitialized()) { | |
| 2783 | - dereference(); | |
| 2784 | - if (this->obj.get()) { | |
| 2785 | - return this->obj->hasDescription(); | |
| 2786 | - } | |
| 2787 | - } | |
| 2788 | - return false; | |
| 2726 | + return dereference() && obj.get() && obj->hasDescription(); | |
| 2789 | 2727 | } |
| 2790 | 2728 | |
| 2791 | 2729 | QPDFObjectHandle |
| ... | ... | @@ -2868,7 +2806,6 @@ QPDFObjectHandle::copyObject( |
| 2868 | 2806 | " reserved object handle direct"); |
| 2869 | 2807 | } |
| 2870 | 2808 | |
| 2871 | - dereference(); | |
| 2872 | 2809 | this->qpdf = 0; |
| 2873 | 2810 | this->objid = 0; |
| 2874 | 2811 | this->generation = 0; |
| ... | ... | @@ -2971,7 +2908,12 @@ QPDFObjectHandle::typeWarning( |
| 2971 | 2908 | { |
| 2972 | 2909 | QPDF* context = nullptr; |
| 2973 | 2910 | std::string description; |
| 2974 | - dereference(); | |
| 2911 | + // Type checks above guarantee that the object has been dereferenced. | |
| 2912 | + // Nevertheless, dereference throws exceptions in the test suite | |
| 2913 | + if (!dereference()) { | |
| 2914 | + throw std::logic_error( | |
| 2915 | + "attempted to dereference an uninitialized QPDFObjectHandle"); | |
| 2916 | + } | |
| 2975 | 2917 | this->obj->getDescription(context, description); |
| 2976 | 2918 | // Null context handled by warn |
| 2977 | 2919 | warn( |
| ... | ... | @@ -2991,8 +2933,7 @@ QPDFObjectHandle::warnIfPossible(std::string const& warning) |
| 2991 | 2933 | { |
| 2992 | 2934 | QPDF* context = 0; |
| 2993 | 2935 | std::string description; |
| 2994 | - dereference(); | |
| 2995 | - if (this->obj->getDescription(context, description)) { | |
| 2936 | + if (dereference() && obj->getDescription(context, description)) { | |
| 2996 | 2937 | warn(context, QPDFExc(qpdf_e_damaged_pdf, "", description, 0, warning)); |
| 2997 | 2938 | } else { |
| 2998 | 2939 | *QPDFLogger::defaultLogger()->getError() << warning << "\n"; |
| ... | ... | @@ -3004,7 +2945,7 @@ QPDFObjectHandle::objectWarning(std::string const& warning) |
| 3004 | 2945 | { |
| 3005 | 2946 | QPDF* context = nullptr; |
| 3006 | 2947 | std::string description; |
| 3007 | - dereference(); | |
| 2948 | + // Type checks above guarantee that the object has been dereferenced. | |
| 3008 | 2949 | this->obj->getDescription(context, description); |
| 3009 | 2950 | // Null context handled by warn |
| 3010 | 2951 | warn(context, QPDFExc(qpdf_e_object, "", description, 0, warning)); |
| ... | ... | @@ -3187,12 +3128,11 @@ QPDFObjectHandle::assertPageObject() |
| 3187 | 3128 | } |
| 3188 | 3129 | } |
| 3189 | 3130 | |
| 3190 | -void | |
| 3131 | +bool | |
| 3191 | 3132 | QPDFObjectHandle::dereference() |
| 3192 | 3133 | { |
| 3193 | 3134 | if (!this->initialized) { |
| 3194 | - throw std::logic_error( | |
| 3195 | - "attempted to dereference an uninitialized QPDFObjectHandle"); | |
| 3135 | + return false; | |
| 3196 | 3136 | } |
| 3197 | 3137 | if (this->obj.get() && getObjectID() && |
| 3198 | 3138 | QPDF::Resolver::objectChanged(this->qpdf, getObjGen(), this->obj)) { |
| ... | ... | @@ -3213,6 +3153,7 @@ QPDFObjectHandle::dereference() |
| 3213 | 3153 | this->obj = obj; |
| 3214 | 3154 | } |
| 3215 | 3155 | } |
| 3156 | + return true; | |
| 3216 | 3157 | } |
| 3217 | 3158 | |
| 3218 | 3159 | void | ... | ... |