Commit 9b2eb01e2505e301ce95d31f5387fea0de35eff0
1 parent
6c2fb5b8
Exercise object description in tests
Showing
8 changed files
with
77 additions
and
16 deletions
TODO
| @@ -58,9 +58,6 @@ Some of this documentation has drifted from the actual implementation. | @@ -58,9 +58,6 @@ Some of this documentation has drifted from the actual implementation. | ||
| 58 | 58 | ||
| 59 | Make sure pages tree repair generates warnings. | 59 | Make sure pages tree repair generates warnings. |
| 60 | 60 | ||
| 61 | -* Have a test case if possible that exercises the object description | ||
| 62 | - which means we need some kind of semantic error that gets caught | ||
| 63 | - after creation. | ||
| 64 | * Document that /Length is ignored in stream dictionary replacements | 61 | * Document that /Length is ignored in stream dictionary replacements |
| 65 | 62 | ||
| 66 | Try to never flatten pages tree. Make sure we do something reasonable | 63 | Try to never flatten pages tree. Make sure we do something reasonable |
include/qpdf/QPDF.hh
| @@ -1041,12 +1041,15 @@ class QPDF | @@ -1041,12 +1041,15 @@ class QPDF | ||
| 1041 | 1041 | ||
| 1042 | void containerStart(); | 1042 | void containerStart(); |
| 1043 | void nestedState(std::string const& key, JSON const& value, state_e); | 1043 | void nestedState(std::string const& key, JSON const& value, state_e); |
| 1044 | + void setObjectDescription(QPDFObjectHandle& oh, JSON const& value); | ||
| 1044 | QPDFObjectHandle makeObject(JSON const& value); | 1045 | QPDFObjectHandle makeObject(JSON const& value); |
| 1045 | void error(size_t offset, std::string const& message); | 1046 | void error(size_t offset, std::string const& message); |
| 1046 | QPDFObjectHandle | 1047 | QPDFObjectHandle |
| 1047 | reserveObject(std::string const& obj, std::string const& gen); | 1048 | reserveObject(std::string const& obj, std::string const& gen); |
| 1048 | void replaceObject( | 1049 | void replaceObject( |
| 1049 | - QPDFObjectHandle to_replace, QPDFObjectHandle replacement); | 1050 | + QPDFObjectHandle to_replace, |
| 1051 | + QPDFObjectHandle replacement, | ||
| 1052 | + JSON const& value); | ||
| 1050 | 1053 | ||
| 1051 | QPDF& pdf; | 1054 | QPDF& pdf; |
| 1052 | std::shared_ptr<InputSource> is; | 1055 | std::shared_ptr<InputSource> is; |
libqpdf/QPDF_json.cc
| @@ -249,11 +249,15 @@ QPDF::JSONReactor::reserveObject(std::string const& obj, std::string const& gen) | @@ -249,11 +249,15 @@ QPDF::JSONReactor::reserveObject(std::string const& obj, std::string const& gen) | ||
| 249 | 249 | ||
| 250 | void | 250 | void |
| 251 | QPDF::JSONReactor::replaceObject( | 251 | QPDF::JSONReactor::replaceObject( |
| 252 | - QPDFObjectHandle to_replace, QPDFObjectHandle replacement) | 252 | + QPDFObjectHandle to_replace, |
| 253 | + QPDFObjectHandle replacement, | ||
| 254 | + JSON const& value) | ||
| 253 | { | 255 | { |
| 254 | auto og = to_replace.getObjGen(); | 256 | auto og = to_replace.getObjGen(); |
| 255 | this->reserved.erase(og); | 257 | this->reserved.erase(og); |
| 256 | this->pdf.replaceObject(og, replacement); | 258 | this->pdf.replaceObject(og, replacement); |
| 259 | + auto oh = pdf.getObjectByObjGen(og); | ||
| 260 | + setObjectDescription(oh, value); | ||
| 257 | } | 261 | } |
| 258 | 262 | ||
| 259 | void | 263 | void |
| @@ -326,9 +330,10 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | @@ -326,9 +330,10 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | ||
| 326 | nestedState(key, value, st_trailer); | 330 | nestedState(key, value, st_trailer); |
| 327 | this->cur_object = "trailer"; | 331 | this->cur_object = "trailer"; |
| 328 | } else if (std::regex_match(key, m, OBJ_KEY_RE)) { | 332 | } else if (std::regex_match(key, m, OBJ_KEY_RE)) { |
| 329 | - object_stack.push_back(reserveObject(m[1].str(), m[2].str())); | ||
| 330 | - nestedState(key, value, st_object_top); | ||
| 331 | this->cur_object = key; | 333 | this->cur_object = key; |
| 334 | + auto oh = reserveObject(m[1].str(), m[2].str()); | ||
| 335 | + object_stack.push_back(oh); | ||
| 336 | + nestedState(key, value, st_object_top); | ||
| 332 | } else { | 337 | } else { |
| 333 | QTC::TC("qpdf", "QPDF_json bad object key"); | 338 | QTC::TC("qpdf", "QPDF_json bad object key"); |
| 334 | error( | 339 | error( |
| @@ -348,7 +353,7 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | @@ -348,7 +353,7 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | ||
| 348 | this->saw_value = true; | 353 | this->saw_value = true; |
| 349 | next_state = st_object; | 354 | next_state = st_object; |
| 350 | replacement = makeObject(value); | 355 | replacement = makeObject(value); |
| 351 | - replaceObject(tos, replacement); | 356 | + replaceObject(tos, replacement, value); |
| 352 | } else if (key == "stream") { | 357 | } else if (key == "stream") { |
| 353 | this->saw_stream = true; | 358 | this->saw_stream = true; |
| 354 | nestedState(key, value, st_stream); | 359 | nestedState(key, value, st_stream); |
| @@ -359,7 +364,7 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | @@ -359,7 +364,7 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | ||
| 359 | this->this_stream_needs_data = true; | 364 | this->this_stream_needs_data = true; |
| 360 | replacement = | 365 | replacement = |
| 361 | pdf.reserveStream(tos.getObjectID(), tos.getGeneration()); | 366 | pdf.reserveStream(tos.getObjectID(), tos.getGeneration()); |
| 362 | - replaceObject(tos, replacement); | 367 | + replaceObject(tos, replacement, value); |
| 363 | } | 368 | } |
| 364 | } else { | 369 | } else { |
| 365 | // Ignore unknown keys for forward compatibility | 370 | // Ignore unknown keys for forward compatibility |
| @@ -376,6 +381,7 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | @@ -376,6 +381,7 @@ QPDF::JSONReactor::dictionaryItem(std::string const& key, JSON const& value) | ||
| 376 | // The trailer must be a dictionary, so we can use nestedState. | 381 | // The trailer must be a dictionary, so we can use nestedState. |
| 377 | nestedState("trailer.value", value, st_object); | 382 | nestedState("trailer.value", value, st_object); |
| 378 | this->pdf.m->trailer = makeObject(value); | 383 | this->pdf.m->trailer = makeObject(value); |
| 384 | + setObjectDescription(this->pdf.m->trailer, value); | ||
| 379 | } else if (key == "stream") { | 385 | } else if (key == "stream") { |
| 380 | // Don't need to set saw_stream here since there's already | 386 | // Don't need to set saw_stream here since there's already |
| 381 | // an error. | 387 | // an error. |
| @@ -471,6 +477,17 @@ QPDF::JSONReactor::arrayItem(JSON const& value) | @@ -471,6 +477,17 @@ QPDF::JSONReactor::arrayItem(JSON const& value) | ||
| 471 | return true; | 477 | return true; |
| 472 | } | 478 | } |
| 473 | 479 | ||
| 480 | +void | ||
| 481 | +QPDF::JSONReactor::setObjectDescription(QPDFObjectHandle& oh, JSON const& value) | ||
| 482 | +{ | ||
| 483 | + std::string description = this->is->getName(); | ||
| 484 | + if (!this->cur_object.empty()) { | ||
| 485 | + description += ", " + this->cur_object; | ||
| 486 | + } | ||
| 487 | + description += " at offset " + QUtil::uint_to_string(value.getStart()); | ||
| 488 | + oh.setObjectDescription(&this->pdf, description); | ||
| 489 | +} | ||
| 490 | + | ||
| 474 | QPDFObjectHandle | 491 | QPDFObjectHandle |
| 475 | QPDF::JSONReactor::makeObject(JSON const& value) | 492 | QPDF::JSONReactor::makeObject(JSON const& value) |
| 476 | { | 493 | { |
| @@ -515,12 +532,9 @@ QPDF::JSONReactor::makeObject(JSON const& value) | @@ -515,12 +532,9 @@ QPDF::JSONReactor::makeObject(JSON const& value) | ||
| 515 | "JSONReactor::makeObject didn't initialize the object"); | 532 | "JSONReactor::makeObject didn't initialize the object"); |
| 516 | } | 533 | } |
| 517 | 534 | ||
| 518 | - std::string description = this->is->getName(); | ||
| 519 | - if (!this->cur_object.empty()) { | ||
| 520 | - description += " " + this->cur_object + ","; | 535 | + if (!result.hasObjectDescription()) { |
| 536 | + setObjectDescription(result, value); | ||
| 521 | } | 537 | } |
| 522 | - description += " offset " + QUtil::uint_to_string(value.getStart()); | ||
| 523 | - result.setObjectDescription(&this->pdf, description); | ||
| 524 | return result; | 538 | return result; |
| 525 | } | 539 | } |
| 526 | 540 |
qpdf/qtest/qpdf-json.test
| @@ -202,6 +202,16 @@ foreach my $f (@update_files) { | @@ -202,6 +202,16 @@ foreach my $f (@update_files) { | ||
| 202 | {$td->FILE => "$f-updated.pdf"}); | 202 | {$td->FILE => "$f-updated.pdf"}); |
| 203 | } | 203 | } |
| 204 | 204 | ||
| 205 | +# Exercise object description | ||
| 206 | +$n_tests += 2; | ||
| 207 | +$td->runtest("json-input object description", | ||
| 208 | + {$td->COMMAND => "test_driver 89 manual-qpdf-json.json"}, | ||
| 209 | + {$td->FILE => "test-89.out", $td->EXIT_STATUS => 0}, | ||
| 210 | + $td->NORMALIZE_NEWLINES); | ||
| 211 | +$td->runtest("update-from-json object description", | ||
| 212 | + {$td->COMMAND => "test_driver 90 good13.pdf various-updates.json"}, | ||
| 213 | + {$td->FILE => "test-90.out", $td->EXIT_STATUS => 0}, | ||
| 214 | + $td->NORMALIZE_NEWLINES); | ||
| 205 | 215 | ||
| 206 | cleanup(); | 216 | cleanup(); |
| 207 | $td->report($n_tests); | 217 | $td->report($n_tests); |
qpdf/qtest/qpdf/qjson-object-not-dict.out
qpdf/qtest/qpdf/test-89.out
0 → 100644
| 1 | +WARNING: manual-qpdf-json.json, trailer at offset 1761: operation for array attempted on object of type dictionary: ignoring attempt to append item | ||
| 2 | +WARNING: manual-qpdf-json.json, obj:1 0 R at offset 1079: operation for array attempted on object of type dictionary: ignoring attempt to append item | ||
| 3 | +WARNING: manual-qpdf-json.json, obj:5 0 R at offset 1404: operation for dictionary attempted on object of type array: ignoring key replacement request | ||
| 4 | +WARNING: manual-qpdf-json.json, obj:5 0 R at offset 1416: operation for dictionary attempted on object of type name: ignoring key replacement request | ||
| 5 | +test 89 done |
qpdf/qtest/qpdf/test-90.out
0 → 100644
| 1 | +WARNING: various-updates.json, trailer at offset 580: operation for array attempted on object of type dictionary: ignoring attempt to append item | ||
| 2 | +WARNING: various-updates.json, obj:7 0 R at offset 171: operation for array attempted on object of type dictionary: ignoring attempt to append item | ||
| 3 | +WARNING: various-updates.json, obj:7 0 R at offset 283: operation for integer attempted on object of type array: returning 0 | ||
| 4 | +WARNING: good13.pdf, object 1 0 at offset 19: operation for array attempted on object of type dictionary: ignoring attempt to append item | ||
| 5 | +test 90 done |
qpdf/test_driver.cc
| @@ -3172,6 +3172,31 @@ test_88(QPDF& pdf, char const* arg2) | @@ -3172,6 +3172,31 @@ test_88(QPDF& pdf, char const* arg2) | ||
| 3172 | assert(arr2.eraseItemAndGet(50).isNull()); | 3172 | assert(arr2.eraseItemAndGet(50).isNull()); |
| 3173 | } | 3173 | } |
| 3174 | 3174 | ||
| 3175 | +static void | ||
| 3176 | +test_89(QPDF& pdf, char const* arg2) | ||
| 3177 | +{ | ||
| 3178 | + // Generate object warning with json-input. Crafted to work with | ||
| 3179 | + // manual-qpdf-json.json. | ||
| 3180 | + auto null = QPDFObjectHandle::newNull(); | ||
| 3181 | + pdf.getTrailer().appendItem(null); | ||
| 3182 | + pdf.getRoot().appendItem(null); | ||
| 3183 | + pdf.getObjectByID(5, 0).replaceKey("/X", null); | ||
| 3184 | + pdf.getObjectByID(5, 0).getArrayItem(0).replaceKey("/X", null); | ||
| 3185 | +} | ||
| 3186 | + | ||
| 3187 | +static void | ||
| 3188 | +test_90(QPDF& pdf, char const* arg2) | ||
| 3189 | +{ | ||
| 3190 | + // Generate object warning with update-from-json. Crafted to work | ||
| 3191 | + // with good13.pdf and various-updates.json. JSON file is arg2. | ||
| 3192 | + pdf.updateFromJSON(arg2); | ||
| 3193 | + pdf.getTrailer().appendItem(QPDFObjectHandle::newNull()); | ||
| 3194 | + pdf.getTrailer().getKey("/QTest").appendItem(QPDFObjectHandle::newNull()); | ||
| 3195 | + pdf.getTrailer().getKey("/QTest").getKey("/strings").getIntValue(); | ||
| 3196 | + // not from json | ||
| 3197 | + pdf.getRoot().appendItem(QPDFObjectHandle::newNull()); | ||
| 3198 | +} | ||
| 3199 | + | ||
| 3175 | void | 3200 | void |
| 3176 | runtest(int n, char const* filename1, char const* arg2) | 3201 | runtest(int n, char const* filename1, char const* arg2) |
| 3177 | { | 3202 | { |
| @@ -3235,6 +3260,8 @@ runtest(int n, char const* filename1, char const* arg2) | @@ -3235,6 +3260,8 @@ runtest(int n, char const* filename1, char const* arg2) | ||
| 3235 | (std::string(filename1) + ".pdf").c_str(), p, size); | 3260 | (std::string(filename1) + ".pdf").c_str(), p, size); |
| 3236 | } else if (ignore_filename.count(n)) { | 3261 | } else if (ignore_filename.count(n)) { |
| 3237 | // Ignore filename argument entirely | 3262 | // Ignore filename argument entirely |
| 3263 | + } else if (n == 89) { | ||
| 3264 | + pdf.createFromJSON(filename1); | ||
| 3238 | } else if (n % 2 == 0) { | 3265 | } else if (n % 2 == 0) { |
| 3239 | if (n % 4 == 0) { | 3266 | if (n % 4 == 0) { |
| 3240 | QTC::TC("qpdf", "exercise processFile(name)"); | 3267 | QTC::TC("qpdf", "exercise processFile(name)"); |
| @@ -3274,7 +3301,7 @@ runtest(int n, char const* filename1, char const* arg2) | @@ -3274,7 +3301,7 @@ runtest(int n, char const* filename1, char const* arg2) | ||
| 3274 | {76, test_76}, {77, test_77}, {78, test_78}, {79, test_79}, | 3301 | {76, test_76}, {77, test_77}, {78, test_78}, {79, test_79}, |
| 3275 | {80, test_80}, {81, test_81}, {82, test_82}, {83, test_83}, | 3302 | {80, test_80}, {81, test_81}, {82, test_82}, {83, test_83}, |
| 3276 | {84, test_84}, {85, test_85}, {86, test_86}, {87, test_87}, | 3303 | {84, test_84}, {85, test_85}, {86, test_86}, {87, test_87}, |
| 3277 | - {88, test_88}}; | 3304 | + {88, test_88}, {89, test_89}, {90, test_90}}; |
| 3278 | 3305 | ||
| 3279 | auto fn = test_functions.find(n); | 3306 | auto fn = test_functions.find(n); |
| 3280 | if (fn == test_functions.end()) { | 3307 | if (fn == test_functions.end()) { |