Use span in CXFA_LocaleValue::GetDoubleNum()
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>
diff --git a/BUILD.gn b/BUILD.gn
index e78b2cf..c005575 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -2929,6 +2929,7 @@
"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 057eaeb..78f6b3b 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 @@
}
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++;
- }
-
- int32_t nIntegralLen = 0;
- while (cc < len) {
- if (str[cc] == '.' || !FXSYS_isDecimalDigit(str[cc]) ||
- nIntegralLen > 17) {
- 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++;
- 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';
- cc++;
- }
- nExponent = bExpSign ? -nExponent : nExponent;
- }
-
- double dValue = dwFractional / 4294967296.0;
- dValue = nIntegral + (nIntegral >= 0 ? dValue : -dValue);
- if (nExponent != 0)
- dValue *= FXSYS_pow(10, static_cast<float>(nExponent));
-
- return dValue;
+ 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;
}
- 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;
+
+ 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;
+ }
+ }
+ if (cc < str.size() && (str[cc] == 'E' || str[cc] == 'e')) {
+ cc++;
+ bool bExpSign = false;
+ if (cc < str.size()) {
+ if (str[cc] == '+') {
+ cc++;
+ } else if (str[cc] == '-') {
+ bExpSign = true;
+ cc++;
+ }
+ }
+ while (cc < str.size() && FXSYS_isDecimalDigit(str[cc]))
+ nExponent = nExponent * 10 + str[cc++] - '0';
+
+ nExponent = bExpSign ? -nExponent : nExponent;
+ }
+
+ 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 0000000..5b97eb3
--- /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"));
+}
