From a4fd4b91c6f7f732536bd113cd7b4e23a08ca599 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Thu, 27 Jul 2017 18:18:18 -0400 Subject: [PATCH] Convert stream filtering errors to warnings --- ChangeLog | 4 +++ include/qpdf/QPDF.hh | 1 + libqpdf/QPDF_Stream.cc | 33 +++++++++++++------ libqpdf/qpdf-c.cc | 2 +- libqpdf/qpdf/QPDF_Stream.hh | 1 + qpdf/qpdf.testcov | 2 +- qpdf/qtest/qpdf.test | 15 +++------ qpdf/qtest/qpdf/bad30-recover.out | 6 +++- qpdf/qtest/qpdf/bad30.out | 6 +++- qpdf/qtest/qpdf/bad33-recover.out | 6 +++- qpdf/qtest/qpdf/c-write-errors.out | 5 --- ...gs-and-errors.out => c-write-warnings.out} | 7 +++- qpdf/test_driver.cc | 2 +- 13 files changed, 58 insertions(+), 32 deletions(-) delete mode 100644 qpdf/qtest/qpdf/c-write-errors.out rename qpdf/qtest/qpdf/{c-write-warnings-and-errors.out => c-write-warnings.out} (65%) diff --git a/ChangeLog b/ChangeLog index bff50287..119a4c6c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2017-07-27 Jay Berkenbilt + * Recover gracefully from streams that aren't filterable because + the filter parameters are invalid in the stream dictionary or the + dictionary itself is invalid. + * Significantly improve recoverability from invalid qpdf objects. Most conditions in basic object parsing that used to cause qpdf to exit are now warnings. There are still many more opportunities for diff --git a/include/qpdf/QPDF.hh b/include/qpdf/QPDF.hh index 0788316d..18a6851f 100644 --- a/include/qpdf/QPDF.hh +++ b/include/qpdf/QPDF.hh @@ -526,6 +526,7 @@ class QPDF class Warner { friend class QPDFObjectHandle; + friend class QPDF_Stream; private: static void warn(QPDF* qpdf, QPDFExc const& e) { diff --git a/libqpdf/QPDF_Stream.cc b/libqpdf/QPDF_Stream.cc index 5704e83f..b4d14441 100644 --- a/libqpdf/QPDF_Stream.cc +++ b/libqpdf/QPDF_Stream.cc @@ -235,9 +235,10 @@ QPDF_Stream::filterable(std::vector& filters, if (! filters_okay) { QTC::TC("qpdf", "QPDF_Stream invalid filter"); - throw QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), - "", this->offset, - "stream filter type is not name or array"); + warn(QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), + "", this->offset, + "stream filter type is not name or array")); + return false; } bool filterable = true; @@ -300,12 +301,16 @@ QPDF_Stream::filterable(std::vector& filters, // /Filters was empty has been seen in the wild. if ((filters.size() != 0) && (decode_parms.size() != filters.size())) { - // We should just issue a warning and treat this as not - // filterable. - throw QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), - "", this->offset, - "stream /DecodeParms length is" - " inconsistent with filters"); + warn(QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), + "", this->offset, + "stream /DecodeParms length is" + " inconsistent with filters")); + filterable = false; + } + + if (! filterable) + { + return false; } for (unsigned int i = 0; i < filters.size(); ++i) @@ -454,7 +459,9 @@ QPDF_Stream::pipeStreamData(Pipeline* pipeline, bool filter, else { QTC::TC("qpdf", "QPDF_Stream provider length mismatch"); - throw std::logic_error( + // This would be caused by programmer error on the + // part of a library user, not by invalid input data. + throw std::runtime_error( "stream data provider for " + QUtil::int_to_string(this->objid) + " " + QUtil::int_to_string(this->generation) + @@ -542,3 +549,9 @@ QPDF_Stream::replaceDict(QPDFObjectHandle new_dict) this->length = 0; } } + +void +QPDF_Stream::warn(QPDFExc const& e) +{ + QPDF::Warner::warn(this->qpdf, e); +} diff --git a/libqpdf/qpdf-c.cc b/libqpdf/qpdf-c.cc index 975e9707..797476c1 100644 --- a/libqpdf/qpdf-c.cc +++ b/libqpdf/qpdf-c.cc @@ -646,6 +646,6 @@ QPDF_ERROR_CODE qpdf_write(qpdf_data qpdf) { QPDF_ERROR_CODE status = QPDF_SUCCESS; status = trap_errors(qpdf, &call_write); - QTC::TC("qpdf", "qpdf-c called qpdf_write", status); + QTC::TC("qpdf", "qpdf-c called qpdf_write", (status == 0) ? 0 : 1); return status; } diff --git a/libqpdf/qpdf/QPDF_Stream.hh b/libqpdf/qpdf/QPDF_Stream.hh index 5a5d555b..fa405d70 100644 --- a/libqpdf/qpdf/QPDF_Stream.hh +++ b/libqpdf/qpdf/QPDF_Stream.hh @@ -52,6 +52,7 @@ class QPDF_Stream: public QPDFObject int& predictor, int& columns, bool& early_code_change); bool filterable(std::vector& filters, int& predictor, int& columns, bool& early_code_change); + void warn(QPDFExc const& e); QPDF* qpdf; int objid; diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 20859984..35a7ba78 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -139,7 +139,7 @@ qpdf-c called qpdf_set_preserve_encryption 0 qpdf-c called qpdf_set_r2_encryption_parameters 0 qpdf-c called qpdf_set_r3_encryption_parameters 0 qpdf-c called qpdf_set_linearization 0 -qpdf-c called qpdf_write 3 +qpdf-c called qpdf_write 1 qpdf-c called qpdf_allow_accessibility 0 qpdf-c called qpdf_allow_extract_all 0 qpdf-c called qpdf_allow_print_low_res 0 diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index d69ccdc2..8378e7b3 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -787,7 +787,7 @@ my @badfiles = ("not a PDF file", # 1 "bad dictionary key", # 36 ); -$n_tests += @badfiles + 4; +$n_tests += @badfiles + 3; # Test 6 contains errors in the free table consistency, but we no # longer have any consistency check for this since it is not important @@ -796,7 +796,7 @@ $n_tests += @badfiles + 4; # non-fatal. my %badtest_overrides = (6 => 0, 12 => 0, 13 => 0, 14 => 0, 15 => 0, 17 => 0, - 28 => 0, 31 => 0, 36 => 0); + 28 => 0, 30 => 0, 31 => 0, 36 => 0); for (my $i = 1; $i <= scalar(@badfiles); ++$i) { my $status = $badtest_overrides{$i}; @@ -813,14 +813,9 @@ $td->runtest("C API: errors", {$td->FILE => "c-read-errors.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); -$td->runtest("C API: errors writing", - {$td->COMMAND => "qpdf-ctest 2 bad30.pdf '' a.pdf"}, - {$td->FILE => "c-write-errors.out", - $td->EXIT_STATUS => 0}, - $td->NORMALIZE_NEWLINES); -$td->runtest("C API: errors and warnings writing", +$td->runtest("C API: warnings writing", {$td->COMMAND => "qpdf-ctest 2 bad33.pdf '' a.pdf"}, - {$td->FILE => "c-write-warnings-and-errors.out", + {$td->FILE => "c-write-warnings.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); $td->runtest("C API: no recovery", @@ -840,7 +835,7 @@ $n_tests += @badfiles + 8; # though in some cases it may. Acrobat Reader would not be able to # recover any of these files any better. my %recover_failures = (); -for (1, 7, 16, 18..21, 24, 29..30, 33, 35) +for (1, 7, 16, 18..21, 24, 29, 35) { $recover_failures{$_} = 1; } diff --git a/qpdf/qtest/qpdf/bad30-recover.out b/qpdf/qtest/qpdf/bad30-recover.out index 5e228a58..dc59da20 100644 --- a/qpdf/qtest/qpdf/bad30-recover.out +++ b/qpdf/qtest/qpdf/bad30-recover.out @@ -3,4 +3,8 @@ Raw stream data: x%11 b;t4| wXID8G>rQu O E:IWPlII)rp4~;As/҅jcszT.?u<*6 Uncompressed stream data: -bad30.pdf (file position 629): stream filter type is not name or array +WARNING: bad30.pdf (file position 629): stream filter type is not name or array +Stream data is not filterable. +unparse: 7 0 R +unparseResolved: 7 0 R +test 1 done diff --git a/qpdf/qtest/qpdf/bad30.out b/qpdf/qtest/qpdf/bad30.out index 5e228a58..fc0780c6 100644 --- a/qpdf/qtest/qpdf/bad30.out +++ b/qpdf/qtest/qpdf/bad30.out @@ -3,4 +3,8 @@ Raw stream data: x%11 b;t4| wXID8G>rQu O E:IWPlII)rp4~;As/҅jcszT.?u<*6 Uncompressed stream data: -bad30.pdf (file position 629): stream filter type is not name or array +WARNING: bad30.pdf (file position 629): stream filter type is not name or array +Stream data is not filterable. +unparse: 7 0 R +unparseResolved: 7 0 R +test 0 done diff --git a/qpdf/qtest/qpdf/bad33-recover.out b/qpdf/qtest/qpdf/bad33-recover.out index 7cb645a7..2269cd41 100644 --- a/qpdf/qtest/qpdf/bad33-recover.out +++ b/qpdf/qtest/qpdf/bad33-recover.out @@ -6,4 +6,8 @@ WARNING: bad33.pdf: Attempting to reconstruct cross-reference table Raw stream data: x%11 b;t4| wXID8G>rQu O E:IWPlII)rp4~;As/҅jcszT.?u<*6 Uncompressed stream data: -bad33.pdf (file position 629): stream filter type is not name or array +WARNING: bad33.pdf (file position 629): stream filter type is not name or array +Stream data is not filterable. +unparse: 7 0 R +unparseResolved: 7 0 R +test 1 done diff --git a/qpdf/qtest/qpdf/c-write-errors.out b/qpdf/qtest/qpdf/c-write-errors.out deleted file mode 100644 index 211cfdf9..00000000 --- a/qpdf/qtest/qpdf/c-write-errors.out +++ /dev/null @@ -1,5 +0,0 @@ -error: bad30.pdf (file position 629): stream filter type is not name or array - code: 5 - file: bad30.pdf - pos : 629 - text: stream filter type is not name or array diff --git a/qpdf/qtest/qpdf/c-write-warnings-and-errors.out b/qpdf/qtest/qpdf/c-write-warnings.out similarity index 65% rename from qpdf/qtest/qpdf/c-write-warnings-and-errors.out rename to qpdf/qtest/qpdf/c-write-warnings.out index d486d599..bc412658 100644 --- a/qpdf/qtest/qpdf/c-write-warnings-and-errors.out +++ b/qpdf/qtest/qpdf/c-write-warnings.out @@ -13,7 +13,12 @@ warning: bad33.pdf: Attempting to reconstruct cross-reference table file: bad33.pdf pos : 0 text: Attempting to reconstruct cross-reference table -error: bad33.pdf (file position 629): stream filter type is not name or array +warning: bad33.pdf (file position 629): stream filter type is not name or array + code: 5 + file: bad33.pdf + pos : 629 + text: stream filter type is not name or array +warning: bad33.pdf (file position 629): stream filter type is not name or array code: 5 file: bad33.pdf pos : 629 diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index a40df34e..d1db805e 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -567,7 +567,7 @@ void runtest(int n, char const* filename1, char const* arg2) qstream.getStreamData(); std::cout << "oops -- getStreamData didn't throw" << std::endl; } - catch (std::logic_error const& e) + catch (std::exception const& e) { std::cout << "exception: " << e.what() << std::endl; }