From 8be827761347b7a0a4ce6e7bdfa6fd4585606b21 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Tue, 26 Feb 2013 12:31:00 -0500 Subject: [PATCH] Rewrite QUtil::int_to_string and QUtil::double_to_string Make them safer by avoiding any internal limits and replacing sprintf with std::ostringstream. --- ChangeLog | 6 +++ configure.ac | 18 -------- libqpdf/QUtil.cc | 80 +++++++++++----------------------- libtests/qtest/qutil/qutil.out | 9 ++-- libtests/qutil.cc | 57 +++--------------------- 5 files changed, 41 insertions(+), 129 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9a382293..0ce3358a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2013-02-26 Jay Berkenbilt + + * Rewrite QUtil::int_to_string and QUtil::double_to_string to + remove internal length limits but to remain backward compatible + with the old versions for valid inputs. + 2013-02-23 Jay Berkenbilt * Bug fix: properly handle overridden compressed objects. When diff --git a/configure.ac b/configure.ac index c423b086..06a9e207 100644 --- a/configure.ac +++ b/configure.ac @@ -68,24 +68,6 @@ AC_CHECK_FUNCS([fseeko64]) AC_TYPE_UINT16_T AC_TYPE_UINT32_T -AC_MSG_CHECKING(for whether printf supports %ll) -AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ -#include -#include -#include -int -main() -{ - long long a = 160591605916059ll; - char t[50]; - sprintf(t, "%lld", a); -} -]])],[qpdf_PRINTF_LL=yes],[qpdf_PRINTF_LL=no]) -AC_MSG_RESULT($qpdf_PRINTF_LL) -if test "$qpdf_PRINTF_LL" = "yes"; then - AC_DEFINE([HAVE_PRINTF_LL], [1], [Whether printf supports %ll]) -fi - AC_CHECK_FUNCS(random) # Check if LD supports linker scripts, and define conditional diff --git a/libqpdf/QUtil.cc b/libqpdf/QUtil.cc index 9549bfa1..967e832c 100644 --- a/libqpdf/QUtil.cc +++ b/libqpdf/QUtil.cc @@ -4,6 +4,9 @@ #include #include +#include +#include +#include #include #include #include @@ -19,72 +22,41 @@ #endif std::string -QUtil::int_to_string(long long num, int fullpad) +QUtil::int_to_string(long long num, int length) { - // This routine will need to be recompiled if an int can be longer than - // 49 digits. - char t[50]; - - // -2 or -1 to leave space for the possible negative sign and for NUL... - if (abs(fullpad) > sizeof(t) - ((num < 0)?2:1)) + // Backward compatibility -- this function used to use sprintf + // with %0*d, so we interpret length such that a negative value + // appends spaces and a positive value prepends zeroes. + std::ostringstream buf; + buf << num; + std::string result; + if ((length > 0) && + (buf.str().length() < static_cast(length))) { - throw std::logic_error("Util::int_to_string has been called with " - "a padding value greater than its internal " - "limit"); + result.append(length - buf.str().length(), '0'); } - -#ifdef HAVE_PRINTF_LL -# define PRINTF_LL "ll" -#else -# define PRINTF_LL "l" -#endif - if (fullpad) + result += buf.str(); + if ((length < 0) && (buf.str().length() < static_cast(-length))) { - sprintf(t, "%0*" PRINTF_LL "d", fullpad, num); + result.append(-length - buf.str().length(), ' '); } - else - { - sprintf(t, "%" PRINTF_LL "d", num); - } -#undef PRINTF_LL - - return std::string(t); + return result; } std::string QUtil::double_to_string(double num, int decimal_places) { - // This routine will need to be recompiled if a double can be longer than - // 99 digits. - char t[100]; - - std::string lhs = int_to_string(static_cast(num)); - - // lhs.length() gives us the length of the part on the right hand - // side of the dot + 1 for the dot + decimal_places: total size of - // the required string. -1 on the sizeof side to allow for NUL at - // the end. - // - // If decimal_places <= 0, it is as if no precision was provided - // so trust the buffer is big enough. The following test will - // always pass in those cases. - if (decimal_places + 1 + static_cast(lhs.length()) > - static_cast(sizeof(t)) - 1) + // Backward compatibility -- this code used to use sprintf and + // treated decimal_places <= 0 to mean to use the default, which + // was six decimal places. Also sprintf with %*.f interprents the + // length as fixed point rather than significant figures. + if (decimal_places <= 0) { - throw std::logic_error("Util::double_to_string has been called with " - "a number and a decimal places specification " - "that would break an internal limit"); + decimal_places = 6; } - - if (decimal_places) - { - sprintf(t, "%.*f", decimal_places, num); - } - else - { - sprintf(t, "%f", num); - } - return std::string(t); + std::ostringstream buf; + buf << std::setprecision(decimal_places) << std::fixed << num; + return buf.str(); } long long diff --git a/libtests/qtest/qutil/qutil.out b/libtests/qtest/qutil/qutil.out index a48e6266..c855e98e 100644 --- a/libtests/qtest/qutil/qutil.out +++ b/libtests/qtest/qutil/qutil.out @@ -4,11 +4,10 @@ 3.141590 3.142 1000.123000 -exception 1: Util::int_to_string has been called with a padding value greater than its internal limit -exception 2: Util::int_to_string has been called with a padding value greater than its internal limit -exception 3: Util::int_to_string has been called with a padding value greater than its internal limit -exception 4: Util::double_to_string has been called with a number and a decimal places specification that would break an internal limit -exception 5: Util::double_to_string has been called with a number and a decimal places specification that would break an internal limit +0.12340 +0.00012 +0.12346 +0.00012 one 7 compare okay diff --git a/libtests/qutil.cc b/libtests/qutil.cc index ad77f3ea..4e8c7362 100644 --- a/libtests/qutil.cc +++ b/libtests/qutil.cc @@ -19,58 +19,11 @@ void string_conversion_test() << QUtil::int_to_string(16059, -7) << std::endl << QUtil::double_to_string(3.14159) << std::endl << QUtil::double_to_string(3.14159, 3) << std::endl - << QUtil::double_to_string(1000.123, -1024) << std::endl; - - try - { - // int_to_string bounds error - std::cout << QUtil::int_to_string(1, 50) << std::endl; - } - catch (std::logic_error &e) - { - std::cout << "exception 1: " << e.what() << std::endl; - } - - try - { - // QUtil::int_to_string bounds error - std::cout << QUtil::int_to_string(1, -50) << std::endl; - } - catch (std::logic_error& e) - { - std::cout << "exception 2: " << e.what() << std::endl; - } - - try - { - // QUtil::int_to_string bounds error - std::cout << QUtil::int_to_string(-1, 49) << std::endl; - } - catch (std::logic_error& e) - { - std::cout << "exception 3: " << e.what() << std::endl; - } - - - try - { - // QUtil::double_to_string bounds error - std::cout << QUtil::double_to_string(3.14159, 1024) << std::endl; - } - catch (std::logic_error& e) - { - std::cout << "exception 4: " << e.what() << std::endl; - } - - try - { - // QUtil::double_to_string bounds error - std::cout << QUtil::double_to_string(1000.0, 95) << std::endl; - } - catch (std::logic_error& e) - { - std::cout << "exception 5: " << e.what() << std::endl; - } + << QUtil::double_to_string(1000.123, -1024) << std::endl + << QUtil::double_to_string(.1234, 5) << std::endl + << QUtil::double_to_string(.0001234, 5) << std::endl + << QUtil::double_to_string(.123456, 5) << std::endl + << QUtil::double_to_string(.000123456, 5) << std::endl; std::string embedded_null = "one"; embedded_null += '\0';