From 44bd9db194e87d71b4d7573a2f84c6894256427c Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 10 Nov 2025 00:15:18 +0000 Subject: [PATCH] Refactor: replace `BaseDictionary::replaceKey` method with `BaseHandle::replace` for clarity and consistency across dictionary operations. --- include/qpdf/ObjectHandle.hh | 1 + libqpdf/NNTree.cc | 10 +++++----- libqpdf/QPDF.cc | 6 +++--- libqpdf/QPDFAcroFormDocumentHelper.cc | 10 +++++----- libqpdf/QPDFEFStreamObjectHelper.cc | 2 +- libqpdf/QPDFFormFieldObjectHelper.cc | 2 +- libqpdf/QPDF_Dictionary.cc | 36 +++++++++++++++++++++++------------- libqpdf/qpdf/QPDFObjectHandle_private.hh | 2 +- qpdf/qpdf.testcov | 1 - 9 files changed, 40 insertions(+), 30 deletions(-) diff --git a/include/qpdf/ObjectHandle.hh b/include/qpdf/ObjectHandle.hh index 3e8f84d..8d8f200 100644 --- a/include/qpdf/ObjectHandle.hh +++ b/include/qpdf/ObjectHandle.hh @@ -85,6 +85,7 @@ namespace qpdf bool contains(std::string const& key) const; size_t erase(std::string const& key); + bool replace(std::string const& key, QPDFObjectHandle value); QPDFObjectHandle const& operator[](std::string const& key) const; std::shared_ptr copy(bool shallow = false) 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/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/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/QPDF_Dictionary.cc b/libqpdf/QPDF_Dictionary.cc index f606b2b..b080c0b 100644 --- a/libqpdf/QPDF_Dictionary.cc +++ b/libqpdf/QPDF_Dictionary.cc @@ -63,18 +63,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(); } } @@ -166,11 +177,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 diff --git a/libqpdf/qpdf/QPDFObjectHandle_private.hh b/libqpdf/qpdf/QPDFObjectHandle_private.hh index 3f32915..51b7897 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; 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 -- libgit2 0.21.4