Commit a4fd4b91c6f7f732536bd113cd7b4e23a08ca599
1 parent
40f00122
Convert stream filtering errors to warnings
Showing
13 changed files
with
58 additions
and
32 deletions
ChangeLog
| 1 | 2017-07-27 Jay Berkenbilt <ejb@ql.org> | 1 | 2017-07-27 Jay Berkenbilt <ejb@ql.org> |
| 2 | 2 | ||
| 3 | + * Recover gracefully from streams that aren't filterable because | ||
| 4 | + the filter parameters are invalid in the stream dictionary or the | ||
| 5 | + dictionary itself is invalid. | ||
| 6 | + | ||
| 3 | * Significantly improve recoverability from invalid qpdf objects. | 7 | * Significantly improve recoverability from invalid qpdf objects. |
| 4 | Most conditions in basic object parsing that used to cause qpdf to | 8 | Most conditions in basic object parsing that used to cause qpdf to |
| 5 | exit are now warnings. There are still many more opportunities for | 9 | exit are now warnings. There are still many more opportunities for |
include/qpdf/QPDF.hh
| @@ -526,6 +526,7 @@ class QPDF | @@ -526,6 +526,7 @@ class QPDF | ||
| 526 | class Warner | 526 | class Warner |
| 527 | { | 527 | { |
| 528 | friend class QPDFObjectHandle; | 528 | friend class QPDFObjectHandle; |
| 529 | + friend class QPDF_Stream; | ||
| 529 | private: | 530 | private: |
| 530 | static void warn(QPDF* qpdf, QPDFExc const& e) | 531 | static void warn(QPDF* qpdf, QPDFExc const& e) |
| 531 | { | 532 | { |
libqpdf/QPDF_Stream.cc
| @@ -235,9 +235,10 @@ QPDF_Stream::filterable(std::vector<std::string>& filters, | @@ -235,9 +235,10 @@ QPDF_Stream::filterable(std::vector<std::string>& filters, | ||
| 235 | if (! filters_okay) | 235 | if (! filters_okay) |
| 236 | { | 236 | { |
| 237 | QTC::TC("qpdf", "QPDF_Stream invalid filter"); | 237 | QTC::TC("qpdf", "QPDF_Stream invalid filter"); |
| 238 | - throw QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), | ||
| 239 | - "", this->offset, | ||
| 240 | - "stream filter type is not name or array"); | 238 | + warn(QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), |
| 239 | + "", this->offset, | ||
| 240 | + "stream filter type is not name or array")); | ||
| 241 | + return false; | ||
| 241 | } | 242 | } |
| 242 | 243 | ||
| 243 | bool filterable = true; | 244 | bool filterable = true; |
| @@ -300,12 +301,16 @@ QPDF_Stream::filterable(std::vector<std::string>& filters, | @@ -300,12 +301,16 @@ QPDF_Stream::filterable(std::vector<std::string>& filters, | ||
| 300 | // /Filters was empty has been seen in the wild. | 301 | // /Filters was empty has been seen in the wild. |
| 301 | if ((filters.size() != 0) && (decode_parms.size() != filters.size())) | 302 | if ((filters.size() != 0) && (decode_parms.size() != filters.size())) |
| 302 | { | 303 | { |
| 303 | - // We should just issue a warning and treat this as not | ||
| 304 | - // filterable. | ||
| 305 | - throw QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), | ||
| 306 | - "", this->offset, | ||
| 307 | - "stream /DecodeParms length is" | ||
| 308 | - " inconsistent with filters"); | 304 | + warn(QPDFExc(qpdf_e_damaged_pdf, qpdf->getFilename(), |
| 305 | + "", this->offset, | ||
| 306 | + "stream /DecodeParms length is" | ||
| 307 | + " inconsistent with filters")); | ||
| 308 | + filterable = false; | ||
| 309 | + } | ||
| 310 | + | ||
| 311 | + if (! filterable) | ||
| 312 | + { | ||
| 313 | + return false; | ||
| 309 | } | 314 | } |
| 310 | 315 | ||
| 311 | for (unsigned int i = 0; i < filters.size(); ++i) | 316 | for (unsigned int i = 0; i < filters.size(); ++i) |
| @@ -454,7 +459,9 @@ QPDF_Stream::pipeStreamData(Pipeline* pipeline, bool filter, | @@ -454,7 +459,9 @@ QPDF_Stream::pipeStreamData(Pipeline* pipeline, bool filter, | ||
| 454 | else | 459 | else |
| 455 | { | 460 | { |
| 456 | QTC::TC("qpdf", "QPDF_Stream provider length mismatch"); | 461 | QTC::TC("qpdf", "QPDF_Stream provider length mismatch"); |
| 457 | - throw std::logic_error( | 462 | + // This would be caused by programmer error on the |
| 463 | + // part of a library user, not by invalid input data. | ||
| 464 | + throw std::runtime_error( | ||
| 458 | "stream data provider for " + | 465 | "stream data provider for " + |
| 459 | QUtil::int_to_string(this->objid) + " " + | 466 | QUtil::int_to_string(this->objid) + " " + |
| 460 | QUtil::int_to_string(this->generation) + | 467 | QUtil::int_to_string(this->generation) + |
| @@ -542,3 +549,9 @@ QPDF_Stream::replaceDict(QPDFObjectHandle new_dict) | @@ -542,3 +549,9 @@ QPDF_Stream::replaceDict(QPDFObjectHandle new_dict) | ||
| 542 | this->length = 0; | 549 | this->length = 0; |
| 543 | } | 550 | } |
| 544 | } | 551 | } |
| 552 | + | ||
| 553 | +void | ||
| 554 | +QPDF_Stream::warn(QPDFExc const& e) | ||
| 555 | +{ | ||
| 556 | + QPDF::Warner::warn(this->qpdf, e); | ||
| 557 | +} |
libqpdf/qpdf-c.cc
| @@ -646,6 +646,6 @@ QPDF_ERROR_CODE qpdf_write(qpdf_data qpdf) | @@ -646,6 +646,6 @@ QPDF_ERROR_CODE qpdf_write(qpdf_data qpdf) | ||
| 646 | { | 646 | { |
| 647 | QPDF_ERROR_CODE status = QPDF_SUCCESS; | 647 | QPDF_ERROR_CODE status = QPDF_SUCCESS; |
| 648 | status = trap_errors(qpdf, &call_write); | 648 | status = trap_errors(qpdf, &call_write); |
| 649 | - QTC::TC("qpdf", "qpdf-c called qpdf_write", status); | 649 | + QTC::TC("qpdf", "qpdf-c called qpdf_write", (status == 0) ? 0 : 1); |
| 650 | return status; | 650 | return status; |
| 651 | } | 651 | } |
libqpdf/qpdf/QPDF_Stream.hh
| @@ -52,6 +52,7 @@ class QPDF_Stream: public QPDFObject | @@ -52,6 +52,7 @@ class QPDF_Stream: public QPDFObject | ||
| 52 | int& predictor, int& columns, bool& early_code_change); | 52 | int& predictor, int& columns, bool& early_code_change); |
| 53 | bool filterable(std::vector<std::string>& filters, | 53 | bool filterable(std::vector<std::string>& filters, |
| 54 | int& predictor, int& columns, bool& early_code_change); | 54 | int& predictor, int& columns, bool& early_code_change); |
| 55 | + void warn(QPDFExc const& e); | ||
| 55 | 56 | ||
| 56 | QPDF* qpdf; | 57 | QPDF* qpdf; |
| 57 | int objid; | 58 | int objid; |
qpdf/qpdf.testcov
| @@ -139,7 +139,7 @@ qpdf-c called qpdf_set_preserve_encryption 0 | @@ -139,7 +139,7 @@ qpdf-c called qpdf_set_preserve_encryption 0 | ||
| 139 | qpdf-c called qpdf_set_r2_encryption_parameters 0 | 139 | qpdf-c called qpdf_set_r2_encryption_parameters 0 |
| 140 | qpdf-c called qpdf_set_r3_encryption_parameters 0 | 140 | qpdf-c called qpdf_set_r3_encryption_parameters 0 |
| 141 | qpdf-c called qpdf_set_linearization 0 | 141 | qpdf-c called qpdf_set_linearization 0 |
| 142 | -qpdf-c called qpdf_write 3 | 142 | +qpdf-c called qpdf_write 1 |
| 143 | qpdf-c called qpdf_allow_accessibility 0 | 143 | qpdf-c called qpdf_allow_accessibility 0 |
| 144 | qpdf-c called qpdf_allow_extract_all 0 | 144 | qpdf-c called qpdf_allow_extract_all 0 |
| 145 | qpdf-c called qpdf_allow_print_low_res 0 | 145 | qpdf-c called qpdf_allow_print_low_res 0 |
qpdf/qtest/qpdf.test
| @@ -787,7 +787,7 @@ my @badfiles = ("not a PDF file", # 1 | @@ -787,7 +787,7 @@ my @badfiles = ("not a PDF file", # 1 | ||
| 787 | "bad dictionary key", # 36 | 787 | "bad dictionary key", # 36 |
| 788 | ); | 788 | ); |
| 789 | 789 | ||
| 790 | -$n_tests += @badfiles + 4; | 790 | +$n_tests += @badfiles + 3; |
| 791 | 791 | ||
| 792 | # Test 6 contains errors in the free table consistency, but we no | 792 | # Test 6 contains errors in the free table consistency, but we no |
| 793 | # longer have any consistency check for this since it is not important | 793 | # longer have any consistency check for this since it is not important |
| @@ -796,7 +796,7 @@ $n_tests += @badfiles + 4; | @@ -796,7 +796,7 @@ $n_tests += @badfiles + 4; | ||
| 796 | # non-fatal. | 796 | # non-fatal. |
| 797 | my %badtest_overrides = (6 => 0, 12 => 0, 13 => 0, | 797 | my %badtest_overrides = (6 => 0, 12 => 0, 13 => 0, |
| 798 | 14 => 0, 15 => 0, 17 => 0, | 798 | 14 => 0, 15 => 0, 17 => 0, |
| 799 | - 28 => 0, 31 => 0, 36 => 0); | 799 | + 28 => 0, 30 => 0, 31 => 0, 36 => 0); |
| 800 | for (my $i = 1; $i <= scalar(@badfiles); ++$i) | 800 | for (my $i = 1; $i <= scalar(@badfiles); ++$i) |
| 801 | { | 801 | { |
| 802 | my $status = $badtest_overrides{$i}; | 802 | my $status = $badtest_overrides{$i}; |
| @@ -813,14 +813,9 @@ $td->runtest("C API: errors", | @@ -813,14 +813,9 @@ $td->runtest("C API: errors", | ||
| 813 | {$td->FILE => "c-read-errors.out", | 813 | {$td->FILE => "c-read-errors.out", |
| 814 | $td->EXIT_STATUS => 0}, | 814 | $td->EXIT_STATUS => 0}, |
| 815 | $td->NORMALIZE_NEWLINES); | 815 | $td->NORMALIZE_NEWLINES); |
| 816 | -$td->runtest("C API: errors writing", | ||
| 817 | - {$td->COMMAND => "qpdf-ctest 2 bad30.pdf '' a.pdf"}, | ||
| 818 | - {$td->FILE => "c-write-errors.out", | ||
| 819 | - $td->EXIT_STATUS => 0}, | ||
| 820 | - $td->NORMALIZE_NEWLINES); | ||
| 821 | -$td->runtest("C API: errors and warnings writing", | 816 | +$td->runtest("C API: warnings writing", |
| 822 | {$td->COMMAND => "qpdf-ctest 2 bad33.pdf '' a.pdf"}, | 817 | {$td->COMMAND => "qpdf-ctest 2 bad33.pdf '' a.pdf"}, |
| 823 | - {$td->FILE => "c-write-warnings-and-errors.out", | 818 | + {$td->FILE => "c-write-warnings.out", |
| 824 | $td->EXIT_STATUS => 0}, | 819 | $td->EXIT_STATUS => 0}, |
| 825 | $td->NORMALIZE_NEWLINES); | 820 | $td->NORMALIZE_NEWLINES); |
| 826 | $td->runtest("C API: no recovery", | 821 | $td->runtest("C API: no recovery", |
| @@ -840,7 +835,7 @@ $n_tests += @badfiles + 8; | @@ -840,7 +835,7 @@ $n_tests += @badfiles + 8; | ||
| 840 | # though in some cases it may. Acrobat Reader would not be able to | 835 | # though in some cases it may. Acrobat Reader would not be able to |
| 841 | # recover any of these files any better. | 836 | # recover any of these files any better. |
| 842 | my %recover_failures = (); | 837 | my %recover_failures = (); |
| 843 | -for (1, 7, 16, 18..21, 24, 29..30, 33, 35) | 838 | +for (1, 7, 16, 18..21, 24, 29, 35) |
| 844 | { | 839 | { |
| 845 | $recover_failures{$_} = 1; | 840 | $recover_failures{$_} = 1; |
| 846 | } | 841 | } |
qpdf/qtest/qpdf/bad30-recover.out
No preview for this file type
qpdf/qtest/qpdf/bad30.out
No preview for this file type
qpdf/qtest/qpdf/bad33-recover.out
No preview for this file type
qpdf/qtest/qpdf/c-write-errors.out deleted
qpdf/qtest/qpdf/c-write-warnings-and-errors.out renamed to qpdf/qtest/qpdf/c-write-warnings.out
| @@ -13,7 +13,12 @@ warning: bad33.pdf: Attempting to reconstruct cross-reference table | @@ -13,7 +13,12 @@ warning: bad33.pdf: Attempting to reconstruct cross-reference table | ||
| 13 | file: bad33.pdf | 13 | file: bad33.pdf |
| 14 | pos : 0 | 14 | pos : 0 |
| 15 | text: Attempting to reconstruct cross-reference table | 15 | text: Attempting to reconstruct cross-reference table |
| 16 | -error: bad33.pdf (file position 629): stream filter type is not name or array | 16 | +warning: bad33.pdf (file position 629): stream filter type is not name or array |
| 17 | + code: 5 | ||
| 18 | + file: bad33.pdf | ||
| 19 | + pos : 629 | ||
| 20 | + text: stream filter type is not name or array | ||
| 21 | +warning: bad33.pdf (file position 629): stream filter type is not name or array | ||
| 17 | code: 5 | 22 | code: 5 |
| 18 | file: bad33.pdf | 23 | file: bad33.pdf |
| 19 | pos : 629 | 24 | pos : 629 |
qpdf/test_driver.cc
| @@ -567,7 +567,7 @@ void runtest(int n, char const* filename1, char const* arg2) | @@ -567,7 +567,7 @@ void runtest(int n, char const* filename1, char const* arg2) | ||
| 567 | qstream.getStreamData(); | 567 | qstream.getStreamData(); |
| 568 | std::cout << "oops -- getStreamData didn't throw" << std::endl; | 568 | std::cout << "oops -- getStreamData didn't throw" << std::endl; |
| 569 | } | 569 | } |
| 570 | - catch (std::logic_error const& e) | 570 | + catch (std::exception const& e) |
| 571 | { | 571 | { |
| 572 | std::cout << "exception: " << e.what() << std::endl; | 572 | std::cout << "exception: " << e.what() << std::endl; |
| 573 | } | 573 | } |