diff options
| -rw-r--r-- | ChangeLog | 33 | ||||
| -rw-r--r-- | README-maintainer | 3 | ||||
| -rw-r--r-- | README.md | 2 | ||||
| -rw-r--r-- | TODO | 52 | ||||
| -rwxr-xr-x | check_abi | 4 | ||||
| -rw-r--r-- | include/qpdf/JSON.hh | 35 | ||||
| -rw-r--r-- | include/qpdf/QPDFObjectHandle.hh | 32 | ||||
| -rw-r--r-- | libqpdf/JSON.cc | 65 | ||||
| -rw-r--r-- | libqpdf/QPDFAcroFormDocumentHelper.cc | 10 | ||||
| -rw-r--r-- | libqpdf/QPDFEFStreamObjectHelper.cc | 2 | ||||
| -rw-r--r-- | libqpdf/QPDFEmbeddedFileDocumentHelper.cc | 4 | ||||
| -rw-r--r-- | libqpdf/QPDFJob.cc | 2 | ||||
| -rw-r--r-- | libqpdf/QPDFObjectHandle.cc | 22 | ||||
| -rw-r--r-- | libqpdf/QPDFPageObjectHelper.cc | 6 | ||||
| -rw-r--r-- | libqpdf/QPDFWriter.cc | 4 | ||||
| -rw-r--r-- | libtests/json.cc | 49 | ||||
| -rw-r--r-- | libtests/libtests.testcov | 4 | ||||
| -rw-r--r-- | libtests/qtest/json/json.out | 14 | ||||
| -rw-r--r-- | manual/json.rst | 16 | ||||
| -rw-r--r-- | manual/release-notes.rst | 10 | ||||
| -rw-r--r-- | qpdf/test_driver.cc | 28 |
21 files changed, 245 insertions, 152 deletions
@@ -1,5 +1,28 @@ 2022-07-24 Jay Berkenbilt <ejb@ql.org> + * include/qpdf/JSON.hh: Schema validation: allow a single item to + appear anywhere that the schema has an array of a single item. + This makes it possible to change an element of the schema from an + item to an array to allow the data to accept an array where a + single value was previously required. This change is needed to + allow QPDFJob JSON to start accepting multiple items where a + single item used to be expected without breaking backward + compatibility. Without this change, the earlier fix to + removeAttachment would be a breaking change. Also allow the schema + to contain a multi-element array, which means that the output has + to have an array of the same length in the corresponding location, + and each element is validated against the corresponding schema + element. + + * QPDFObjectHandle: for the methods insertItem, appendItem, + eraseItem, replaceKey, and removeKey, add a corresponding + "AndGetNew" and/or "AndGetOld" methods. The ones that end with + "AndGetNew" return the newly added item. The ones that end with + "AndGetOld" return the old value. The AndGetNew methods make it + possible to create a new object, add it to an array or dictionary, + and get a handle to it all in one line. The AndGetOld methods make + it easier to retrieve an old value when removing or replacing it. + * Thanks to m-holger for doing significant cleanup of private APIs and internals around QPDFObjGen and for significantly improving the performance of QPDFObjGen -- See #731. This includes a few @@ -168,16 +191,6 @@ 128-bit without AES) an error rather than a warning when --allow-weak-crypto is not specified. Fixes #576. -2022-04-29 Jay Berkenbilt <ejb@ql.org> - - * QPDFObjectHandle: for the methods insertItem, appendItem, - eraseItem, replaceKey, and removeKey, add a corresponding "AndGet" - method (insertItemAndGet, appendItemAndGet, eraseItemAndGet, - replaceKeyAndGet, and removeKeyAndGet) that returns the newly - inserted, replaced, or removed item. This makes it possible to - create a new object, add it to an array or dictionary, and get a - handle to it all in one line. - 2022-04-24 Jay Berkenbilt <ejb@ql.org> * Bug fix: "removeAttachment" in the job JSON now takes an array diff --git a/README-maintainer b/README-maintainer index 29291634..c2967c99 100644 --- a/README-maintainer +++ b/README-maintainer @@ -291,7 +291,8 @@ RELEASE PREPARATION * Check for open fuzz crashes at https://oss-fuzz.com -* Check lgtm: https://lgtm.com/projects/g/qpdf/qpdf/?mode=list +* Check lgtm: https://lgtm.com/projects/g/qpdf/qpdf/?mode=list for + anything worth fixing * Check all open issues and pull requests in github and the sourceforge trackers. See ~/scripts/github-issues. Don't forget pull @@ -3,8 +3,6 @@ [](https://qpdf.sourceforge.io) [](https://github.com/qpdf/qpdf/actions) -[](https://lgtm.com/projects/g/qpdf/qpdf/alerts/) -[](https://lgtm.com/projects/g/qpdf/qpdf/context:cpp) [](https://qpdf.readthedocs.io/en/latest/?badge=latest) QPDF is a command-line tool and C++ library that performs content-preserving transformations on PDF files. It supports linearization, encryption, and numerous other features. It can also be used for splitting and merging files, creating PDF files (but you have to supply all the content yourself), and inspecting files for study or analysis. QPDF does not render PDFs or perform text extraction, and it does not contain higher-level interfaces for working with page contents. It is a low-level tool for working with the structure of PDF files and can be a valuable tool for anyone who wants to do programmatic or command-line-based manipulation of PDF files. @@ -8,29 +8,6 @@ Before Release: * Stay on top of https://github.com/pikepdf/pikepdf/pull/315 * Release qtest with updates to qtest-driver and copy back into qpdf -Parent pointer idea: - -* Have replaceKey, removeKey, and eraseItem return the old values. The - comments will clarify the difference between these and the andGet - versions. -* Add std::weak_ptr<QPDFObject> parent to QPDFObject. When adding a - direct object to an array or dictionary, set its parent. When - removing it, clear the parent pointer. -* When a direct object that already has a parent is added to - something, it is a warning and will become an error in qpdf 12. - There needs to be unsafe add methods used by unsafeShallowCopy. - These will add but not modify the parent pointer. - -This allows an object to be moved from one object to another by -removing it, which returns the now orphaned object, and then inserting -it somewhere else. It also doesn't break the pattern of adding a -direct object to something and subsequently mutating it. It just -prevents the same object from being added to more than one thing. - -Note that arrays and dictionaries still need to contain -QPDFObjectHandle because of indirect objects. This only pertains to -direct objects, which are always "resolved" in QPDFObjectHandle. - Next: * JSON v2 fixes @@ -42,10 +19,6 @@ Pending changes: appimage build specifically is setting the runpath, which is actually desirable in this case. Make sure to understand and document this. Maybe add a check for it in the build. -* Make job JSON accept a single element and treat as an array of one - when an array is expected. This allows for making things repeatable - in the future without breaking compatibility and is needed for the - remote-attachment fix to be backward-compatible. * Decide what to do about #664 (get*Box) * Add an option --ignore-encryption to ignore encryption information and treat encrypted files as if they weren't encrypted. This should @@ -70,6 +43,26 @@ Pending changes: about the case of more than 65,536 pages. If the top node has more than 256 children, we'll live with it. +Parent pointer idea: + +* Add std::weak_ptr<QPDFObject> parent to QPDFObject. When adding a + direct object to an array or dictionary, set its parent. When + removing it, clear the parent pointer. +* When a direct object that already has a parent is added to + something, it is a warning and will become an error in qpdf 12. + There needs to be unsafe add methods used by unsafeShallowCopy. + These will add but not modify the parent pointer. + +This allows an object to be moved from one object to another by +removing it, which returns the now orphaned object, and then inserting +it somewhere else. It also doesn't break the pattern of adding a +direct object to something and subsequently mutating it. It just +prevents the same object from being added to more than one thing. + +Note that arrays and dictionaries still need to contain +QPDFObjectHandle because of indirect objects. This only pertains to +direct objects, which are always "resolved" in QPDFObjectHandle. + Soon: Break ground on "Document-level work" @@ -131,11 +124,6 @@ JSON v2 fixes compatibility, but at least this gives people a namespace they can know will never conflict with future keys. - * Change schema validation so that if the schema contains an array - with more than one element, the output has to have an array with - the same number of elements whose individual elements are - validated according to the regular rules. - * When reading back in, we'll have to call pushInheritedAttributesToPage or getAllPages based on the values of the metadata. @@ -111,7 +111,9 @@ class Main: classes = set() for i in sorted(lib): # Find a symbol that looks like a class method. - m = re.match(r'(((?:^\S*?::)?(?:[^:\s]+))::([^:\s]+))\(', i) + m = re.match( + r'(((?:^\S*?::)?(?:[^:\s]+))::([^:\s]+))(?:\[[^\]]+\])?\(', + i) if m: full = m.group(1) clas = m.group(2) diff --git a/include/qpdf/JSON.hh b/include/qpdf/JSON.hh index ef1632c2..63c858b1 100644 --- a/include/qpdf/JSON.hh +++ b/include/qpdf/JSON.hh @@ -183,17 +183,30 @@ class JSON // // * The schema is a nested structure containing dictionaries, // single-element arrays, and strings only. - // * Recursively walk the schema. - // * If the current value is a dictionary, this object must have - // a dictionary in the same place with the same keys. If flags - // contains f_optional, a key in the schema does not have to - // be present in the object. Otherwise, all keys have to be - // present. Any key in the object must be present in the - // schema. - // * If the current value is an array, this object must have an - // array in the same place. The schema's array must contain a - // single element, which is used as a schema to validate each - // element of this object's corresponding array. + // * Recursively walk the schema. In the items below, "schema + // object" refers to an object in the schema, and "checked + // object" refers to the correspondingi part of the object + // being checked. + // * If the schema object is a dictionary, the checked object + // must have a dictionary in the same place with the same + // keys. If flags contains f_optional, a key in the schema + // does not have to be present in the object. Otherwise, all + // keys have to be present. Any key in the object must be + // present in the schema. + // * If the schema object is an array of length 1, the checked + // object may either be a single item or an array of items. + // The single item or each element of the checked object's + // array is validated against the single element of the + // schema's array. The rationale behind this logic is that a + // single element may appear wherever the schema allows a + // variable-length array. This makes it possible to start + // allowing an array in the future where a single element was + // previously required without breaking backward + // compatibility. + // * If the schema object is an array of length > 1, the checked + // object must be an array of the same length. In this case, + // each element of the checked object array is validated + // against the corresponding element of the schema array. // * Otherwise, the value must be a string whose value is a // description of the object's corresponding value, which may // have any type. diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 790acccd..7ea6b062 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -999,18 +999,15 @@ class QPDFObjectHandle // Mutator methods. - // Since qpdf 11: when a mutator object returns QPDFObjectHandle&, - // it is a reference to the object itself. This makes it possible - // to use a fluent style. For example: + // Since qpdf 11: for mutators that may add or remove an item, + // there are additional versions whose names contain "AndGet" that + // return the added or removed item. For example: // - // array.appendItem(i1).appendItem(i2); - // - // would append i1 and then i2 to the array. There are also items - // that end with AndGet and return a QPDFObjectHandle. These - // return the newly added object. For example: - // - // auto new_dict = dict.replaceKeyAndGet( + // auto new_dict = dict.replaceKeyAndGetNew( // "/New", QPDFObjectHandle::newDictionary()); + // + // auto old_value = dict.replaceKeyAndGetOld( + // "/New", "(something)"_qpdf); // Recursively copy this object, making it direct. An exception is // thrown if a loop is detected. With allow_streams true, keep @@ -1036,20 +1033,20 @@ class QPDFObjectHandle void insertItem(int at, QPDFObjectHandle const& item); // Like insertItem but return the item that was inserted. QPDF_DLL - QPDFObjectHandle insertItemAndGet(int at, QPDFObjectHandle const& item); + QPDFObjectHandle insertItemAndGetNew(int at, QPDFObjectHandle const& item); // Append an item to an array. QPDF_DLL void appendItem(QPDFObjectHandle const& item); // Append an item, and return the newly added item. QPDF_DLL - QPDFObjectHandle appendItemAndGet(QPDFObjectHandle const& item); + QPDFObjectHandle appendItemAndGetNew(QPDFObjectHandle const& item); // Remove the item at that position, reducing the size of the // array by one. QPDF_DLL void eraseItem(int at); // Erase and item and return the item that was removed. QPDF_DLL - QPDFObjectHandle eraseItemAndGet(int at); + QPDFObjectHandle eraseItemAndGetOld(int at); // Mutator methods for dictionary objects @@ -1060,14 +1057,19 @@ class QPDFObjectHandle // Replace value of key and return the value. QPDF_DLL QPDFObjectHandle - replaceKeyAndGet(std::string const& key, QPDFObjectHandle const& value); + replaceKeyAndGetNew(std::string const& key, QPDFObjectHandle const& value); + // Replace value of key and return the old value, or null if the + // key was previously not present. + QPDF_DLL + QPDFObjectHandle + replaceKeyAndGetOld(std::string const& key, QPDFObjectHandle const& value); // Remove key, doing nothing if key does not exist. QPDF_DLL void removeKey(std::string const& key); // Remove key and return the old value. If the old value didn't // exist, return a null object. QPDF_DLL - QPDFObjectHandle removeKeyAndGet(std::string const& key); + QPDFObjectHandle removeKeyAndGetOld(std::string const& key); // ABI: Remove in qpdf 12 [[deprecated("use replaceKey -- it does the same thing")]] QPDF_DLL void diff --git a/libqpdf/JSON.cc b/libqpdf/JSON.cc index 1796c859..d380049f 100644 --- a/libqpdf/JSON.cc +++ b/libqpdf/JSON.cc @@ -538,26 +538,55 @@ JSON::checkSchemaInternal( } } } else if (sch_arr) { - if (!this_arr) { - QTC::TC("libtests", "JSON wanted array"); - errors.push_back(err_prefix + " is supposed to be an array"); - return false; - } - if (sch_arr->elements.size() != 1) { - QTC::TC("libtests", "JSON schema array error"); + auto n_elements = sch_arr->elements.size(); + if (n_elements == 1) { + // A single-element array in the schema allows a single + // element in the object or a variable-length array, each + // of whose items must conform to the single element of + // the schema array. This doesn't apply to arrays of + // arrays -- we fall back to the behavior of allowing a + // single item only when the object is not an array. + if (this_arr) { + int i = 0; + for (auto const& element: this_arr->elements) { + checkSchemaInternal( + element.get(), + sch_arr->elements.at(0).get(), + flags, + errors, + prefix + "." + QUtil::int_to_string(i)); + ++i; + } + } else { + QTC::TC("libtests", "JSON schema array for single item"); + checkSchemaInternal( + this_v, + sch_arr->elements.at(0).get(), + flags, + errors, + prefix); + } + } else if (!this_arr || (this_arr->elements.size() != n_elements)) { + QTC::TC("libtests", "JSON schema array length mismatch"); errors.push_back( - err_prefix + " schema array contains other than one item"); + err_prefix + " is supposed to be an array of length " + + QUtil::uint_to_string(n_elements)); return false; - } - int i = 0; - for (auto const& element: this_arr->elements) { - checkSchemaInternal( - element.get(), - sch_arr->elements.at(0).get(), - flags, - errors, - prefix + "." + QUtil::int_to_string(i)); - ++i; + } else { + // A multi-element array in the schema must correspond to + // an element of the same length in the object. Each + // element in the object is validated against the + // corresponding element in the schema. + size_t i = 0; + for (auto const& element: this_arr->elements) { + checkSchemaInternal( + element.get(), + sch_arr->elements.at(i).get(), + flags, + errors, + prefix + "." + QUtil::uint_to_string(i)); + ++i; + } } } else if (!sch_str) { QTC::TC("libtests", "JSON schema other type"); diff --git a/libqpdf/QPDFAcroFormDocumentHelper.cc b/libqpdf/QPDFAcroFormDocumentHelper.cc index 62dbaeee..23d021ff 100644 --- a/libqpdf/QPDFAcroFormDocumentHelper.cc +++ b/libqpdf/QPDFAcroFormDocumentHelper.cc @@ -40,7 +40,7 @@ QPDFAcroFormDocumentHelper::getOrCreateAcroForm() { auto acroform = this->qpdf.getRoot().getKey("/AcroForm"); if (!acroform.isDictionary()) { - acroform = this->qpdf.getRoot().replaceKeyAndGet( + acroform = this->qpdf.getRoot().replaceKeyAndGetNew( "/AcroForm", this->qpdf.makeIndirectObject(QPDFObjectHandle::newDictionary())); } @@ -53,8 +53,8 @@ QPDFAcroFormDocumentHelper::addFormField(QPDFFormFieldObjectHelper ff) auto acroform = getOrCreateAcroForm(); auto fields = acroform.getKey("/Fields"); if (!fields.isArray()) { - fields = - acroform.replaceKeyAndGet("/Fields", QPDFObjectHandle::newArray()); + fields = acroform.replaceKeyAndGetNew( + "/Fields", QPDFObjectHandle::newArray()); } fields.appendItem(ff.getObjectHandle()); std::set<QPDFObjGen> visited; @@ -854,7 +854,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations( } dr.makeResourcesIndirect(this->qpdf); if (!dr.isIndirect()) { - dr = acroform.replaceKeyAndGet( + dr = acroform.replaceKeyAndGetNew( "/DR", this->qpdf.makeIndirectObject(dr)); } // Merge the other document's /DR, creating a conflict @@ -1076,7 +1076,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations( auto apdict = ah.getAppearanceDictionary(); std::vector<QPDFObjectHandle> streams; auto replace_stream = [](auto& dict, auto& key, auto& old) { - return dict.replaceKeyAndGet(key, old.copyStream()); + return dict.replaceKeyAndGetNew(key, old.copyStream()); }; if (apdict.isDictionary()) { for (auto& ap: apdict.ditems()) { diff --git a/libqpdf/QPDFEFStreamObjectHelper.cc b/libqpdf/QPDFEFStreamObjectHelper.cc index cbfe47a3..8380206d 100644 --- a/libqpdf/QPDFEFStreamObjectHelper.cc +++ b/libqpdf/QPDFEFStreamObjectHelper.cc @@ -28,7 +28,7 @@ QPDFEFStreamObjectHelper::setParam( { auto params = this->oh.getDict().getKey("/Params"); if (!params.isDictionary()) { - params = this->oh.getDict().replaceKeyAndGet( + params = this->oh.getDict().replaceKeyAndGetNew( "/Params", QPDFObjectHandle::newDictionary()); } params.replaceKey(pkey, pval); diff --git a/libqpdf/QPDFEmbeddedFileDocumentHelper.cc b/libqpdf/QPDFEmbeddedFileDocumentHelper.cc index 847a9786..fd706c27 100644 --- a/libqpdf/QPDFEmbeddedFileDocumentHelper.cc +++ b/libqpdf/QPDFEmbeddedFileDocumentHelper.cc @@ -62,8 +62,8 @@ QPDFEmbeddedFileDocumentHelper::initEmbeddedFiles() auto root = qpdf.getRoot(); auto names = root.getKey("/Names"); if (!names.isDictionary()) { - names = - root.replaceKeyAndGet("/Names", QPDFObjectHandle::newDictionary()); + names = root.replaceKeyAndGetNew( + "/Names", QPDFObjectHandle::newDictionary()); } auto embedded_files = names.getKey("/EmbeddedFiles"); if (!embedded_files.isDictionary()) { diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc index 26425def..2cbabac0 100644 --- a/libqpdf/QPDFJob.cc +++ b/libqpdf/QPDFJob.cc @@ -2100,7 +2100,7 @@ QPDFJob::doUnderOverlayForPage( QPDFObjectHandle resources = dest_page.getAttribute("/Resources", true); if (!resources.isDictionary()) { QTC::TC("qpdf", "QPDFJob overlay page with no resources"); - resources = dest_page.getObjectHandle().replaceKeyAndGet( + resources = dest_page.getObjectHandle().replaceKeyAndGetNew( "/Resources", QPDFObjectHandle::newDictionary()); } for (int from_pageno: pagenos[pageno]) { diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index bfbc19f9..135b7c39 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -915,7 +915,7 @@ QPDFObjectHandle::insertItem(int at, QPDFObjectHandle const& item) } QPDFObjectHandle -QPDFObjectHandle::insertItemAndGet(int at, QPDFObjectHandle const& item) +QPDFObjectHandle::insertItemAndGetNew(int at, QPDFObjectHandle const& item) { insertItem(at, item); return item; @@ -934,7 +934,7 @@ QPDFObjectHandle::appendItem(QPDFObjectHandle const& item) } QPDFObjectHandle -QPDFObjectHandle::appendItemAndGet(QPDFObjectHandle const& item) +QPDFObjectHandle::appendItemAndGetNew(QPDFObjectHandle const& item) { appendItem(item); return item; @@ -957,7 +957,7 @@ QPDFObjectHandle::eraseItem(int at) } QPDFObjectHandle -QPDFObjectHandle::eraseItemAndGet(int at) +QPDFObjectHandle::eraseItemAndGetOld(int at) { auto result = QPDFObjectHandle::newNull(); if (isArray() && (at < getArrayNItems()) && (at >= 0)) { @@ -1113,7 +1113,8 @@ QPDFObjectHandle::mergeResources( // subdictionaries just to get this shallow copy // functionality. QTC::TC("qpdf", "QPDFObjectHandle replace with copy"); - this_val = replaceKeyAndGet(rtype, this_val.shallowCopy()); + this_val = + replaceKeyAndGetNew(rtype, this_val.shallowCopy()); } std::map<QPDFObjGen, std::string> og_to_name; std::set<std::string> rnames; @@ -1242,13 +1243,22 @@ QPDFObjectHandle::replaceKey( } QPDFObjectHandle -QPDFObjectHandle::replaceKeyAndGet( +QPDFObjectHandle::replaceKeyAndGetNew( std::string const& key, QPDFObjectHandle const& value) { replaceKey(key, value); return value; } +QPDFObjectHandle +QPDFObjectHandle::replaceKeyAndGetOld( + std::string const& key, QPDFObjectHandle const& value) +{ + QPDFObjectHandle old = removeKeyAndGetOld(key); + replaceKey(key, value); + return old; +} + void QPDFObjectHandle::removeKey(std::string const& key) { @@ -1261,7 +1271,7 @@ QPDFObjectHandle::removeKey(std::string const& key) } QPDFObjectHandle -QPDFObjectHandle::removeKeyAndGet(std::string const& key) +QPDFObjectHandle::removeKeyAndGetOld(std::string const& key) { auto result = QPDFObjectHandle::newNull(); if (isDictionary()) { diff --git a/libqpdf/QPDFPageObjectHelper.cc b/libqpdf/QPDFPageObjectHelper.cc index 96a8ce69..9ad75cf8 100644 --- a/libqpdf/QPDFPageObjectHelper.cc +++ b/libqpdf/QPDFPageObjectHelper.cc @@ -595,7 +595,7 @@ QPDFPageObjectHelper::removeUnreferencedResourcesHelper( for (auto const& iter: to_filter) { QPDFObjectHandle dict = resources.getKey(iter); if (dict.isDictionary()) { - dict = resources.replaceKeyAndGet(iter, dict.shallowCopy()); + dict = resources.replaceKeyAndGetNew(iter, dict.shallowCopy()); rdicts.push_back(dict); auto keys = dict.getKeys(); known_names.insert(keys.begin(), keys.end()); @@ -1110,8 +1110,8 @@ QPDFPageObjectHelper::copyAnnotations( afdh->addAndRenameFormFields(new_fields); auto annots = this->oh.getKey("/Annots"); if (!annots.isArray()) { - annots = - this->oh.replaceKeyAndGet("/Annots", QPDFObjectHandle::newArray()); + annots = this->oh.replaceKeyAndGetNew( + "/Annots", QPDFObjectHandle::newArray()); } for (auto const& annot: new_annots) { annots.appendItem(annot); diff --git a/libqpdf/QPDFWriter.cc b/libqpdf/QPDFWriter.cc index b227e95a..e33d0965 100644 --- a/libqpdf/QPDFWriter.cc +++ b/libqpdf/QPDFWriter.cc @@ -1571,7 +1571,7 @@ QPDFWriter::unparseObject( "qpdf", "QPDFWriter create Extensions", this->m->qdf_mode ? 0 : 1); - extensions = object.replaceKeyAndGet( + extensions = object.replaceKeyAndGetNew( "/Extensions", QPDFObjectHandle::newDictionary()); } } else if (!have_extensions_other) { @@ -2277,7 +2277,7 @@ QPDFWriter::prepareFileForWrite() if (oh.isIndirect()) { QTC::TC("qpdf", "QPDFWriter make Extensions direct"); extensions_indirect = true; - oh = root.replaceKeyAndGet(key, oh.shallowCopy()); + oh = root.replaceKeyAndGetNew(key, oh.shallowCopy()); } if (oh.hasKey("/ADBE")) { QPDFObjectHandle adbe = oh.getKey("/ADBE"); diff --git a/libtests/json.cc b/libtests/json.cc index 9c5720f4..ef9188c5 100644 --- a/libtests/json.cc +++ b/libtests/json.cc @@ -162,7 +162,9 @@ test_schema() "x": "ecks" }, "s": [ - "esses" + { + "ss": "esses" + } ] } }, @@ -177,7 +179,11 @@ test_schema() "z": "ebra", "o": "ptional" } - } + }, + "four": [ + { "first": "first element" }, + { "second": "second element" } + ] } )"); @@ -225,17 +231,30 @@ test_schema() "else": { "z": "okay" } - } + }, + "four": [ + {"first": "missing second"} + ] } )"); check_schema(b, schema, 0, false, "missing items"); - check_schema(a, a, 0, false, "top-level schema array error"); - check_schema(b, b, 0, false, "lower-level schema array error"); JSON bad_schema = JSON::parse(R"({"a": true, "b": "potato?"})"); check_schema(bad_schema, bad_schema, 0, false, "bad schema field type"); + JSON c = JSON::parse(R"( +{ + "four": [ + { "first": 1 }, + { "oops": [2] } + ] +} +)"); + check_schema(c, schema, JSON::f_optional, false, "array element mismatch"); + + // "two" exercises the case of the JSON containing a single + // element where the schema has an array. JSON good = JSON::parse(R"( { "one": { @@ -245,22 +264,24 @@ test_schema() "x": [1, null] }, "s": [ - null, - "anything" + {"ss": null}, + {"ss": "anything"} ] } }, - "two": [ - { - "glarp": "enspliel", - "goose": 3.14 - } - ], + "two": { + "glarp": "enspliel", + "goose": 3.14 + }, "three": { "<objid>": { "z": "ebra" } - } + }, + "four": [ + { "first": 1 }, + { "second": [2] } + ] } )"); check_schema(good, schema, 0, false, "not optional"); diff --git a/libtests/libtests.testcov b/libtests/libtests.testcov index 21def9f3..2ceef541 100644 --- a/libtests/libtests.testcov +++ b/libtests/libtests.testcov @@ -36,8 +36,6 @@ Pl_PNGFilter decodePaeth 0 Pl_TIFFPredictor processRow 1 JSON wanted dictionary 0 JSON key missing in object 0 -JSON wanted array 0 -JSON schema array error 0 JSON key extra in object 0 QPDFArgParser read args from stdin 0 QPDFArgParser read args from file 0 @@ -93,3 +91,5 @@ JSON 16 high high 0 JSON 16 low not after high 0 JSON 16 dangling high 0 JSON parse duplicate key 0 +JSON schema array for single item 0 +JSON schema array length mismatch 0 diff --git a/libtests/qtest/json/json.out b/libtests/qtest/json/json.out index c7cb85a7..f346daa6 100644 --- a/libtests/qtest/json/json.out +++ b/libtests/qtest/json/json.out @@ -2,9 +2,11 @@ top-level object is supposed to be a dictionary --- --- missing items +json key ".four" is supposed to be an array of length 2 json key ".one.a": key "q" is present in schema but missing in object json key ".one.a.r" is supposed to be a dictionary -json key ".one.a.s" is supposed to be an array +json key ".one.a.s": key "ss" is present in schema but missing in object +json key ".one.a.s": key "z" is not present in schema but appears in object json key ".one.a": key "t" is not present in schema but appears in object json key ".three.anything": key "z" is present in schema but missing in object json key ".three.anything": key "x" is not present in schema but appears in object @@ -14,16 +16,12 @@ json key ".two.1": key "flarp" is not present in schema but appears in object json key ".two.2" is supposed to be a dictionary json key ".two.3" is supposed to be a dictionary --- ---- top-level schema array error -top-level object schema array contains other than one item ---- ---- lower-level schema array error -json key ".one.a.r" schema array contains other than one item -json key ".two" schema array contains other than one item ---- --- bad schema field type json key ".a" schema value is not dictionary, array, or string --- +--- array element mismatch +json key ".four.1": key "oops" is not present in schema but appears in object +--- --- not optional json key ".three.<objid>": key "o" is present in schema but missing in object --- diff --git a/manual/json.rst b/manual/json.rst index 2e71bdab..92a89b6b 100644 --- a/manual/json.rst +++ b/manual/json.rst @@ -595,11 +595,17 @@ Documentation appears in the corresponding location of the actual output. The corresponding output can have any value including ``null``. - - An array in the help output always contains a single element. It - indicates that the corresponding location in the actual output is - an array of any length, and that each element of the array has - whatever format is implied by the single element of the help - output's array. + - A single-element array in the help output indicates that the + corresponding location in the actual output is either a single + item or is an array of any length. The single item or each + element of the array has whatever format is implied by the single + element of the help output's array. + + - A multi-element array in the help output indicates that the + corresponding location in the actual output is an array of the + same length. Each element of the output array has whatever format + is implied by the corresponding element of the help output's + array. For example, the help output indicates includes a ``"pagelabels"`` key whose value is an array of one element. That element is a diff --git a/manual/release-notes.rst b/manual/release-notes.rst index 3d5b5cc5..ebbfd4f5 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -163,9 +163,13 @@ For a detailed list of changes, please see the file - See examples :file:`examples/qpdfjob-save-attachment.cc` and :file:`examples/qpdfjob-c-save-attachment.cc`. - - New methods ``insertItemAndGet``, ``appendItemAndGet``, - ``eraseItemAndGet``, ``replaceKeyAndGet``, and - ``removeKeyAndGet`` return the newly added or removed object. + - In ``QPDFObjectHandle``, new methods ``insertItemAndGetNew``, + ``appendItemAndGetNew``, and ``replaceKeyAndGetNew`` return the + newly added item. New methods ``eraseItemAndGetOld``, + ``replaceKeyAndGetOld``, and ``removeKeyAndGetOld`` return the + item that was just removed or, in the case of + ``replaceKeyAndGetOld``, a ``null`` object if the object was not + previously there. - Add new ``Pipeline`` methods to reduce the amount of casting that is needed: diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index f1c1d72a..8378c8fa 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -1095,8 +1095,8 @@ test_27(QPDF& pdf, char const* arg2) QPDFObjectHandle s2 = QPDFObjectHandle::newStream(&oldpdf, "potato\n"); auto trailer = pdf.getTrailer(); trailer.replaceKey("/QTest", pdf.copyForeignObject(qtest)); - auto qtest2 = - trailer.replaceKeyAndGet("/QTest2", QPDFObjectHandle::newArray()); + auto qtest2 = trailer.replaceKeyAndGetNew( + "/QTest2", QPDFObjectHandle::newArray()); qtest2.appendItem(pdf.copyForeignObject(s1)); qtest2.appendItem(pdf.copyForeignObject(s2)); qtest2.appendItem(pdf.copyForeignObject(s3)); @@ -3165,15 +3165,23 @@ test_88(QPDF& pdf, char const* arg2) auto dict = QPDFObjectHandle::newDictionary(); dict.replaceKey("/One", QPDFObjectHandle::newInteger(1)); dict.replaceKey("/Two", QPDFObjectHandle::newInteger(2)); - auto three = dict.replaceKeyAndGet("/Three", QPDFObjectHandle::newArray()); + auto three = + dict.replaceKeyAndGetNew("/Three", QPDFObjectHandle::newArray()); three.appendItem("(a)"_qpdf); three.appendItem("(b)"_qpdf); - auto newdict = three.appendItemAndGet(QPDFObjectHandle::newDictionary()); + auto newdict = three.appendItemAndGetNew(QPDFObjectHandle::newDictionary()); newdict.replaceKey("/Z", "/Y"_qpdf); newdict.replaceKey("/X", "/W"_qpdf); + dict.replaceKey("/Quack", "[1 2 3]"_qpdf); + auto quack = dict.replaceKeyAndGetOld("/Quack", "/Moo"_qpdf); + assert(quack.unparse() == "[ 1 2 3 ]"); + auto nothing = + dict.replaceKeyAndGetOld("/NotThere", QPDFObjectHandle::newNull()); + assert(nothing.isNull()); assert(dict.unparse() == R"( << /One 1 + /Quack /Moo /Two 2 /Three [ (a) (b) << /Z /Y /X /W >> ] >> @@ -3184,7 +3192,7 @@ test_88(QPDF& pdf, char const* arg2) assert( arr.unparse() == "[ (00) (0) (a) (b) << /Z /Y /X /W >> ]"_qpdf.unparse()); - auto new_dict = arr.insertItemAndGet(1, "<< /P /Q /R /S >>"_qpdf); + auto new_dict = arr.insertItemAndGetNew(1, "<< /P /Q /R /S >>"_qpdf); arr.eraseItem(2); arr.eraseItem(0); assert( @@ -3200,20 +3208,20 @@ test_88(QPDF& pdf, char const* arg2) assert( arr.unparse() == "[ << /P /Q /T /U >> (a) (b) << /Z /Y /X /W >> ]"_qpdf.unparse()); - auto s = arr.eraseItemAndGet(1); + auto s = arr.eraseItemAndGetOld(1); assert(s.unparse() == "(a)"); assert( arr.unparse() == "[ << /P /Q /T /U >> (b) << /Z /Y /X /W >> ]"_qpdf.unparse()); - assert(new_dict.removeKeyAndGet("/M").isNull()); - assert(new_dict.removeKeyAndGet("/P").unparse() == "/Q"); + assert(new_dict.removeKeyAndGetOld("/M").isNull()); + assert(new_dict.removeKeyAndGetOld("/P").unparse() == "/Q"); assert(new_dict.unparse() == "<< /T /U >>"_qpdf.unparse()); // Test errors - auto arr2 = pdf.getRoot().replaceKeyAndGet("/QTest", "[1 2]"_qpdf); + auto arr2 = pdf.getRoot().replaceKeyAndGetNew("/QTest", "[1 2]"_qpdf); arr2.setObjectDescription(&pdf, "test array"); - assert(arr2.eraseItemAndGet(50).isNull()); + assert(arr2.eraseItemAndGetOld(50).isNull()); } static void |