diff --git a/ChangeLog b/ChangeLog index 81812449..4a59b6a7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2020-10-21 Jay Berkenbilt + * Bug fix: properly handle copying foreign streams that have + indirect /Filter or /DecodeParms keys when stream data has been + replaced. The circumstances leading to this bug are very unusual + but would cause qpdf to either generate an internal error or some + other kind of warning situation if it would occur. Fixes #449. + * Qpdf's build and CI has been migrated from Azure Pipelines (Azure Devops) to GitHub Actions. diff --git a/TODO b/TODO index 32cbd901..3f4e0976 100644 --- a/TODO +++ b/TODO @@ -7,7 +7,6 @@ Candidates for upcoming release * Open "next" issues * bugs * #473: zsh completion with directories - * #449: internal error with case to reproduce (from pikepdf) * #444: concatenated stream/whitespace bug * Non-bugs * #446: recognize edited QDF files diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 1cbef133..3b6d70ee 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -2313,7 +2313,8 @@ QPDF::reserveObjects(QPDFObjectHandle foreign, ObjCopier& obj_copier, if (foreign.isReserved()) { throw std::logic_error( - "QPDF: attempting to copy a foreign reserved object"); + "QPDF: attempting to copy a foreign reserved object: " + + QUtil::int_to_string(foreign.getObjectID())); } if (foreign.isPagesObject()) @@ -2486,6 +2487,16 @@ QPDF::replaceForeignIndirectObjects( } PointerHolder stream_buffer = stream->getStreamDataBuffer(); + // Note: at this stage, dictionary keys may still be reserved. + // We have to handle that explicitly if we access anything. + auto get_as_direct = [&old_dict] (std::string const& key) { + QPDFObjectHandle obj = old_dict.getKey(key); + obj.makeDirect(); + return obj; + }; + + QPDFObjectHandle filter = get_as_direct("/Filter"); + QPDFObjectHandle decode_parms = get_as_direct("/DecodeParms"); if ((foreign_stream_qpdf->m->immediate_copy_from) && (stream_buffer.getPointer() == 0)) { @@ -2495,8 +2506,7 @@ QPDF::replaceForeignIndirectObjects( // have to keep duplicating the memory. QTC::TC("qpdf", "QPDF immediate copy stream data"); foreign.replaceStreamData(foreign.getRawStreamData(), - dict.getKey("/Filter"), - dict.getKey("/DecodeParms")); + filter, decode_parms); stream_buffer = stream->getStreamDataBuffer(); } PointerHolder stream_provider = @@ -2504,9 +2514,7 @@ QPDF::replaceForeignIndirectObjects( if (stream_buffer.getPointer()) { QTC::TC("qpdf", "QPDF copy foreign stream with buffer"); - result.replaceStreamData(stream_buffer, - dict.getKey("/Filter"), - dict.getKey("/DecodeParms")); + result.replaceStreamData(stream_buffer, filter, decode_parms); } else if (stream_provider.getPointer()) { @@ -2515,8 +2523,7 @@ QPDF::replaceForeignIndirectObjects( this->m->copied_stream_data_provider->registerForeignStream( local_og, foreign); result.replaceStreamData(this->m->copied_streams, - dict.getKey("/Filter"), - dict.getKey("/DecodeParms")); + filter, decode_parms); } else { @@ -2534,8 +2541,7 @@ QPDF::replaceForeignIndirectObjects( this->m->copied_stream_data_provider->registerForeignStream( local_og, foreign_stream_data); result.replaceStreamData(this->m->copied_streams, - dict.getKey("/Filter"), - dict.getKey("/DecodeParms")); + filter, decode_parms); } } else diff --git a/make/msvc.mk b/make/msvc.mk index c8bc57bc..cfeaa45a 100644 --- a/make/msvc.mk +++ b/make/msvc.mk @@ -66,7 +66,7 @@ endef # Usage: $(call makelib,objs,library,ldflags,libs,current,revision,age) define makelib cl -nologo -O2 -Zi -Gy -EHsc -MD -LD -Fe$(basename $(2))$(shell expr $(5) - $(7)).dll $(1) \ - -link -SUBSYSTEM:CONSOLE,5.01 -incremental:no \ + -link -SUBSYSTEM:CONSOLE -incremental:no \ $(foreach L,$(subst -L,,$(3)),-LIBPATH:$(L)) \ $(foreach L,$(subst -l,,$(4)),$(L).lib) if [ -f $(basename $(2))$(shell expr $(5) - $(7)).dll.manifest ]; then \ @@ -81,7 +81,7 @@ endef define makebin cl -nologo -O2 -Zi -Gy -EHsc -MD $(1) \ $(if $(5),$(5),$(WINDOWS_MAIN_XLINK_FLAGS)) \ - -link -SUBSYSTEM:CONSOLE,5.01 -incremental:no -OUT:$(2) \ + -link -SUBSYSTEM:CONSOLE -incremental:no -OUT:$(2) \ $(foreach L,$(subst -L,,$(3)),-LIBPATH:$(L)) \ $(foreach L,$(subst -l,,$(4)),$(L).lib) if [ -f $(2).manifest ]; then \ diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index c2a38fa3..a0ff2a57 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -2417,7 +2417,7 @@ $td->runtest("check output", show_ntests(); # ---------- $td->notify("--- Copy Foreign Objects ---"); -$n_tests += 7; +$n_tests += 10; foreach my $d ([25, 1], [26, 2], [27, 3]) { @@ -2438,6 +2438,19 @@ $td->runtest("copy objects error", {$td->FILE => "copy-foreign-objects-errors.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); + +$td->runtest("indirect filters", + {$td->COMMAND => "test_driver 69 indirect-filter.pdf"}, + {$td->STRING => "test 69 done\n", $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); +foreach my $i (0, 1) +{ + $td->runtest("check output", + {$td->FILE => "auto-$i.pdf"}, + {$td->FILE => "indirect-filter-out-$i.pdf"}); +} + + show_ntests(); # ---------- $td->notify("--- Error Condition Tests ---"); diff --git a/qpdf/qtest/qpdf/indirect-filter-out-0.pdf b/qpdf/qtest/qpdf/indirect-filter-out-0.pdf new file mode 100644 index 00000000..79e80601 Binary files /dev/null and b/qpdf/qtest/qpdf/indirect-filter-out-0.pdf differ diff --git a/qpdf/qtest/qpdf/indirect-filter-out-1.pdf b/qpdf/qtest/qpdf/indirect-filter-out-1.pdf new file mode 100644 index 00000000..0e8aac8e Binary files /dev/null and b/qpdf/qtest/qpdf/indirect-filter-out-1.pdf differ diff --git a/qpdf/qtest/qpdf/indirect-filter.pdf b/qpdf/qtest/qpdf/indirect-filter.pdf new file mode 100644 index 00000000..9717bb89 Binary files /dev/null and b/qpdf/qtest/qpdf/indirect-filter.pdf differ diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 07445dea..167c509b 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -2195,6 +2195,22 @@ void runtest(int n, char const* filename1, char const* arg2) std::cout << "raw stream data okay" << std::endl; } } + else if (n == 69) + { + pdf.setImmediateCopyFrom(true); + auto pages = pdf.getAllPages(); + for (size_t i = 0; i < pages.size(); ++i) + { + QPDF out; + out.emptyPDF(); + out.addPage(pages.at(i), false); + std::string outname = std::string("auto-") + + QUtil::uint_to_string(i) + ".pdf"; + QPDFWriter w(out, outname.c_str()); + w.setStaticID(true); + w.write(); + } + } else { throw std::runtime_error(std::string("invalid test ") +