diff --git a/include/qpdf/ObjectHandle.hh b/include/qpdf/ObjectHandle.hh index 3e8f84d..02b6674 100644 --- a/include/qpdf/ObjectHandle.hh +++ b/include/qpdf/ObjectHandle.hh @@ -83,8 +83,11 @@ namespace qpdf QPDFObjectHandle operator[](size_t n) const; QPDFObjectHandle operator[](int n) const; + QPDFObjectHandle& at(std::string const& key) const; bool contains(std::string const& key) const; size_t erase(std::string const& key); + QPDFObjectHandle& find(std::string const& key) const; + bool replace(std::string const& key, QPDFObjectHandle value); QPDFObjectHandle const& operator[](std::string const& key) const; std::shared_ptr copy(bool shallow = false) const; @@ -134,6 +137,8 @@ namespace qpdf std::string description() const; + inline QPDFObjectHandle const& get(std::string const& key) const; + void no_ci_warn_if(bool condition, std::string const& warning) const; void no_ci_stop_if(bool condition, std::string const& warning) const; void no_ci_stop_damaged_if(bool condition, std::string const& warning) const; diff --git a/libqpdf/NNTree.cc b/libqpdf/NNTree.cc index 42a5cdb..170bf5c 100644 --- a/libqpdf/NNTree.cc +++ b/libqpdf/NNTree.cc @@ -168,7 +168,7 @@ NNTreeIterator::resetLimits(Dictionary a_node, std::list::iterator } } if (a_node != path.begin()->node) { - a_node.replaceKey("/Limits", Array({first, last})); + a_node.replace("/Limits", Array({first, last})); } } @@ -266,7 +266,7 @@ NNTreeIterator::split(Dictionary to_split, std::list::iterator pare new_kids.push_back(first_node); to_split.erase("/Limits"); // already shouldn't be there for root to_split.erase(impl.itemsKey()); - to_split.replaceKey("/Kids", new_kids); + to_split.replace("/Kids", new_kids); if (is_leaf) { node = first_node; } else { @@ -446,7 +446,7 @@ NNTreeIterator::remove() if (parent == path.end()) { // We erased the very last item. Convert the root to an empty items array. element->node.erase("/Kids"); - element->node.replaceKey(impl.itemsKey(), Array::empty()); + element->node.replace(impl.itemsKey(), Array::empty()); path.clear(); setItemNumber(impl.tree_root, -1); return; @@ -673,8 +673,8 @@ NNTreeImpl::repair() for (auto const& [key, value]: items) { repl.insert(key, value); } - tree_root.replaceKey("/Kids", new_node["/Kids"]); - tree_root.replaceKey(itemsKey(), new_node[itemsKey()]); + tree_root.replace("/Kids", new_node["/Kids"]); + tree_root.replace(itemsKey(), new_node[itemsKey()]); } NNTreeImpl::iterator diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 424a9f1..3dabac0 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -572,7 +572,7 @@ Objects::Foreign::Copier::replace_indirect_object(QPDFObjectHandle const& foreig auto result = Dictionary::empty(); for (auto const& [key, value]: Dictionary(foreign)) { if (!value.null()) { - result.replaceKey(key, replace_indirect_object(value)); + result.replace(key, replace_indirect_object(value)); } } return result; @@ -584,7 +584,7 @@ Objects::Foreign::Copier::replace_indirect_object(QPDFObjectHandle const& foreig auto dict = result.getDict(); for (auto const& [key, value]: stream.getDict()) { if (!value.null()) { - dict.replaceKey(key, replace_indirect_object(value)); + dict.replace(key, replace_indirect_object(value)); } } stream.copy_data_to(result); @@ -658,7 +658,7 @@ QPDF::getRoot() // approach to more extensive checks and warning levels. if (m->cf.check_mode() && Name(Root["/Type"]) != "/Catalog") { warn(m->c.damagedPDF("", -1, "catalog /Type entry missing or invalid")); - Root.replaceKey("/Type", Name("/Catalog")); + Root.replace("/Type", Name("/Catalog")); } return Root.oh(); } diff --git a/libqpdf/QPDFAcroFormDocumentHelper.cc b/libqpdf/QPDFAcroFormDocumentHelper.cc index 4fb3ec3..1dbc229 100644 --- a/libqpdf/QPDFAcroFormDocumentHelper.cc +++ b/libqpdf/QPDFAcroFormDocumentHelper.cc @@ -799,7 +799,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations( } QPDFObjectHandle(dr).makeResourcesIndirect(qpdf); if (!dr.indirect()) { - acroform.replaceKey("/DR", qpdf.makeIndirectObject(dr)); + acroform.replace("/DR", qpdf.makeIndirectObject(dr)); dr = acroform["/DR"]; } // Merge the other document's /DR, creating a conflict map. mergeResources checks to @@ -839,7 +839,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations( if (parent.indirect()) { auto parent_og = parent.id_gen(); if (orig_to_copy.contains(parent_og)) { - obj.replaceKey("/Parent", orig_to_copy[parent_og]); + obj.replace("/Parent", orig_to_copy[parent_og]); } else { parent.warn( "while traversing field " + obj.id_gen().unparse(',') + @@ -869,7 +869,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations( // chrome, firefox, the mac Preview application, and several of the free // readers on Linux all ignore /DR at the field level. if (obj.contains("/DR")) { - obj.replaceKey("/DR", dr); + obj.replace("/DR", dr); } if (obj["/DA"].isString() && !dr_map.empty()) { adjustDefaultAppearances(obj, dr_map); @@ -988,7 +988,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations( Dictionary apdict = ah.getAppearanceDictionary(); std::vector streams; auto replace_stream = [](auto& dict, auto& key, auto& old) { - dict.replaceKey(key, old.copyStream()); + dict.replace(key, old.copyStream()); return dict[key]; }; @@ -1015,7 +1015,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations( } apcm.concat(cm); if (omatrix || apcm != QPDFMatrix()) { - dict.replaceKey("/Matrix", QPDFObjectHandle::newFromMatrix(apcm)); + dict.replace("/Matrix", QPDFObjectHandle::newFromMatrix(apcm)); } Dictionary resources = dict["/Resources"]; if (!dr_map.empty() && resources) { diff --git a/libqpdf/QPDFAnnotationObjectHelper.cc b/libqpdf/QPDFAnnotationObjectHelper.cc index 63e3628..9467ffd 100644 --- a/libqpdf/QPDFAnnotationObjectHelper.cc +++ b/libqpdf/QPDFAnnotationObjectHelper.cc @@ -33,14 +33,14 @@ QPDFAnnotationObjectHelper::getAppearanceDictionary() std::string QPDFAnnotationObjectHelper::getAppearanceState() { - Name AS = (*this)["/AS"]; + Name AS = get("/AS"); return AS ? AS.value() : ""; } int QPDFAnnotationObjectHelper::getFlags() { - Integer flags_obj = (*this)["/F"]; + Integer flags_obj = get("/F"); return flags_obj ? flags_obj : 0; } diff --git a/libqpdf/QPDFEFStreamObjectHelper.cc b/libqpdf/QPDFEFStreamObjectHelper.cc index 61c929c..c74fe21 100644 --- a/libqpdf/QPDFEFStreamObjectHelper.cc +++ b/libqpdf/QPDFEFStreamObjectHelper.cc @@ -31,7 +31,7 @@ void QPDFEFStreamObjectHelper::setParam(std::string const& pkey, QPDFObjectHandle const& pval) { if (Dictionary Params = oh().getDict()["/Params"]) { - Params.replaceKey(pkey, pval); + Params.replace(pkey, pval); return; } oh().getDict().replaceKey("/Params", Dictionary({{pkey, pval}})); diff --git a/libqpdf/QPDFFileSpecObjectHelper.cc b/libqpdf/QPDFFileSpecObjectHelper.cc index 570b056..09c8df3 100644 --- a/libqpdf/QPDFFileSpecObjectHelper.cc +++ b/libqpdf/QPDFFileSpecObjectHelper.cc @@ -42,7 +42,7 @@ std::string QPDFFileSpecObjectHelper::getFilename() { for (auto const& i: name_keys) { - if (String k = oh()[i]) { + if (String k = get(i)) { return k.utf8_value(); } } @@ -54,7 +54,7 @@ QPDFFileSpecObjectHelper::getFilenames() { std::map result; for (auto const& i: name_keys) { - if (String k = oh()[i]) { + if (String k = get(i)) { result[i] = k.utf8_value(); } } @@ -64,7 +64,7 @@ QPDFFileSpecObjectHelper::getFilenames() QPDFObjectHandle QPDFFileSpecObjectHelper::getEmbeddedFileStream(std::string const& key) { - if (Dictionary EF = oh()["/EF"]) { + if (Dictionary EF = get("/EF")) { if (!key.empty() && EF.contains(key)) { if (auto result = EF[key]) { return result; diff --git a/libqpdf/QPDFFormFieldObjectHelper.cc b/libqpdf/QPDFFormFieldObjectHelper.cc index 11efeef..54f854e 100644 --- a/libqpdf/QPDFFormFieldObjectHelper.cc +++ b/libqpdf/QPDFFormFieldObjectHelper.cc @@ -956,7 +956,7 @@ FormField::generateTextAppearance(QPDFAnnotationObjectHelper& aoh) aoh.getObjectHandle().replaceKey("/AP", Dictionary::empty()); AP = aoh.getAppearanceDictionary(); } - AP.replaceKey("/N", AS); + AP.replace("/N", AS); } if (!AS.isStream()) { aoh.warn("unable to get normal appearance stream for update"); diff --git a/libqpdf/QPDFOutlineObjectHelper.cc b/libqpdf/QPDFOutlineObjectHelper.cc index 3a163b6..f098804 100644 --- a/libqpdf/QPDFOutlineObjectHelper.cc +++ b/libqpdf/QPDFOutlineObjectHelper.cc @@ -53,9 +53,9 @@ QPDFOutlineObjectHelper::getKids() QPDFObjectHandle QPDFOutlineObjectHelper::getDest() { - auto dest = (*this)["/Dest"]; + auto dest = get("/Dest"); if (dest.null()) { - auto const& A = (*this)["/A"]; + auto const& A = get("/A"); if (Name(A["/S"]) == "/GoTo") { dest = A["/D"]; } diff --git a/libqpdf/QPDF_Dictionary.cc b/libqpdf/QPDF_Dictionary.cc index f606b2b..b9c9d9d 100644 --- a/libqpdf/QPDF_Dictionary.cc +++ b/libqpdf/QPDF_Dictionary.cc @@ -2,6 +2,7 @@ #include #include +#include using namespace std::literals; using namespace qpdf; @@ -29,12 +30,70 @@ BaseHandle::operator[](std::string const& key) const return null_obj; } +/// Retrieves a reference to the QPDFObjectHandle associated with the given key in the +/// dictionary object contained within this instance. +/// +/// If the current object is not of dictionary type, a `std::runtime_error` is thrown. +/// According to the PDF specification, missing keys in the dictionary are treated as +/// keys with a `null` value. This behavior is reflected in this function's implementation, +/// where a missing key will still return a reference to a newly inserted null value entry. +/// +/// @param key The key for which the corresponding value in the dictionary is retrieved. +/// @return A reference to the QPDFObjectHandle associated with the specified key. +/// @throws std::runtime_error if the current object is not a dictionary. +QPDFObjectHandle& +BaseHandle::at(std::string const& key) const +{ + auto d = as(); + if (!d) { + throw std::runtime_error("Expected a dictionary but found a non-dictionary object"); + } + return d->items[key]; +} + +/// @brief Checks if the specified key exists in the object. +/// +/// This method determines whether the given key is present in the object by verifying if the +/// associated value is non-null. +/// +/// @param key The key to look for in the object. +/// @return True if the key exists and its associated value is non-null. Otherwise, returns false. bool BaseHandle::contains(std::string const& key) const { return !(*this)[key].null(); } +/// @brief Retrieves the value associated with the given key from dictionary. +/// +/// This method attempts to find the value corresponding to the specified key for objects that can +/// be interpreted as dictionaries. +/// +/// - If the object is a dictionary and the specified key exists, it returns a reference +/// to the associated value. +/// - If the object is not a dictionary or the specified key does not exist, it returns +/// a reference to a static uninitialized object handle. +/// +/// @note Modifying the uninitialized object returned when the key is not found is strictly +/// prohibited. +/// +/// @param key The key whose associated value should be retrieved. +/// @return A reference to the associated value if the key is found or a reference to a static +/// uninitialized object if the key is not found. +QPDFObjectHandle& +BaseHandle::find(std::string const& key) const +{ + static const QPDFObjectHandle null_obj; + qpdf_invariant(!null_obj); + if (auto d = as()) { + auto it = d->items.find(key); + if (it != d->items.end()) { + return it->second; + } + } + return const_cast(null_obj); +} + std::set BaseDictionary::getKeys() { @@ -63,18 +122,29 @@ BaseHandle::erase(const std::string& key) return 0; } +bool +BaseHandle::replace(std::string const& key, QPDFObjectHandle value) +{ + if (auto d = as()) { + if (value.null() && !value.indirect()) { + // The PDF spec doesn't distinguish between keys with null values and missing keys. + // Allow indirect nulls which are equivalent to a dangling reference, which is permitted + // by the spec. + d->items.erase(key); + } else { + // add or replace value + d->items[key] = value; + } + return true; + } + return false; +} + void -BaseDictionary::replaceKey(std::string const& key, QPDFObjectHandle value) -{ - auto d = dict(); - if (value.null() && !value.indirect()) { - // The PDF spec doesn't distinguish between keys with null values and missing keys. - // Allow indirect nulls which are equivalent to a dangling reference, which is - // permitted by the spec. - d->items.erase(key); - } else { - // add or replace value - d->items[key] = value; +BaseDictionary::replace(std::string const& key, QPDFObjectHandle value) +{ + if (!BaseHandle::replace(key, value)) { + (void)dict(); } } @@ -121,7 +191,7 @@ QPDFObjectHandle::hasKey(std::string const& key) const QPDFObjectHandle QPDFObjectHandle::getKey(std::string const& key) const { - if (auto result = (*this)[key]) { + if (auto result = get(key)) { return result; } if (isDictionary()) { @@ -166,11 +236,10 @@ QPDFObjectHandle::replaceKey(std::string const& key, QPDFObjectHandle const& val { if (auto dict = as_dictionary(strict)) { checkOwnership(value); - dict.replaceKey(key, value); + dict.replace(key, value); return; } typeWarning("dictionary", "ignoring key replacement request"); - QTC::TC("qpdf", "QPDFObjectHandle dictionary ignoring replaceKey"); } QPDFObjectHandle @@ -200,7 +269,7 @@ QPDFObjectHandle::removeKey(std::string const& key) QPDFObjectHandle QPDFObjectHandle::removeKeyAndGetOld(std::string const& key) { - auto result = (*this)[key]; + auto result = get(key); erase(key); return result ? result : newNull(); } diff --git a/libqpdf/qpdf/QPDFObjectHandle_private.hh b/libqpdf/qpdf/QPDFObjectHandle_private.hh index 3f32915..cc30057 100644 --- a/libqpdf/qpdf/QPDFObjectHandle_private.hh +++ b/libqpdf/qpdf/QPDFObjectHandle_private.hh @@ -127,7 +127,7 @@ namespace qpdf // The following methods are not part of the public API. std::set getKeys(); std::map const& getAsMap() const; - void replaceKey(std::string const& key, QPDFObjectHandle value); + void replace(std::string const& key, QPDFObjectHandle value); using iterator = std::map::iterator; using const_iterator = std::map::const_iterator; @@ -806,6 +806,20 @@ namespace qpdf } } + /// @brief Retrieves the QPDFObjectHandle const& associated with the given key. + /// + /// This method provides a convenience alternative to the direct use of the subscript operator + /// "(*this)[key]" or "oh()[key]" in derived classes, enabling a simplified and readable way to + /// access object handles by key. + /// + /// @param key The string key used to look up the corresponding QPDFObjectHandle. + /// @return A constant reference to the QPDFObjectHandle associated with the specified key. + inline QPDFObjectHandle const& + BaseHandle::get(std::string const& key) const + { + return (*this)[key]; + } + inline bool BaseHandle::null() const { diff --git a/libtests/objects.cc b/libtests/objects.cc index ee9c7c5..6f20aee 100644 --- a/libtests/objects.cc +++ b/libtests/objects.cc @@ -54,35 +54,30 @@ test_0(QPDF& pdf, char const* arg2) assert_compare_numbers(INT_MAX, t.getKey("/Q1").getIntValueAsInt()); try { assert_compare_numbers(0u, QPDFObjectHandle::newNull().getUIntValueAsUInt()); - std::cerr << "convert null to uint did not throw\n"; } catch (QPDFExc const&) { std::cerr << "caught expected type error\n"; } assert_compare_numbers(std::numeric_limits::max(), Integer(q1).value()); assert_compare_numbers(std::numeric_limits::min(), Integer(-q1).value()); try { - int8_t q1_8 = Integer(q1); - std::cerr << "q1_8: " << std::to_string(q1_8) << '\n'; + [[maybe_unused]] int8_t q1_8 = Integer(q1); } catch (std::overflow_error const&) { std::cerr << "caught expected int8_t overflow error\n"; } try { - int8_t q1_8 = Integer(-q1); - std::cerr << "q1_8: " << std::to_string(q1_8) << '\n'; + [[maybe_unused]] int8_t q1_8 = Integer(-q1); } catch (std::underflow_error const&) { std::cerr << "caught expected int8_t underflow error\n"; } assert_compare_numbers(std::numeric_limits::max(), Integer(q1).value()); assert_compare_numbers(0, Integer(-q1).value()); try { - uint8_t q1_u8 = Integer(q1); - std::cerr << "q1_u8: " << std::to_string(q1_u8) << '\n'; + [[maybe_unused]] uint8_t q1_u8 = Integer(q1); } catch (std::overflow_error const&) { std::cerr << "caught expected uint8_t overflow error\n"; } try { - uint8_t q1_u8 = Integer(-q1); - std::cerr << "q1_u8: " << std::to_string(q1_u8) << '\n'; + [[maybe_unused]] uint8_t q1_u8 = Integer(-q1); } catch (std::underflow_error const&) { std::cerr << "caught expected uint8_t underflow error\n"; } @@ -95,6 +90,70 @@ test_0(QPDF& pdf, char const* arg2) assert_compare_numbers(UINT_MAX, t.getKey("/Q3").getUIntValueAsUInt()); } + +static void +test_1(QPDF& pdf, char const* arg2) +{ + // Test new dictionary methods. + using namespace qpdf; + auto d = Dictionary({{"/A", {}}, {"/B", Null()}, {"/C", Dictionary::empty()}}); + + // contains + assert(!d.contains("/A")); + assert(!d.contains("/B")); + assert(d.contains("/C")); + + auto i = Integer(42); + assert(!i.contains("/A")); + + // at + assert(!d.at("/A")); + assert(d.at("/B")); + assert(d.at("/B").null()); + assert(d.at("/C")); + assert(!d.at("/C").null()); + d.at("/C") = Integer(42); + assert(d.at("/C") == 42); + assert(!d.at("/D")); + assert(d.at("/D").null()); + assert(QPDFObjectHandle(d).getDictAsMap().contains("/D")); + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); + + bool thrown = false; + try { + i.at("/A"); + } catch (std::runtime_error const&) { + thrown = true; + } + assert(thrown); + + // find + assert(!d.find("/A")); + assert(d.find("/B")); + assert(d.find("/B").null()); + assert(d.find("/C")); + assert(Integer(d.find("/C")) == 42); + d.find("/C") = Name("/DontPanic"); + assert(Name(d.find("/C")) == "/DontPanic"); + assert(!d.find("/E")); + assert(!QPDFObjectHandle(d).getDictAsMap().contains("/E")); + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); + + // replace + assert(!i.replace("/A", Name("/DontPanic"))); + Dictionary di = i.oh(); + thrown = false; + try { + di.replace("/A", Name("/DontPanic")); + } catch (std::runtime_error const&) { + thrown = true; + } + assert(thrown); + d.replace("/C", Integer(42)); + assert(Integer(d["/C"]) == 42); + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); +} + void runtest(int n, char const* filename1, char const* arg2) { @@ -102,7 +161,7 @@ runtest(int n, char const* filename1, char const* arg2) // the test suite to see how the test is invoked to find the file // that the test is supposed to operate on. - std::set ignore_filename = {}; + std::set ignore_filename = {1,}; QPDF pdf; std::shared_ptr file_buf; @@ -116,7 +175,7 @@ runtest(int n, char const* filename1, char const* arg2) } std::map test_functions = { - {0, test_0}, + {0, test_0}, {1, test_1}, }; auto fn = test_functions.find(n); diff --git a/libtests/qtest/objects.test b/libtests/qtest/objects.test index 7dec720..f6fde63 100644 --- a/libtests/qtest/objects.test +++ b/libtests/qtest/objects.test @@ -11,12 +11,16 @@ require TestDriver; my $td = new TestDriver('objects'); -my $n_tests = 1; - +my $n_tests = 2; $td->runtest("integer type checks", {$td->COMMAND => "objects 0 minimal.pdf"}, {$td->FILE => "test0.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); +$td->runtest("dictionary checks", + {$td->COMMAND => "objects 1 -"}, + {$td->STRING => => "test 1 done\n", $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); + $td->report($n_tests); diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 8c9646e..157e38d 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -171,7 +171,6 @@ QPDFObjectHandle array ignoring erase item 0 QPDFObjectHandle dictionary false for hasKey 0 QPDFObjectHandle dictionary empty set for getKeys 0 QPDFObjectHandle dictionary empty map for asMap 0 -QPDFObjectHandle dictionary ignoring replaceKey 0 QPDFObjectHandle numeric non-numeric 0 QPDFObjectHandle erase array bounds 0 qpdf-c called qpdf_check_pdf 0