From 3608afd5c528b7a9d95d227cb6c4f33d303fcfcd Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Thu, 20 Jun 2019 13:04:57 -0400 Subject: [PATCH] Add new integer accessors to QPDFObjectHandle --- ChangeLog | 4 ++ include/qpdf/QPDFObjectHandle.hh | 6 +++ libqpdf/QPDFObjectHandle.cc | 77 ++++++++++++++++++++++++++++++++ qpdf/qpdf.testcov | 5 +++ qpdf/qtest/qpdf.test | 7 ++- qpdf/test_driver.cc | 48 ++++++++++++++++++-- 6 files changed, 142 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index d51a92c5..bcd21011 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2019-06-20 Jay Berkenbilt + * Add methods to QPDFObjectHandle to return the value of Integer + objects as int and unsigned int with range checking and fallback + behavior to avoid silent underflow/overflow conditions. + * Add functions to QUtil to convert unsigned integers to strings, avoiding implicit conversion between unsigned and signed integer types. diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 978cddff..0039f8ee 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -491,6 +491,12 @@ class QPDFObjectHandle // Methods for integer objects QPDF_DLL long long getIntValue(); + QPDF_DLL + int getIntValueAsInt(); + QPDF_DLL + unsigned long long getUIntValue(); + QPDF_DLL + unsigned int getUIntValueAsUInt(); // Methods for real objects QPDF_DLL diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 2cffb166..b3f7daec 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -26,6 +26,7 @@ #include #include #include +#include class TerminateParsing { @@ -406,6 +407,82 @@ QPDFObjectHandle::getIntValue() } } +int +QPDFObjectHandle::getIntValueAsInt() +{ + int result = 0; + long long v = getIntValue(); + if (v < INT_MIN) + { + QTC::TC("qpdf", "QPDFObjectHandle int returning INT_MIN"); + warnIfPossible( + "requested value of integer is too small; returning INT_MIN", + false); + result = INT_MIN; + } + else if (v > INT_MAX) + { + QTC::TC("qpdf", "QPDFObjectHandle int returning INT_MAX"); + warnIfPossible( + "requested value of integer is too big; returning INT_MAX", + false); + result = INT_MAX; + } + else + { + result = static_cast(v); + } + return result; +} + +unsigned long long +QPDFObjectHandle::getUIntValue() +{ + unsigned long long result = 0; + long long v = getIntValue(); + if (v < 0) + { + QTC::TC("qpdf", "QPDFObjectHandle uint returning 0"); + warnIfPossible( + "unsigned value request for negative number; returning 0", + false); + } + else + { + result = static_cast(v); + } + return result; +} + +unsigned int +QPDFObjectHandle::getUIntValueAsUInt() +{ + unsigned int result = 0; + long long v = getIntValue(); + if (v < 0) + { + QTC::TC("qpdf", "QPDFObjectHandle uint uint returning 0"); + warnIfPossible( + "unsigned integer value request for negative number; returning 0", + false); + result = 0; + } + else if (v > UINT_MAX) + { + QTC::TC("qpdf", "QPDFObjectHandle uint returning UINT_MAX"); + warnIfPossible( + "requested value of unsigned integer is too big;" + " returning UINT_MAX", + false); + result = UINT_MAX; + } + else + { + result = static_cast(v); + } + return result; +} + // Real accessors std::string diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index f6a1cedf..b0d15984 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -440,3 +440,8 @@ QPDFPageObjectHelper keep inline image 0 qpdf image optimize colorspace 0 qpdf image optimize bits per component 0 QPDFWriter remove empty DecodeParms 0 +QPDFObjectHandle uint returning 0 0 +QPDFObjectHandle int returning INT_MIN 0 +QPDFObjectHandle int returning INT_MAX 0 +QPDFObjectHandle uint returning UINT_MAX 0 +QPDFObjectHandle uint uint returning 0 0 diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index eb2af1a4..76f9d8aa 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -2286,7 +2286,7 @@ my @badfiles = ("not a PDF file", # 1 "bad dictionary key", # 36 ); -$n_tests += @badfiles + 5; +$n_tests += @badfiles + 6; # Test 6 contains errors in the free table consistency, but we no # longer have any consistency check for this since it is not important @@ -2334,6 +2334,11 @@ $td->runtest("C API: no recovery", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); +$td->runtest("integer type checks", + {$td->COMMAND => "test_driver 62 minimal.pdf"}, + {$td->STRING => "test 62 done\n", $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); + show_ntests(); # ---------- $td->notify("--- Recovery Tests ---"); diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index d3abbe46..a6a0ef38 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include static char const* whoami = 0; @@ -189,21 +190,35 @@ static void read_file_into_memory( throw std::runtime_error( std::string("failure reading file ") + filename + " into memory: read " + - QUtil::int_to_string(bytes_read) + "; wanted " + - QUtil::int_to_string(size)); + QUtil::uint_to_string(bytes_read) + "; wanted " + + QUtil::uint_to_string(size)); } else { throw std::logic_error( std::string("premature eof reading file ") + filename + " into memory: read " + - QUtil::int_to_string(bytes_read) + "; wanted " + - QUtil::int_to_string(size)); + QUtil::uint_to_string(bytes_read) + "; wanted " + + QUtil::uint_to_string(size)); } } fclose(f); } +#define assert_compare_numbers(expected, expr) \ + compare_numbers(#expr, expected, expr) + +template +static void compare_numbers( + char const* description, T1 const& expected, T2 const& actual) +{ + if (expected != actual) + { + std::cerr << description << ": expected = " << expected + << "; actual = " << actual << std::endl; + } +} + void runtest(int n, char const* filename1, char const* arg2) { // Most tests here are crafted to work on specific files. Look at @@ -2090,6 +2105,31 @@ void runtest(int n, char const* filename1, char const* arg2) std::cout << "Caught runtime_error as expected" << std::endl; } } + else if (n == 62) + { + // Test int size checks. This test will fail if int and long + // long are the same size. + QPDFObjectHandle t = pdf.getTrailer(); + unsigned long long q1_l = 3L * INT_MAX; + long long q1 = static_cast(q1_l); + long long q2_l = 3L * INT_MIN; + long long q2 = static_cast(q2_l); + unsigned int q3_i = UINT_MAX; + long long q3 = static_cast(q3_i); + t.replaceKey("/Q1", QPDFObjectHandle::newInteger(q1)); + t.replaceKey("/Q2", QPDFObjectHandle::newInteger(q2)); + t.replaceKey("/Q3", QPDFObjectHandle::newInteger(q3)); + assert_compare_numbers(q1, t.getKey("/Q1").getIntValue()); + assert_compare_numbers(q1_l, t.getKey("/Q1").getUIntValue()); + assert_compare_numbers(INT_MAX, t.getKey("/Q1").getIntValueAsInt()); + assert_compare_numbers(UINT_MAX, t.getKey("/Q1").getUIntValueAsUInt()); + assert_compare_numbers(q2_l, t.getKey("/Q2").getIntValue()); + assert_compare_numbers(0U, t.getKey("/Q2").getUIntValue()); + assert_compare_numbers(INT_MIN, t.getKey("/Q2").getIntValueAsInt()); + assert_compare_numbers(0U, t.getKey("/Q2").getUIntValueAsUInt()); + assert_compare_numbers(INT_MAX, t.getKey("/Q3").getIntValueAsInt()); + assert_compare_numbers(UINT_MAX, t.getKey("/Q3").getUIntValueAsUInt()); + } else { throw std::runtime_error(std::string("invalid test ") +