From 8b4a3c7ef32b2ffb4874e4cc65b38ee555d8806e Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 17 Apr 2018 17:59:58 +0000 Subject: 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 Reviewed-by: dsinclair --- BUILD.gn | 1 + xfa/fxfa/parser/cxfa_localevalue.cpp | 125 ++++++++++++-------------- xfa/fxfa/parser/cxfa_localevalue_unittest.cpp | 37 ++++++++ 3 files changed, 95 insertions(+), 68 deletions(-) create mode 100644 xfa/fxfa/parser/cxfa_localevalue_unittest.cpp 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 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(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(nExponent)); - - return dValue; + nExponent = bExpSign ? -nExponent : nExponent; } - return 0; + + double dValue = nIntegral + (bNegative ? -fraction : fraction); + if (nExponent != 0) + dValue *= FXSYS_pow(10, static_cast(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( + 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")); +} -- cgit v1.2.3