diff --git a/include/qpdf/QPDFAcroFormDocumentHelper.hh b/include/qpdf/QPDFAcroFormDocumentHelper.hh index 0ac2313..8aff780 100644 --- a/include/qpdf/QPDFAcroFormDocumentHelper.hh +++ b/include/qpdf/QPDFAcroFormDocumentHelper.hh @@ -226,8 +226,7 @@ class QPDFAcroFormDocumentHelper: public QPDFDocumentHelper private: void analyze(); - void traverseField( - QPDFObjectHandle field, QPDFObjectHandle parent, int depth, QPDFObjGen::set& visited); + void traverseField(QPDFObjectHandle const& field, QPDFObjectHandle const& parent, int depth); QPDFObjectHandle getOrCreateAcroForm(); void adjustInheritedFields( QPDFObjectHandle obj, diff --git a/libqpdf/QPDFAcroFormDocumentHelper.cc b/libqpdf/QPDFAcroFormDocumentHelper.cc index faef618..182da1a 100644 --- a/libqpdf/QPDFAcroFormDocumentHelper.cc +++ b/libqpdf/QPDFAcroFormDocumentHelper.cc @@ -89,8 +89,7 @@ QPDFAcroFormDocumentHelper::addFormField(QPDFFormFieldObjectHelper ff) fields = acroform.replaceKeyAndGetNew("/Fields", QPDFObjectHandle::newArray()); } fields.appendItem(ff.getObjectHandle()); - QPDFObjGen::set visited; - traverseField(ff.getObjectHandle(), QPDFObjectHandle::newNull(), 0, visited); + traverseField(ff.getObjectHandle(), {}, 0); } void @@ -188,9 +187,7 @@ void QPDFAcroFormDocumentHelper::setFormFieldName(QPDFFormFieldObjectHelper ff, std::string const& name) { ff.setFieldAttribute("/T", name); - QPDFObjGen::set visited; - auto ff_oh = ff.getObjectHandle(); - traverseField(ff_oh, ff_oh.getKey("/Parent"), 0, visited); + traverseField(ff, ff["/Parent"], 0); } std::vector @@ -199,7 +196,7 @@ QPDFAcroFormDocumentHelper::getFormFields() analyze(); std::vector result; for (auto const& [og, data]: m->field_to) { - if (!(data.annotations.empty())) { + if (!data.annotations.empty()) { result.emplace_back(qpdf.getObject(og)); } } @@ -279,18 +276,14 @@ QPDFAcroFormDocumentHelper::analyze() return; } QPDFObjectHandle fields = acroform.getKey("/Fields"); - if (auto fa = fields.as_array(strict)) { + if (Array fa = fields) { // Traverse /AcroForm to find annotations and map them bidirectionally to fields. - - QPDFObjGen::set visited; - QPDFObjectHandle null(QPDFObjectHandle::newNull()); for (auto const& field: fa) { - traverseField(field, null, 0, visited); + traverseField(field, {}, 0); } } else { - QTC::TC("qpdf", "QPDFAcroFormDocumentHelper fields not array"); acroform.warn("/Fields key of /AcroForm dictionary is not an array; ignoring"); - fields = QPDFObjectHandle::newArray(); + fields = Array::empty(); } // All Widget annotations should have been encountered by traversing /AcroForm, but in case any @@ -305,7 +298,6 @@ QPDFAcroFormDocumentHelper::analyze() QPDFObjectHandle annot(iter.getObjectHandle()); QPDFObjGen og(annot.getObjGen()); if (!m->annotation_to_field.contains(og)) { - QTC::TC("qpdf", "QPDFAcroFormDocumentHelper orphaned widget"); // This is not supposed to happen, but it's easy enough for us to handle this case. // Treat the annotation as its own field. This could allow qpdf to sensibly handle a // case such as a PDF creator adding a self-contained annotation (merged with the @@ -323,7 +315,7 @@ QPDFAcroFormDocumentHelper::analyze() void QPDFAcroFormDocumentHelper::traverseField( - QPDFObjectHandle field, QPDFObjectHandle parent, int depth, QPDFObjGen::set& visited) + QPDFObjectHandle const& field, QPDFObjectHandle const& parent, int depth) { if (depth > 100) { // Arbitrarily cut off recursion at a fixed depth to avoid specially crafted files that @@ -331,22 +323,19 @@ QPDFAcroFormDocumentHelper::traverseField( return; } if (!field.indirect()) { - QTC::TC("qpdf", "QPDFAcroFormDocumentHelper direct field"); field.warn( - "encountered a direct object as a field or annotation while " - "traversing /AcroForm; ignoring field or annotation"); + "encountered a direct object as a field or annotation while traversing /AcroForm; " + "ignoring field or annotation"); return; } if (!field.isDictionary()) { - QTC::TC("qpdf", "QPDFAcroFormDocumentHelper non-dictionary field"); field.warn( - "encountered a non-dictionary as a field or annotation while" - " traversing /AcroForm; ignoring field or annotation"); + "encountered a non-dictionary as a field or annotation while traversing /AcroForm; " + "ignoring field or annotation"); return; } QPDFObjGen og(field.getObjGen()); - if (!visited.add(og)) { - QTC::TC("qpdf", "QPDFAcroFormDocumentHelper loop"); + if (m->field_to.contains(og) || m->annotation_to_field.contains(og)) { field.warn("loop detected while traversing /AcroForm"); return; } @@ -357,21 +346,10 @@ QPDFAcroFormDocumentHelper::traverseField( // fields can be merged with terminal field dictionaries. Otherwise, the annotation fields might // be there to be inherited by annotations below it. - bool is_annotation = false; - bool is_field = (0 == depth); - if (auto a = field.getKey("/Kids").as_array(strict)) { - is_field = true; - for (auto const& item: a) { - traverseField(item, field, 1 + depth, visited); - } - } else { - if (field.hasKey("/Parent")) { - is_field = true; - } - if (field.hasKey("/Subtype") || field.hasKey("/Rect") || field.hasKey("/AP")) { - is_annotation = true; - } - } + Array Kids = field["/Kids"]; + const bool is_field = depth == 0 || Kids || field.hasKey("/Parent"); + const bool is_annotation = + !Kids && (field.hasKey("/Subtype") || field.hasKey("/Rect") || field.hasKey("/AP")); QTC::TC("qpdf", "QPDFAcroFormDocumentHelper field found", (depth == 0) ? 0 : 1); QTC::TC("qpdf", "QPDFAcroFormDocumentHelper annotation found", (is_field ? 0 : 1)); @@ -384,15 +362,18 @@ QPDFAcroFormDocumentHelper::traverseField( if (is_field && (field.hasKey("/T"))) { QPDFFormFieldObjectHelper foh(field); - auto f_og = field.getObjGen(); std::string name = foh.getFullyQualifiedName(); - auto old = m->field_to.find(f_og); + auto old = m->field_to.find(og); if (old != m->field_to.end() && !old->second.name.empty()) { // We might be updating after a name change, so remove any old information - m->name_to_fields[old->second.name].erase(f_og); + m->name_to_fields[old->second.name].erase(og); } - m->field_to[f_og].name = name; - m->name_to_fields[name].insert(f_og); + m->field_to[og].name = name; + m->name_to_fields[name].insert(og); + } + + for (auto const& kid: Kids) { + traverseField(kid, field, 1 + depth); } } diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 167e4ea..3f5664b 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -204,11 +204,6 @@ QPDFAnnotationObjectHelper AP stream 0 QPDFAnnotationObjectHelper AP dictionary 0 QPDFAnnotationObjectHelper AP sub stream 0 QPDFAnnotationObjectHelper AP null 0 -QPDFAcroFormDocumentHelper fields not array 0 -QPDFAcroFormDocumentHelper orphaned widget 0 -QPDFAcroFormDocumentHelper direct field 0 -QPDFAcroFormDocumentHelper non-dictionary field 0 -QPDFAcroFormDocumentHelper loop 0 QPDFAcroFormDocumentHelper field found 1 QPDFAcroFormDocumentHelper annotation found 1 QPDFJob automatically set keep files open 1