OpenSystemsDevelopment / qpdf

Commit 956c8f643219778c445d7062d1d0e7e3b96c7676

Authored by Jay Berkenbilt
1 parent ad96e1ad

Obscure bug fix copying foreign streams in special cases (fixes #449) 

Specifically, if a stream had its stream data replaced and had
indirect /Filter or /DecodeParms, it would result in non-silent loss
of data and/or internal error.
Inline Side-by-side
Showing 9 changed files with 54 additions and 14 deletions
ChangeLog
  View file @956c8f6
1 1 2020-10-21 Jay Berkenbilt <ejb@ql.org>
2 2  
  3 + * Bug fix: properly handle copying foreign streams that have
  4 + indirect /Filter or /DecodeParms keys when stream data has been
  5 + replaced. The circumstances leading to this bug are very unusual
  6 + but would cause qpdf to either generate an internal error or some
  7 + other kind of warning situation if it would occur. Fixes #449.
  8 +
3 9 * Qpdf's build and CI has been migrated from Azure Pipelines
4 10 (Azure Devops) to GitHub Actions.
5 11  
... ...
... ... @@ -7,7 +7,6 @@ Candidates for upcoming release
7 7 * Open "next" issues
8 8 * bugs
9 9 * #473: zsh completion with directories
10   - * #449: internal error with case to reproduce (from pikepdf)
11 10 * #444: concatenated stream/whitespace bug
12 11 * Non-bugs
13 12 * #446: recognize edited QDF files
... ...
libqpdf/QPDF.cc
  View file @956c8f6
... ... @@ -2313,7 +2313,8 @@ QPDF::reserveObjects(QPDFObjectHandle foreign, ObjCopier&amp; obj_copier,
2313 2313 if (foreign.isReserved())
2314 2314 {
2315 2315 throw std::logic_error(
2316   - "QPDF: attempting to copy a foreign reserved object");
  2316 + "QPDF: attempting to copy a foreign reserved object: " +
  2317 + QUtil::int_to_string(foreign.getObjectID()));
2317 2318 }
2318 2319  
2319 2320 if (foreign.isPagesObject())
... ... @@ -2486,6 +2487,16 @@ QPDF::replaceForeignIndirectObjects(
2486 2487 }
2487 2488 PointerHolder<Buffer> stream_buffer =
2488 2489 stream->getStreamDataBuffer();
  2490 + // Note: at this stage, dictionary keys may still be reserved.
  2491 + // We have to handle that explicitly if we access anything.
  2492 + auto get_as_direct = [&old_dict] (std::string const& key) {
  2493 + QPDFObjectHandle obj = old_dict.getKey(key);
  2494 + obj.makeDirect();
  2495 + return obj;
  2496 + };
  2497 +
  2498 + QPDFObjectHandle filter = get_as_direct("/Filter");
  2499 + QPDFObjectHandle decode_parms = get_as_direct("/DecodeParms");
2489 2500 if ((foreign_stream_qpdf->m->immediate_copy_from) &&
2490 2501 (stream_buffer.getPointer() == 0))
2491 2502 {
... ... @@ -2495,8 +2506,7 @@ QPDF::replaceForeignIndirectObjects(
2495 2506 // have to keep duplicating the memory.
2496 2507 QTC::TC("qpdf", "QPDF immediate copy stream data");
2497 2508 foreign.replaceStreamData(foreign.getRawStreamData(),
2498   - dict.getKey("/Filter"),
2499   - dict.getKey("/DecodeParms"));
  2509 + filter, decode_parms);
2500 2510 stream_buffer = stream->getStreamDataBuffer();
2501 2511 }
2502 2512 PointerHolder<QPDFObjectHandle::StreamDataProvider> stream_provider =
... ... @@ -2504,9 +2514,7 @@ QPDF::replaceForeignIndirectObjects(
2504 2514 if (stream_buffer.getPointer())
2505 2515 {
2506 2516 QTC::TC("qpdf", "QPDF copy foreign stream with buffer");
2507   - result.replaceStreamData(stream_buffer,
2508   - dict.getKey("/Filter"),
2509   - dict.getKey("/DecodeParms"));
  2517 + result.replaceStreamData(stream_buffer, filter, decode_parms);
2510 2518 }
2511 2519 else if (stream_provider.getPointer())
2512 2520 {
... ... @@ -2515,8 +2523,7 @@ QPDF::replaceForeignIndirectObjects(
2515 2523 this->m->copied_stream_data_provider->registerForeignStream(
2516 2524 local_og, foreign);
2517 2525 result.replaceStreamData(this->m->copied_streams,
2518   - dict.getKey("/Filter"),
2519   - dict.getKey("/DecodeParms"));
  2526 + filter, decode_parms);
2520 2527 }
2521 2528 else
2522 2529 {
... ... @@ -2534,8 +2541,7 @@ QPDF::replaceForeignIndirectObjects(
2534 2541 this->m->copied_stream_data_provider->registerForeignStream(
2535 2542 local_og, foreign_stream_data);
2536 2543 result.replaceStreamData(this->m->copied_streams,
2537   - dict.getKey("/Filter"),
2538   - dict.getKey("/DecodeParms"));
  2544 + filter, decode_parms);
2539 2545 }
2540 2546 }
2541 2547 else
... ...
make/msvc.mk
  View file @956c8f6
... ... @@ -66,7 +66,7 @@ endef
66 66 # Usage: $(call makelib,objs,library,ldflags,libs,current,revision,age)
67 67 define makelib
68 68 cl -nologo -O2 -Zi -Gy -EHsc -MD -LD -Fe$(basename $(2))$(shell expr $(5) - $(7)).dll $(1) \
69   - -link -SUBSYSTEM:CONSOLE,5.01 -incremental:no \
  69 + -link -SUBSYSTEM:CONSOLE -incremental:no \
70 70 $(foreach L,$(subst -L,,$(3)),-LIBPATH:$(L)) \
71 71 $(foreach L,$(subst -l,,$(4)),$(L).lib)
72 72 if [ -f $(basename $(2))$(shell expr $(5) - $(7)).dll.manifest ]; then \
... ... @@ -81,7 +81,7 @@ endef
81 81 define makebin
82 82 cl -nologo -O2 -Zi -Gy -EHsc -MD $(1) \
83 83 $(if $(5),$(5),$(WINDOWS_MAIN_XLINK_FLAGS)) \
84   - -link -SUBSYSTEM:CONSOLE,5.01 -incremental:no -OUT:$(2) \
  84 + -link -SUBSYSTEM:CONSOLE -incremental:no -OUT:$(2) \
85 85 $(foreach L,$(subst -L,,$(3)),-LIBPATH:$(L)) \
86 86 $(foreach L,$(subst -l,,$(4)),$(L).lib)
87 87 if [ -f $(2).manifest ]; then \
... ...
qpdf/qtest/qpdf.test
  View file @956c8f6
... ... @@ -2417,7 +2417,7 @@ $td-&gt;runtest(&quot;check output&quot;,
2417 2417 show_ntests();
2418 2418 # ----------
2419 2419 $td->notify("--- Copy Foreign Objects ---");
2420   -$n_tests += 7;
  2420 +$n_tests += 10;
2421 2421  
2422 2422 foreach my $d ([25, 1], [26, 2], [27, 3])
2423 2423 {
... ... @@ -2438,6 +2438,19 @@ $td-&gt;runtest(&quot;copy objects error&quot;,
2438 2438 {$td->FILE => "copy-foreign-objects-errors.out",
2439 2439 $td->EXIT_STATUS => 0},
2440 2440 $td->NORMALIZE_NEWLINES);
  2441 +
  2442 +$td->runtest("indirect filters",
  2443 + {$td->COMMAND => "test_driver 69 indirect-filter.pdf"},
  2444 + {$td->STRING => "test 69 done\n", $td->EXIT_STATUS => 0},
  2445 + $td->NORMALIZE_NEWLINES);
  2446 +foreach my $i (0, 1)
  2447 +{
  2448 + $td->runtest("check output",
  2449 + {$td->FILE => "auto-$i.pdf"},
  2450 + {$td->FILE => "indirect-filter-out-$i.pdf"});
  2451 +}
  2452 +
  2453 +
2441 2454 show_ntests();
2442 2455 # ----------
2443 2456 $td->notify("--- Error Condition Tests ---");
... ...
qpdf/qtest/qpdf/indirect-filter-out-0.pdf 0 → 100644
View file @956c8f6
No preview for this file type
qpdf/qtest/qpdf/indirect-filter-out-1.pdf 0 → 100644
View file @956c8f6
No preview for this file type
qpdf/qtest/qpdf/indirect-filter.pdf 0 → 100644
View file @956c8f6
No preview for this file type
qpdf/test_driver.cc
  View file @956c8f6
... ... @@ -2195,6 +2195,22 @@ void runtest(int n, char const* filename1, char const* arg2)
2195 2195 std::cout << "raw stream data okay" << std::endl;
2196 2196 }
2197 2197 }
  2198 + else if (n == 69)
  2199 + {
  2200 + pdf.setImmediateCopyFrom(true);
  2201 + auto pages = pdf.getAllPages();
  2202 + for (size_t i = 0; i < pages.size(); ++i)
  2203 + {
  2204 + QPDF out;
  2205 + out.emptyPDF();
  2206 + out.addPage(pages.at(i), false);
  2207 + std::string outname = std::string("auto-") +
  2208 + QUtil::uint_to_string(i) + ".pdf";
  2209 + QPDFWriter w(out, outname.c_str());
  2210 + w.setStaticID(true);
  2211 + w.write();
  2212 + }
  2213 + }
2198 2214 else
2199 2215 {
2200 2216 throw std::runtime_error(std::string("invalid test ") +
... ...