commit 65e915172f9d59dca05845e5355d96c08914482d
author Tom Sepez <tsepez@chromium.org> Thu Mar 21 22:01:27 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Mar 21 22:01:27 2019 +0000
tree a62d9ea235411aaa84c834c4d5e40023f0e0fdd5
parent 462832c08374b31dc8b3d5e5db24ab7fa726e9b5

Fix another CHECK() in CFGAS_StringFormatter::FormatNum()

This was broken in f234dde95a which introduced a length calculation
which was off by one. Additionally, it looks like the calculation
would be wrong since we have the position inside the new string which
is concatenated with the old one at line 1017, so the position needs
adjustment based on the length of the old string.

In turn, this makes an old test actually pass.

Bug: pdfium:1271
Change-Id: I3de697bfc29f7dc5b133e5bc7c08f574ff9eeebd
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/52173
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

xfa/fgas/crt/cfgas_stringformatter.cpp

diff --git a/xfa/fgas/crt/cfgas_stringformatter.cpp b/xfa/fgas/crt/cfgas_stringformatter.cpp
index 9c7d0f2..6264510 100644
--- a/xfa/fgas/crt/cfgas_stringformatter.cpp
+++ b/xfa/fgas/crt/cfgas_stringformatter.cpp
@@ -1010,7 +1010,7 @@
           auto result = wsSubCategory.Find('.');
           if (result.has_value() && result.value() != 0) {
             if (!bFindDot)
-              *iDotIndex = wsPurgePattern->GetLength() - 1;
+              *iDotIndex = wsPurgePattern->GetLength() + result.value();
             bFindDot = true;
             *dwStyle |= FX_NUMSTYLE_DotVorv;
           }

xfa/fgas/crt/cfgas_stringformatter_unittest.cpp

diff --git a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp
index 3fa3032..6f0ec2b 100644
--- a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp
+++ b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp
@@ -452,6 +452,8 @@
       {L"en", L"123.5 ", L"zzz.z)", L"123.5"},
       {L"en", L"123.5 ", L"zzz.z(", L"123.5"},
       {L"en", L"123.545,4", L"zzz.zzz,z", L"123.5454"},
+      // https://crbug.com/938724
+      {L"en", L"1", L" num.().().}", L"1"},
   };
 
   static const TestCase failures[] = {
@@ -460,9 +462,6 @@
 
       // https://crbug.com/938626
       {L"en", L"PDF", L"num( ", L"."},
-
-      // https://crbug.com/938724
-      {L"en", L"1", L" num.().().}", L"."},
   };
 
   for (const auto& test : tests) {
@@ -589,6 +588,11 @@
       {L"en", L"1", L"9.", L"1"},
   };
 
+  static const TestCase failures[] = {
+      // https://crbug.com/pdfium/1271
+      {L"en", L"1", L"num.{E", L"1"},
+  };
+
   for (const auto& test : tests) {
     WideString result;
     EXPECT_TRUE(fmt(test.locale, test.pattern)->FormatNum(test.input, &result))
@@ -596,6 +600,12 @@
     EXPECT_STREQ(test.output, result.c_str())
         << " TEST: " << test.input << ", " << test.pattern;
   }
+
+  for (const auto& test : failures) {
+    WideString result;
+    EXPECT_FALSE(fmt(test.locale, test.pattern)->FormatNum(test.input, &result))
+        << " TEST: " << test.input << ", " << test.pattern;
+  }
 }
 
 TEST_F(CFGAS_StringFormatterTest, TextParse) {