From 08ba21cf4935fec39d3454714c03d36ff6a6b836 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Sun, 24 Apr 2022 10:08:32 -0400 Subject: Fix some bugs around null values in dictionaries Make it so that a key with a null value is always treated as not being present. This was inconsistent before. --- libqpdf/QPDF_Dictionary.cc | 36 ++++++++++++++++-------------------- qpdf/qtest/qpdf.test | 11 +++++++++++ qpdf/qtest/qpdf/bad34-recover.out | 2 +- qpdf/qtest/qpdf/good11.out | 4 ++-- qpdf/test_driver.cc | 24 ++++++++++++++++++++++-- 5 files changed, 52 insertions(+), 25 deletions(-) diff --git a/libqpdf/QPDF_Dictionary.cc b/libqpdf/QPDF_Dictionary.cc index 0423f41f..c3343b21 100644 --- a/libqpdf/QPDF_Dictionary.cc +++ b/libqpdf/QPDF_Dictionary.cc @@ -24,12 +24,11 @@ std::string QPDF_Dictionary::unparse() { std::string result = "<< "; - for (std::map::iterator iter = - this->items.begin(); - iter != this->items.end(); - ++iter) { - result += QPDF_Name::normalizeName((*iter).first) + " " + - (*iter).second.unparse() + " "; + for (auto& iter : this->items) { + if (!iter.second.isNull()) { + result += QPDF_Name::normalizeName(iter.first) + " " + + iter.second.unparse() + " "; + } } result += ">>"; return result; @@ -39,12 +38,11 @@ JSON QPDF_Dictionary::getJSON() { JSON j = JSON::makeDictionary(); - for (std::map::iterator iter = - this->items.begin(); - iter != this->items.end(); - ++iter) { - j.addDictionaryMember( - QPDF_Name::normalizeName((*iter).first), (*iter).second.getJSON()); + for (auto& iter : this->items) { + if (!iter.second.isNull()) { + j.addDictionaryMember( + QPDF_Name::normalizeName(iter.first), iter.second.getJSON()); + } } return j; } @@ -78,9 +76,10 @@ QPDF_Dictionary::getKey(std::string const& key) { // PDF spec says fetching a non-existent key from a dictionary // returns the null object. - if (this->items.count(key)) { + auto item = this->items.find(key); + if (item != this->items.end()) { // May be a null object - return (*(this->items.find(key))).second; + return item->second; } else { QPDFObjectHandle null = QPDFObjectHandle::newNull(); QPDF* qpdf = 0; @@ -97,12 +96,9 @@ std::set QPDF_Dictionary::getKeys() { std::set result; - for (std::map::const_iterator iter = - this->items.begin(); - iter != this->items.end(); - ++iter) { - if (hasKey((*iter).first)) { - result.insert((*iter).first); + for (auto& iter : this->items) { + if (!iter.second.isNull()) { + result.insert(iter.first); } } return result; diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index 36f843f7..5bf6be76 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -1419,6 +1419,17 @@ foreach (my $i = 1; $i <= 3; ++$i) $td->NORMALIZE_NEWLINES); } +show_ntests(); +# ---------- +$td->notify("--- Dictionary keys ---"); +$n_tests += 1; + +$td->runtest("dictionary keys", + {$td->COMMAND => "test_driver 87 - -"}, + {$td->STRING => "test 87 done\n", + $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); + show_ntests(); # ---------- $td->notify("--- Stream data ---"); diff --git a/qpdf/qtest/qpdf/bad34-recover.out b/qpdf/qtest/qpdf/bad34-recover.out index 4cee51be..def6c378 100644 --- a/qpdf/qtest/qpdf/bad34-recover.out +++ b/qpdf/qtest/qpdf/bad34-recover.out @@ -2,7 +2,7 @@ WARNING: bad34.pdf: file is damaged WARNING: bad34.pdf (object 4 0, offset 322): expected n n obj WARNING: bad34.pdf: Attempting to reconstruct cross-reference table /QTest is indirect and has type stream (10) -/QTest is a stream. Dictionary: << /Length 44 /Quack 9 0 R >> +/QTest is a stream. Dictionary: << /Length 44 >> Raw stream data: BT /F1 24 Tf diff --git a/qpdf/qtest/qpdf/good11.out b/qpdf/qtest/qpdf/good11.out index 2079096a..06358d49 100644 --- a/qpdf/qtest/qpdf/good11.out +++ b/qpdf/qtest/qpdf/good11.out @@ -1,6 +1,6 @@ /QTest is direct and has type dictionary (9) /QTest is a dictionary /a is direct -unparse: << /a (a) /b 8 0 R >> -unparseResolved: << /a (a) /b 8 0 R >> +unparse: << /a (a) >> +unparseResolved: << /a (a) >> test 1 done diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 0245e0f5..ead0b484 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -3158,6 +3158,26 @@ test_86(QPDF& pdf, char const* arg2) assert(h.getUTF8Value() == utf8_val); } +static void +test_87(QPDF& pdf, char const* arg2) +{ + // Explicitly demonstrate null dictionary values being the same as + // missing keys. + auto dict = "<< /A 1 /B null >>"_qpdf; + assert(dict.unparse() == "<< /A 1 >>"); + assert(dict.getKeys() == std::set({"/A"})); + dict.replaceKey("/A", QPDFObjectHandle::newNull()); + assert(dict.unparse() == "<< >>"); + assert(dict.getKeys() == std::set()); + dict = QPDFObjectHandle::newDictionary({ + {"/A", "2"_qpdf}, + {"/B", QPDFObjectHandle::newNull()}, + }); + assert(dict.unparse() == "<< /A 2 >>"); + assert(dict.getKeys() == std::set({"/A"})); + assert(dict.getJSON().unparse() == "{\n \"/A\": 2\n}"); +} + void runtest(int n, char const* filename1, char const* arg2) { @@ -3165,7 +3185,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 = {61, 81, 83, 84, 85, 86}; + std::set ignore_filename = {61, 81, 83, 84, 85, 86, 87}; if (n == 0) { // Throw in some random test cases that don't fit anywhere @@ -3259,7 +3279,7 @@ runtest(int n, char const* filename1, char const* arg2) {72, test_72}, {73, test_73}, {74, test_74}, {75, test_75}, {76, test_76}, {77, test_77}, {78, test_78}, {79, test_79}, {80, test_80}, {81, test_81}, {82, test_82}, {83, test_83}, - {84, test_84}, {85, test_85}, {86, test_86}, + {84, test_84}, {85, test_85}, {86, test_86}, {87, test_87}, }; auto fn = test_functions.find(n); -- cgit v1.2.3-70-g09d2