Commit 3608afd5c528b7a9d95d227cb6c4f33d303fcfcd
1 parent
42306e2f
Add new integer accessors to QPDFObjectHandle
Showing
6 changed files
with
142 additions
and
5 deletions
ChangeLog
| 1 | 2019-06-20 Jay Berkenbilt <ejb@ql.org> | 1 | 2019-06-20 Jay Berkenbilt <ejb@ql.org> |
| 2 | 2 | ||
| 3 | + * Add methods to QPDFObjectHandle to return the value of Integer | ||
| 4 | + objects as int and unsigned int with range checking and fallback | ||
| 5 | + behavior to avoid silent underflow/overflow conditions. | ||
| 6 | + | ||
| 3 | * Add functions to QUtil to convert unsigned integers to strings, | 7 | * Add functions to QUtil to convert unsigned integers to strings, |
| 4 | avoiding implicit conversion between unsigned and signed integer | 8 | avoiding implicit conversion between unsigned and signed integer |
| 5 | types. | 9 | types. |
include/qpdf/QPDFObjectHandle.hh
| @@ -491,6 +491,12 @@ class QPDFObjectHandle | @@ -491,6 +491,12 @@ class QPDFObjectHandle | ||
| 491 | // Methods for integer objects | 491 | // Methods for integer objects |
| 492 | QPDF_DLL | 492 | QPDF_DLL |
| 493 | long long getIntValue(); | 493 | long long getIntValue(); |
| 494 | + QPDF_DLL | ||
| 495 | + int getIntValueAsInt(); | ||
| 496 | + QPDF_DLL | ||
| 497 | + unsigned long long getUIntValue(); | ||
| 498 | + QPDF_DLL | ||
| 499 | + unsigned int getUIntValueAsUInt(); | ||
| 494 | 500 | ||
| 495 | // Methods for real objects | 501 | // Methods for real objects |
| 496 | QPDF_DLL | 502 | QPDF_DLL |
libqpdf/QPDFObjectHandle.cc
| @@ -26,6 +26,7 @@ | @@ -26,6 +26,7 @@ | ||
| 26 | #include <stdexcept> | 26 | #include <stdexcept> |
| 27 | #include <stdlib.h> | 27 | #include <stdlib.h> |
| 28 | #include <ctype.h> | 28 | #include <ctype.h> |
| 29 | +#include <limits.h> | ||
| 29 | 30 | ||
| 30 | class TerminateParsing | 31 | class TerminateParsing |
| 31 | { | 32 | { |
| @@ -406,6 +407,82 @@ QPDFObjectHandle::getIntValue() | @@ -406,6 +407,82 @@ QPDFObjectHandle::getIntValue() | ||
| 406 | } | 407 | } |
| 407 | } | 408 | } |
| 408 | 409 | ||
| 410 | +int | ||
| 411 | +QPDFObjectHandle::getIntValueAsInt() | ||
| 412 | +{ | ||
| 413 | + int result = 0; | ||
| 414 | + long long v = getIntValue(); | ||
| 415 | + if (v < INT_MIN) | ||
| 416 | + { | ||
| 417 | + QTC::TC("qpdf", "QPDFObjectHandle int returning INT_MIN"); | ||
| 418 | + warnIfPossible( | ||
| 419 | + "requested value of integer is too small; returning INT_MIN", | ||
| 420 | + false); | ||
| 421 | + result = INT_MIN; | ||
| 422 | + } | ||
| 423 | + else if (v > INT_MAX) | ||
| 424 | + { | ||
| 425 | + QTC::TC("qpdf", "QPDFObjectHandle int returning INT_MAX"); | ||
| 426 | + warnIfPossible( | ||
| 427 | + "requested value of integer is too big; returning INT_MAX", | ||
| 428 | + false); | ||
| 429 | + result = INT_MAX; | ||
| 430 | + } | ||
| 431 | + else | ||
| 432 | + { | ||
| 433 | + result = static_cast<int>(v); | ||
| 434 | + } | ||
| 435 | + return result; | ||
| 436 | +} | ||
| 437 | + | ||
| 438 | +unsigned long long | ||
| 439 | +QPDFObjectHandle::getUIntValue() | ||
| 440 | +{ | ||
| 441 | + unsigned long long result = 0; | ||
| 442 | + long long v = getIntValue(); | ||
| 443 | + if (v < 0) | ||
| 444 | + { | ||
| 445 | + QTC::TC("qpdf", "QPDFObjectHandle uint returning 0"); | ||
| 446 | + warnIfPossible( | ||
| 447 | + "unsigned value request for negative number; returning 0", | ||
| 448 | + false); | ||
| 449 | + } | ||
| 450 | + else | ||
| 451 | + { | ||
| 452 | + result = static_cast<unsigned long long>(v); | ||
| 453 | + } | ||
| 454 | + return result; | ||
| 455 | +} | ||
| 456 | + | ||
| 457 | +unsigned int | ||
| 458 | +QPDFObjectHandle::getUIntValueAsUInt() | ||
| 459 | +{ | ||
| 460 | + unsigned int result = 0; | ||
| 461 | + long long v = getIntValue(); | ||
| 462 | + if (v < 0) | ||
| 463 | + { | ||
| 464 | + QTC::TC("qpdf", "QPDFObjectHandle uint uint returning 0"); | ||
| 465 | + warnIfPossible( | ||
| 466 | + "unsigned integer value request for negative number; returning 0", | ||
| 467 | + false); | ||
| 468 | + result = 0; | ||
| 469 | + } | ||
| 470 | + else if (v > UINT_MAX) | ||
| 471 | + { | ||
| 472 | + QTC::TC("qpdf", "QPDFObjectHandle uint returning UINT_MAX"); | ||
| 473 | + warnIfPossible( | ||
| 474 | + "requested value of unsigned integer is too big;" | ||
| 475 | + " returning UINT_MAX", | ||
| 476 | + false); | ||
| 477 | + result = UINT_MAX; | ||
| 478 | + } | ||
| 479 | + else | ||
| 480 | + { | ||
| 481 | + result = static_cast<unsigned int>(v); | ||
| 482 | + } | ||
| 483 | + return result; | ||
| 484 | +} | ||
| 485 | + | ||
| 409 | // Real accessors | 486 | // Real accessors |
| 410 | 487 | ||
| 411 | std::string | 488 | std::string |
qpdf/qpdf.testcov
| @@ -440,3 +440,8 @@ QPDFPageObjectHelper keep inline image 0 | @@ -440,3 +440,8 @@ QPDFPageObjectHelper keep inline image 0 | ||
| 440 | qpdf image optimize colorspace 0 | 440 | qpdf image optimize colorspace 0 |
| 441 | qpdf image optimize bits per component 0 | 441 | qpdf image optimize bits per component 0 |
| 442 | QPDFWriter remove empty DecodeParms 0 | 442 | QPDFWriter remove empty DecodeParms 0 |
| 443 | +QPDFObjectHandle uint returning 0 0 | ||
| 444 | +QPDFObjectHandle int returning INT_MIN 0 | ||
| 445 | +QPDFObjectHandle int returning INT_MAX 0 | ||
| 446 | +QPDFObjectHandle uint returning UINT_MAX 0 | ||
| 447 | +QPDFObjectHandle uint uint returning 0 0 |
qpdf/qtest/qpdf.test
| @@ -2286,7 +2286,7 @@ my @badfiles = ("not a PDF file", # 1 | @@ -2286,7 +2286,7 @@ my @badfiles = ("not a PDF file", # 1 | ||
| 2286 | "bad dictionary key", # 36 | 2286 | "bad dictionary key", # 36 |
| 2287 | ); | 2287 | ); |
| 2288 | 2288 | ||
| 2289 | -$n_tests += @badfiles + 5; | 2289 | +$n_tests += @badfiles + 6; |
| 2290 | 2290 | ||
| 2291 | # Test 6 contains errors in the free table consistency, but we no | 2291 | # Test 6 contains errors in the free table consistency, but we no |
| 2292 | # longer have any consistency check for this since it is not important | 2292 | # longer have any consistency check for this since it is not important |
| @@ -2334,6 +2334,11 @@ $td->runtest("C API: no recovery", | @@ -2334,6 +2334,11 @@ $td->runtest("C API: no recovery", | ||
| 2334 | $td->EXIT_STATUS => 0}, | 2334 | $td->EXIT_STATUS => 0}, |
| 2335 | $td->NORMALIZE_NEWLINES); | 2335 | $td->NORMALIZE_NEWLINES); |
| 2336 | 2336 | ||
| 2337 | +$td->runtest("integer type checks", | ||
| 2338 | + {$td->COMMAND => "test_driver 62 minimal.pdf"}, | ||
| 2339 | + {$td->STRING => "test 62 done\n", $td->EXIT_STATUS => 0}, | ||
| 2340 | + $td->NORMALIZE_NEWLINES); | ||
| 2341 | + | ||
| 2337 | show_ntests(); | 2342 | show_ntests(); |
| 2338 | # ---------- | 2343 | # ---------- |
| 2339 | $td->notify("--- Recovery Tests ---"); | 2344 | $td->notify("--- Recovery Tests ---"); |
qpdf/test_driver.cc
| @@ -24,6 +24,7 @@ | @@ -24,6 +24,7 @@ | ||
| 24 | #include <string.h> | 24 | #include <string.h> |
| 25 | #include <stdlib.h> | 25 | #include <stdlib.h> |
| 26 | #include <assert.h> | 26 | #include <assert.h> |
| 27 | +#include <limits.h> | ||
| 27 | #include <map> | 28 | #include <map> |
| 28 | 29 | ||
| 29 | static char const* whoami = 0; | 30 | static char const* whoami = 0; |
| @@ -189,21 +190,35 @@ static void read_file_into_memory( | @@ -189,21 +190,35 @@ static void read_file_into_memory( | ||
| 189 | throw std::runtime_error( | 190 | throw std::runtime_error( |
| 190 | std::string("failure reading file ") + filename + | 191 | std::string("failure reading file ") + filename + |
| 191 | " into memory: read " + | 192 | " into memory: read " + |
| 192 | - QUtil::int_to_string(bytes_read) + "; wanted " + | ||
| 193 | - QUtil::int_to_string(size)); | 193 | + QUtil::uint_to_string(bytes_read) + "; wanted " + |
| 194 | + QUtil::uint_to_string(size)); | ||
| 194 | } | 195 | } |
| 195 | else | 196 | else |
| 196 | { | 197 | { |
| 197 | throw std::logic_error( | 198 | throw std::logic_error( |
| 198 | std::string("premature eof reading file ") + filename + | 199 | std::string("premature eof reading file ") + filename + |
| 199 | " into memory: read " + | 200 | " into memory: read " + |
| 200 | - QUtil::int_to_string(bytes_read) + "; wanted " + | ||
| 201 | - QUtil::int_to_string(size)); | 201 | + QUtil::uint_to_string(bytes_read) + "; wanted " + |
| 202 | + QUtil::uint_to_string(size)); | ||
| 202 | } | 203 | } |
| 203 | } | 204 | } |
| 204 | fclose(f); | 205 | fclose(f); |
| 205 | } | 206 | } |
| 206 | 207 | ||
| 208 | +#define assert_compare_numbers(expected, expr) \ | ||
| 209 | + compare_numbers(#expr, expected, expr) | ||
| 210 | + | ||
| 211 | +template <typename T1, typename T2> | ||
| 212 | +static void compare_numbers( | ||
| 213 | + char const* description, T1 const& expected, T2 const& actual) | ||
| 214 | +{ | ||
| 215 | + if (expected != actual) | ||
| 216 | + { | ||
| 217 | + std::cerr << description << ": expected = " << expected | ||
| 218 | + << "; actual = " << actual << std::endl; | ||
| 219 | + } | ||
| 220 | +} | ||
| 221 | + | ||
| 207 | void runtest(int n, char const* filename1, char const* arg2) | 222 | void runtest(int n, char const* filename1, char const* arg2) |
| 208 | { | 223 | { |
| 209 | // Most tests here are crafted to work on specific files. Look at | 224 | // 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) | @@ -2090,6 +2105,31 @@ void runtest(int n, char const* filename1, char const* arg2) | ||
| 2090 | std::cout << "Caught runtime_error as expected" << std::endl; | 2105 | std::cout << "Caught runtime_error as expected" << std::endl; |
| 2091 | } | 2106 | } |
| 2092 | } | 2107 | } |
| 2108 | + else if (n == 62) | ||
| 2109 | + { | ||
| 2110 | + // Test int size checks. This test will fail if int and long | ||
| 2111 | + // long are the same size. | ||
| 2112 | + QPDFObjectHandle t = pdf.getTrailer(); | ||
| 2113 | + unsigned long long q1_l = 3L * INT_MAX; | ||
| 2114 | + long long q1 = static_cast<long long>(q1_l); | ||
| 2115 | + long long q2_l = 3L * INT_MIN; | ||
| 2116 | + long long q2 = static_cast<long long>(q2_l); | ||
| 2117 | + unsigned int q3_i = UINT_MAX; | ||
| 2118 | + long long q3 = static_cast<long long>(q3_i); | ||
| 2119 | + t.replaceKey("/Q1", QPDFObjectHandle::newInteger(q1)); | ||
| 2120 | + t.replaceKey("/Q2", QPDFObjectHandle::newInteger(q2)); | ||
| 2121 | + t.replaceKey("/Q3", QPDFObjectHandle::newInteger(q3)); | ||
| 2122 | + assert_compare_numbers(q1, t.getKey("/Q1").getIntValue()); | ||
| 2123 | + assert_compare_numbers(q1_l, t.getKey("/Q1").getUIntValue()); | ||
| 2124 | + assert_compare_numbers(INT_MAX, t.getKey("/Q1").getIntValueAsInt()); | ||
| 2125 | + assert_compare_numbers(UINT_MAX, t.getKey("/Q1").getUIntValueAsUInt()); | ||
| 2126 | + assert_compare_numbers(q2_l, t.getKey("/Q2").getIntValue()); | ||
| 2127 | + assert_compare_numbers(0U, t.getKey("/Q2").getUIntValue()); | ||
| 2128 | + assert_compare_numbers(INT_MIN, t.getKey("/Q2").getIntValueAsInt()); | ||
| 2129 | + assert_compare_numbers(0U, t.getKey("/Q2").getUIntValueAsUInt()); | ||
| 2130 | + assert_compare_numbers(INT_MAX, t.getKey("/Q3").getIntValueAsInt()); | ||
| 2131 | + assert_compare_numbers(UINT_MAX, t.getKey("/Q3").getUIntValueAsUInt()); | ||
| 2132 | + } | ||
| 2093 | else | 2133 | else |
| 2094 | { | 2134 | { |
| 2095 | throw std::runtime_error(std::string("invalid test ") + | 2135 | throw std::runtime_error(std::string("invalid test ") + |