From 18a0b94b0e676d5030e96ca655080fb43a23ac7d Mon Sep 17 00:00:00 2001
From: Victor Zverovich <victor.zverovich@gmail.com>
Date: Sun, 12 Nov 2017 06:58:11 -0800
Subject: [PATCH] Fix overflow check

---
 include/fmt/format.h | 33 ++++++++++++++++++++-------------
 test/format-test.cc  | 15 ++++++++++++---
 test/util-test.cc    | 15 +++++++++++++++
 3 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/include/fmt/format.h b/include/fmt/format.h
index ca656170..a87d0eed 100644
--- a/include/fmt/format.h
+++ b/include/fmt/format.h
@@ -2040,26 +2040,26 @@ constexpr bool is_name_start(Char c) {
 // first character is a digit and presence of a non-digit character at the end.
 // it: an iterator pointing to the beginning of the input range.
 template <typename Iterator, typename ErrorHandler>
-constexpr unsigned parse_nonnegative_int(Iterator &it, ErrorHandler& handler) {
+constexpr unsigned parse_nonnegative_int(Iterator &it, ErrorHandler &&eh) {
   assert('0' <= *it && *it <= '9');
   unsigned value = 0;
+  // Convert to unsigned to prevent a warning.
+  unsigned max_int = (std::numeric_limits<int>::max)();
+  unsigned big = max_int / 10;
   do {
-    unsigned new_value = value * 10 + (*it - '0');
+    // Check for overflow.
+    if (value > big) {
+      value = max_int + 1;
+      break;
+    }
+    value = value * 10 + (*it - '0');
     // Workaround for MSVC "setup_exception stack overflow" error:
     auto next = it;
     ++next;
     it = next;
-    // Check if value wrapped around.
-    if (new_value < value) {
-      value = (std::numeric_limits<unsigned>::max)();
-      break;
-    }
-    value = new_value;
   } while ('0' <= *it && *it <= '9');
-  // Convert to unsigned to prevent a warning.
-  unsigned max_int = (std::numeric_limits<int>::max)();
   if (value > max_int)
-    handler.on_error("number is too big");
+    eh.on_error("number is too big");
   return value;
 }
 
@@ -2145,6 +2145,8 @@ class specs_setter : public error_handler {
   explicit constexpr specs_setter(basic_format_specs<Char> &specs):
     specs_(specs) {}
 
+  constexpr specs_setter(const specs_setter &other) : specs_(other.specs_) {}
+
   constexpr void on_align(alignment align) { specs_.align_ = align; }
   constexpr void on_fill(Char fill) { specs_.fill_ = fill; }
   constexpr void on_plus() { specs_.flags_ |= SIGN_FLAG | PLUS_FLAG; }
@@ -2177,6 +2179,9 @@ class specs_checker : public Handler {
   constexpr specs_checker(const Handler& handler, internal::type arg_type)
     : Handler(handler), arg_type_(arg_type) {}
 
+  constexpr specs_checker(const specs_checker &other)
+    : Handler(other), arg_type_(other.arg_type_) {}
+
   constexpr void on_align(alignment align) {
     if (align == ALIGN_NUMERIC)
       require_numeric_argument('=');
@@ -2330,6 +2335,9 @@ class dynamic_specs_handler :
       dynamic_format_specs<char_type> &specs, ParseContext &ctx)
     : specs_setter<char_type>(specs), specs_(specs), context_(ctx) {}
 
+  constexpr dynamic_specs_handler(const dynamic_specs_handler &other)
+    : specs_setter(other), specs_(other.specs_), context_(other.context_) {}
+
   template <typename Id>
   constexpr void on_dynamic_width(Id arg_id) {
     specs_.width_ref = make_arg_ref(arg_id);
@@ -2636,8 +2644,7 @@ class format_string_checker : public ErrorHandler {
 template <typename Char, typename ErrorHandler, typename... Args>
 constexpr bool check_format_string(
     basic_string_view<Char> s, ErrorHandler eh = ErrorHandler()) {
-  format_string_checker<Char, ErrorHandler, Args...>
-      checker(std::move(eh), s.end());
+  format_string_checker<Char, ErrorHandler, Args...> checker(eh, s.end());
   parse_format_string(s.begin(), checker);
   return true;
 }
diff --git a/test/format-test.cc b/test/format-test.cc
index b9ba69dc..38bf2dd8 100644
--- a/test/format-test.cc
+++ b/test/format-test.cc
@@ -1821,7 +1821,15 @@ TEST(FormatTest, UdlTemplate) {
 struct test_error_handler {
   const char *&error;
 
-  constexpr void on_error(const char *message) { error = message; }
+  constexpr test_error_handler(const char *&err): error(err) {}
+
+  constexpr test_error_handler(const test_error_handler &other)
+    : error(other.error) {}
+
+  constexpr void on_error(const char *message) {
+    if (!error)
+      error = message;
+  }
 };
 
 constexpr size_t len(const char *s) {
@@ -1831,7 +1839,7 @@ constexpr size_t len(const char *s) {
   return len;
 }
 
-constexpr bool eq(const char *s1, const char *s2) {
+constexpr bool equal(const char *s1, const char *s2) {
   if (!s1 && !s2)
     return true;
   while (*s1 && *s1 == *s2) {
@@ -1848,7 +1856,7 @@ constexpr bool test_error(const char *fmt, const char *expected_error) {
   using ref = std::reference_wrapper<test_error_handler>;
   fmt::internal::check_format_string<char, test_error_handler, Args...>(
         string_view(fmt, len(fmt)), eh);
-  return eq(actual_error, expected_error);
+  return equal(actual_error, expected_error);
 }
 
 #define EXPECT_ERROR(fmt, error, ...) \
@@ -1860,4 +1868,5 @@ TEST(FormatTest, FormatStringErrors) {
   EXPECT_ERROR("{0:s", "unknown format specifier", Date);
   EXPECT_ERROR("{0:=5", "unknown format specifier", char);
   EXPECT_ERROR("{foo", "missing '}' in format string", int);
+  EXPECT_ERROR("{10000000000}", "number is too big");
 }
diff --git a/test/util-test.cc b/test/util-test.cc
index 5c276e31..fdd66b48 100644
--- a/test/util-test.cc
+++ b/test/util-test.cc
@@ -838,3 +838,18 @@ TEST(UtilTest, IsEnumConvertibleToInt) {
   EXPECT_TRUE(fmt::internal::convert_to_int<TestEnum>::enable_conversion);
 }
 #endif
+
+TEST(UtilTest, ParseNonnegativeInt) {
+  if (std::numeric_limits<int>::max() != (1 << 31)) {
+    fmt::print("Skipping parse_nonnegative_int test\n");
+    return;
+  }
+  const char *s = "10000000000";
+  EXPECT_THROW_MSG(
+        parse_nonnegative_int(s, fmt::internal::error_handler()),
+        fmt::format_error, "number is too big");
+  s = "2147483649";
+  EXPECT_THROW_MSG(
+        parse_nonnegative_int(s, fmt::internal::error_handler()),
+        fmt::format_error, "number is too big");
+}