Use span in CXFA_LocaleValue::GetDoubleNum()chromium/3399

Get runtime checks instead of just asserts.
Use early return while at it to save some indentation.
Fix one out-of-bounds case noted by inspection.
Add tests, and fix bugs that gave the wrong answers.

This should be removed at our earliest convenience. Its likely to
still be wrong.

Change-Id: I2a68edc854c8b28c434fe28397b7834e5c9c3670
Reviewed-on: https://pdfium-review.googlesource.com/30530
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

3 files changed, 95 insertions, 68 deletions

diff --git a/BUILD.gn b/BUILD.gn
index e78b2cf6e4..c00557594b 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -2929,6 +2929,7 @@ test("pdfium_unittests") {
       "xfa/fxfa/fm2js/cxfa_fmlexer_unittest.cpp",
       "xfa/fxfa/fm2js/cxfa_fmparser_unittest.cpp",
       "xfa/fxfa/fm2js/cxfa_fmsimpleexpression_unittest.cpp",
+      "xfa/fxfa/parser/cxfa_localevalue_unittest.cpp",
       "xfa/fxfa/parser/cxfa_node_unittest.cpp",
       "xfa/fxfa/parser/cxfa_nodeiteratortemplate_unittest.cpp",
       "xfa/fxfa/parser/cxfa_xmllocale_unittest.cpp",
diff --git a/xfa/fxfa/parser/cxfa_localevalue.cpp b/xfa/fxfa/parser/cxfa_localevalue.cpp
index 057eaeba2c..78f6b3b5e4 100644
--- a/xfa/fxfa/parser/cxfa_localevalue.cpp
+++ b/xfa/fxfa/parser/cxfa_localevalue.cpp
@@ -10,6 +10,7 @@
 
 #include "core/fxcrt/fx_extension.h"
 #include "third_party/base/ptr_util.h"
+#include "third_party/base/span.h"
 #include "third_party/base/stl_util.h"
 #include "xfa/fgas/crt/cfgas_formatstring.h"
 #include "xfa/fxfa/parser/cxfa_document.h"
@@ -192,83 +193,71 @@ bool CXFA_LocaleValue::ValidateValue(const WideString& wsValue,
 }
 
 double CXFA_LocaleValue::GetDoubleNum() const {
-  if (m_bValid && (m_dwType == XFA_VT_BOOLEAN || m_dwType == XFA_VT_INTEGER ||
-                   m_dwType == XFA_VT_DECIMAL || m_dwType == XFA_VT_FLOAT)) {
-    int64_t nIntegral = 0;
-    uint32_t dwFractional = 0;
-    int32_t nExponent = 0;
-    int32_t cc = 0;
-    bool bNegative = false;
-    bool bExpSign = false;
-    const wchar_t* str = m_wsValue.c_str();
-    int len = m_wsValue.GetLength();
-    while (FXSYS_iswspace(str[cc]) && cc < len)
-      cc++;
-
-    if (cc >= len)
-      return 0;
-    if (str[0] == '+') {
-      cc++;
-    } else if (str[0] == '-') {
-      bNegative = true;
-      cc++;
-    }
+  if (!m_bValid || (m_dwType != XFA_VT_BOOLEAN && m_dwType != XFA_VT_INTEGER &&
+                    m_dwType != XFA_VT_DECIMAL && m_dwType != XFA_VT_FLOAT)) {
+    return 0;
+  }
+
+  size_t cc = 0;
+  pdfium::span<const wchar_t> str = m_wsValue.AsSpan();
+  while (cc < str.size() && FXSYS_iswspace(str[cc]))
+    cc++;
+
+  if (cc >= str.size())
+    return 0;
+
+  bool bNegative = false;
+  if (str[cc] == '+') {
+    cc++;
+  } else if (str[cc] == '-') {
+    bNegative = true;
+    cc++;
+  }
+
+  int64_t nIntegral = 0;
+  size_t nIntegralLen = 0;
+  while (cc < str.size()) {
+    if (str[cc] == '.' || !FXSYS_isDecimalDigit(str[cc]) || nIntegralLen > 17)
+      break;
 
-    int32_t nIntegralLen = 0;
-    while (cc < len) {
-      if (str[cc] == '.' || !FXSYS_isDecimalDigit(str[cc]) ||
-          nIntegralLen > 17) {
+    nIntegral = nIntegral * 10 + str[cc++] - '0';
+    nIntegralLen++;
+  }
+  nIntegral = bNegative ? -nIntegral : nIntegral;
+
+  int32_t scale = 0;
+  int32_t nExponent = 0;
+  double fraction = 0.0;
+  if (cc < str.size() && str[cc] == '.') {
+    cc++;
+    while (cc < str.size() && FXSYS_isDecimalDigit(str[cc])) {
+      fraction += XFA_GetFractionalScale(scale) * (str[cc++] - '0');
+      if (++scale == XFA_GetMaxFractionalScale())
         break;
-      }
-      nIntegral = nIntegral * 10 + str[cc] - '0';
-      cc++;
-      nIntegralLen++;
     }
-
-    nIntegral = bNegative ? -nIntegral : nIntegral;
-    int32_t scale = 0;
-    double fraction = 0.0;
-    if (cc < len && str[cc] == '.') {
-      cc++;
-      while (cc < len) {
-        fraction += XFA_GetFractionalScale(scale) * (str[cc] - '0');
-        scale++;
+  }
+  if (cc < str.size() && (str[cc] == 'E' || str[cc] == 'e')) {
+    cc++;
+    bool bExpSign = false;
+    if (cc < str.size()) {
+      if (str[cc] == '+') {
         cc++;
-        if (scale == XFA_GetMaxFractionalScale() ||
-            !FXSYS_isDecimalDigit(str[cc])) {
-          break;
-        }
-      }
-      dwFractional = static_cast<uint32_t>(fraction * 4294967296.0);
-    }
-    if (cc < len && (str[cc] == 'E' || str[cc] == 'e')) {
-      cc++;
-      if (cc < len) {
-        if (str[cc] == '+') {
-          cc++;
-        } else if (str[cc] == '-') {
-          bExpSign = true;
-          cc++;
-        }
-      }
-      while (cc < len) {
-        if (str[cc] == '.' || !FXSYS_isDecimalDigit(str[cc]))
-          break;
-
-        nExponent = nExponent * 10 + str[cc] - '0';
+      } else if (str[cc] == '-') {
+        bExpSign = true;
         cc++;
       }
-      nExponent = bExpSign ? -nExponent : nExponent;
     }
+    while (cc < str.size() && FXSYS_isDecimalDigit(str[cc]))
+      nExponent = nExponent * 10 + str[cc++] - '0';
 
-    double dValue = dwFractional / 4294967296.0;
-    dValue = nIntegral + (nIntegral >= 0 ? dValue : -dValue);
-    if (nExponent != 0)
-      dValue *= FXSYS_pow(10, static_cast<float>(nExponent));
-
-    return dValue;
+    nExponent = bExpSign ? -nExponent : nExponent;
   }
-  return 0;
+
+  double dValue = nIntegral + (bNegative ? -fraction : fraction);
+  if (nExponent != 0)
+    dValue *= FXSYS_pow(10, static_cast<float>(nExponent));
+
+  return dValue;
 }
 
 CFX_DateTime CXFA_LocaleValue::GetDate() const {
diff --git a/xfa/fxfa/parser/cxfa_localevalue_unittest.cpp b/xfa/fxfa/parser/cxfa_localevalue_unittest.cpp
new file mode 100644
index 0000000000..5b97eb3785
--- /dev/null
+++ b/xfa/fxfa/parser/cxfa_localevalue_unittest.cpp
@@ -0,0 +1,37 @@
+// Copyright 2018 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "xfa/fxfa/parser/cxfa_localevalue.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/test_support.h"
+
+namespace {
+
+// We don't expect more precision than a float's worth from this code.
+float MakeDoubleNumAsFloat(const wchar_t* str) {
+  return static_cast<float>(
+      CXFA_LocaleValue(XFA_VT_FLOAT, str, nullptr).GetDoubleNum());
+}
+
+}  // namespace
+
+TEST(CXFALocaleValueTest, GetDoubleNum) {
+  EXPECT_EQ(0.0, MakeDoubleNumAsFloat(L""));
+  EXPECT_EQ(0.0, MakeDoubleNumAsFloat(L"0"));
+  EXPECT_EQ(0.0, MakeDoubleNumAsFloat(L"0."));
+  EXPECT_EQ(0.0, MakeDoubleNumAsFloat(L"0.0"));
+  EXPECT_EQ(0.0, MakeDoubleNumAsFloat(L"0.x"));
+  EXPECT_EQ(7.0, MakeDoubleNumAsFloat(L"7.x"));
+  EXPECT_FLOAT_EQ(0.54321f, MakeDoubleNumAsFloat(L".54321"));
+  EXPECT_FLOAT_EQ(0.54321f, MakeDoubleNumAsFloat(L"0.54321"));
+  EXPECT_FLOAT_EQ(0.54321f, MakeDoubleNumAsFloat(L"+0.54321"));
+  EXPECT_FLOAT_EQ(0.54321f, MakeDoubleNumAsFloat(L"   +0.54321"));
+  EXPECT_FLOAT_EQ(-0.54321f, MakeDoubleNumAsFloat(L"-.54321"));
+  EXPECT_FLOAT_EQ(-0.54321f, MakeDoubleNumAsFloat(L"-0.54321"));
+  EXPECT_FLOAT_EQ(-0.54321f, MakeDoubleNumAsFloat(L"  -0.54321"));
+  EXPECT_FLOAT_EQ(-0.054321f, MakeDoubleNumAsFloat(L"-0.54321e-1"));
+  EXPECT_FLOAT_EQ(-0.54321f, MakeDoubleNumAsFloat(L"-0.54321e0"));
+  EXPECT_FLOAT_EQ(-5.4321f, MakeDoubleNumAsFloat(L"-0.54321e1"));
+}