Bugs in CJS_PublicMethods::ParseNumber().

Fix the bugs by removing ParseNumber() entirely.

For PDFium's JavaScript bindings, we want to get out of the
numeric conversion business and inflict that on V8 as
possible, avoiding platform-specific issue in strtod().

For other uses, there is a FX_atof() which is similarly
buggy, but we can consolidate the use.  Add an overloaded
FX_atof() to handle wide strings more simply.

BUG=pdfium:361
R=jochen@chromium.org

Review URL: https://codereview.chromium.org/1586203006 .

7 files changed, 97 insertions, 272 deletions

diff --git a/core/include/fxcrt/fx_string.h b/core/include/fxcrt/fx_string.h
index 1c1e237076..6abcf52c76 100644
--- a/core/include/fxcrt/fx_string.h
+++ b/core/include/fxcrt/fx_string.h
@@ -794,9 +794,7 @@ inline bool operator!=(const wchar_t* lhs, const CFX_WideString& rhs) {
 inline bool operator!=(const CFX_WideStringC& lhs, const CFX_WideString& rhs) {
   return rhs != lhs;
 }
-FX_FLOAT FX_atof(const CFX_ByteStringC& str);
-void FX_atonum(const CFX_ByteStringC& str, FX_BOOL& bInteger, void* pData);
-FX_STRSIZE FX_ftoa(FX_FLOAT f, FX_CHAR* buf);
+
 CFX_ByteString FX_UTF8Encode(const FX_WCHAR* pwsStr, FX_STRSIZE len);
 inline CFX_ByteString FX_UTF8Encode(const CFX_WideStringC& wsStr) {
   return FX_UTF8Encode(wsStr.GetPtr(), wsStr.GetLength());
@@ -805,4 +803,11 @@ inline CFX_ByteString FX_UTF8Encode(const CFX_WideString& wsStr) {
   return FX_UTF8Encode(wsStr.c_str(), wsStr.GetLength());
 }
 
+FX_FLOAT FX_atof(const CFX_ByteStringC& str);
+inline FX_FLOAT FX_atof(const CFX_WideStringC& wsStr) {
+  return FX_atof(FX_UTF8Encode(wsStr.GetPtr(), wsStr.GetLength()));
+}
+void FX_atonum(const CFX_ByteStringC& str, FX_BOOL& bInteger, void* pData);
+FX_STRSIZE FX_ftoa(FX_FLOAT f, FX_CHAR* buf);
+
 #endif  // CORE_INCLUDE_FXCRT_FX_STRING_H_
diff --git a/fpdfsdk/src/javascript/Field.cpp b/fpdfsdk/src/javascript/Field.cpp
index 959e8dcadd..d97001aae2 100644
--- a/fpdfsdk/src/javascript/Field.cpp
+++ b/fpdfsdk/src/javascript/Field.cpp
@@ -2738,19 +2738,7 @@ FX_BOOL Field::value(IJS_Context* cc,
         return FALSE;
       case FIELDTYPE_COMBOBOX:
       case FIELDTYPE_TEXTFIELD: {
-        CFX_WideString swValue = pFormField->GetValue();
-
-        double dRet;
-        FX_BOOL bDot;
-        if (CJS_PublicMethods::ConvertStringToNumber(swValue.c_str(), dRet,
-                                                     bDot)) {
-          if (bDot)
-            vp << dRet;
-          else
-            vp << dRet;
-        } else {
-          vp << swValue;
-        }
+        vp << pFormField->GetValue();
       } break;
       case FIELDTYPE_LISTBOX: {
         if (pFormField->CountSelectedItems() > 1) {
@@ -2766,40 +2754,18 @@ FX_BOOL Field::value(IJS_Context* cc,
           }
           vp << ValueArray;
         } else {
-          CFX_WideString swValue = pFormField->GetValue();
-
-          double dRet;
-          FX_BOOL bDot;
-          if (CJS_PublicMethods::ConvertStringToNumber(swValue.c_str(), dRet,
-                                                       bDot)) {
-            if (bDot)
-              vp << dRet;
-            else
-              vp << dRet;
-          } else {
-            vp << swValue;
-          }
+          vp << pFormField->GetValue();
         }
       } break;
       case FIELDTYPE_CHECKBOX:
       case FIELDTYPE_RADIOBUTTON: {
-        FX_BOOL bFind = FALSE;
+        bool bFind = false;
         for (int i = 0, sz = pFormField->CountControls(); i < sz; i++) {
-          if (!pFormField->GetControl(i)->IsChecked())
-            continue;
-
-          CFX_WideString swValue = pFormField->GetControl(i)->GetExportValue();
-          double dRet;
-          FX_BOOL bDotDummy;
-          if (CJS_PublicMethods::ConvertStringToNumber(swValue.c_str(), dRet,
-                                                       bDotDummy)) {
-            vp << dRet;
-          } else {
-            vp << swValue;
+          if (pFormField->GetControl(i)->IsChecked()) {
+            vp << pFormField->GetControl(i)->GetExportValue();
+            bFind = true;
+            break;
           }
-
-          bFind = TRUE;
-          break;
         }
         if (!bFind)
           vp << L"Off";
@@ -2809,7 +2775,7 @@ FX_BOOL Field::value(IJS_Context* cc,
         break;
     }
   }
-
+  vp.MaybeCoerceToNumber();
   return TRUE;
 }
 
diff --git a/fpdfsdk/src/javascript/JS_Value.cpp b/fpdfsdk/src/javascript/JS_Value.cpp
index bd00adcf49..cfa565e4d4 100644
--- a/fpdfsdk/src/javascript/JS_Value.cpp
+++ b/fpdfsdk/src/javascript/JS_Value.cpp
@@ -19,8 +19,6 @@ static double GetNan() {
   return *(double*)g_nan;
 }
 
-/* ---------------------------- CJS_Value ---------------------------- */
-
 CJS_Value::CJS_Value(CJS_Runtime* pRuntime)
     : m_eType(VT_unknown), m_pJSRuntime(pRuntime) {
 }
@@ -98,9 +96,6 @@ void CJS_Value::Detach() {
   m_eType = VT_unknown;
 }
 
-/* ----------------------------------------------------------------------------------------
- */
-
 int CJS_Value::ToInt() const {
   return FXJS_ToInt32(m_pJSRuntime->GetIsolate(), m_pValue);
 }
@@ -146,8 +141,26 @@ v8::Local<v8::Array> CJS_Value::ToV8Array() const {
   return v8::Local<v8::Array>();
 }
 
-/* ----------------------------------------------------------------------------------------
- */
+void CJS_Value::MaybeCoerceToNumber() {
+  bool bAllowNaN = false;
+  if (m_eType == VT_string) {
+    CFX_ByteString bstr = ToCFXByteString();
+    if (bstr.GetLength() == 0)
+      return;
+    if (bstr == "NaN")
+      bAllowNaN = true;
+  }
+  v8::TryCatch(m_pJSRuntime->GetIsolate());
+  v8::MaybeLocal<v8::Number> maybeNum =
+      m_pValue->ToNumber(m_pJSRuntime->GetIsolate()->GetCurrentContext());
+  if (maybeNum.IsEmpty())
+    return;
+  v8::Local<v8::Number> num = maybeNum.ToLocalChecked();
+  if (std::isnan(num->Value()) && !bAllowNaN)
+    return;
+  m_pValue = num;
+  m_eType = VT_number;
+}
 
 void CJS_Value::operator=(int iValue) {
   m_pValue = FXJS_NewNumber(m_pJSRuntime->GetIsolate(), iValue);
@@ -217,9 +230,6 @@ void CJS_Value::operator=(CJS_Value value) {
   m_pJSRuntime = value.m_pJSRuntime;
 }
 
-/* ----------------------------------------------------------------------------------------
- */
-
 CJS_Value::Type CJS_Value::GetType() const {
   if (m_pValue.IsEmpty())
     return VT_unknown;
diff --git a/fpdfsdk/src/javascript/JS_Value.h b/fpdfsdk/src/javascript/JS_Value.h
index 8517b76dac..20a6e38b46 100644
--- a/fpdfsdk/src/javascript/JS_Value.h
+++ b/fpdfsdk/src/javascript/JS_Value.h
@@ -62,6 +62,11 @@ class CJS_Value {
   v8::Local<v8::Array> ToV8Array() const;
   v8::Local<v8::Value> ToV8Value() const;
 
+  // Replace the current |m_pValue| with a v8::Number if possible
+  // to make one from the current |m_pValue|, updating |m_eType|
+  // as appropriate to indicate the result.
+  void MaybeCoerceToNumber();
+
   void operator=(int iValue);
   void operator=(bool bValue);
   void operator=(double);
diff --git a/fpdfsdk/src/javascript/PublicMethods.cpp b/fpdfsdk/src/javascript/PublicMethods.cpp
index d2f7fb09ac..3c6d36fdd8 100644
--- a/fpdfsdk/src/javascript/PublicMethods.cpp
+++ b/fpdfsdk/src/javascript/PublicMethods.cpp
@@ -194,149 +194,6 @@ CFX_ByteString CJS_PublicMethods::StrTrim(const FX_CHAR* pStr) {
   return StrRTrim(StrLTrim(pStr));
 }
 
-double CJS_PublicMethods::ParseNumber(const FX_WCHAR* swSource,
-                                      FX_BOOL& bAllDigits,
-                                      FX_BOOL& bDot,
-                                      FX_BOOL& bSign,
-                                      FX_BOOL& bKXJS) {
-  bDot = FALSE;
-  bSign = FALSE;
-  bKXJS = FALSE;
-
-  FX_BOOL bDigitExist = FALSE;
-
-  const FX_WCHAR* p = swSource;
-  wchar_t c;
-
-  const FX_WCHAR* pStart = NULL;
-  const FX_WCHAR* pEnd = NULL;
-
-  while ((c = *p)) {
-    if (!pStart && c != L' ') {
-      pStart = p;
-    }
-
-    pEnd = p;
-    p++;
-  }
-
-  if (!pStart) {
-    bAllDigits = FALSE;
-    return 0;
-  }
-
-  while (pEnd != pStart) {
-    if (*pEnd == L' ')
-      pEnd--;
-    else
-      break;
-  }
-
-  double dRet = 0;
-  p = pStart;
-  bAllDigits = TRUE;
-  CFX_WideString swDigits;
-
-  while (p <= pEnd) {
-    c = *p;
-
-    if (FXSYS_iswdigit(c)) {
-      swDigits += c;
-      bDigitExist = TRUE;
-    } else {
-      switch (c) {
-        case L' ':
-          bAllDigits = FALSE;
-          break;
-        case L'.':
-        case L',':
-          if (!bDot) {
-            if (bDigitExist) {
-              swDigits += L'.';
-            } else {
-              swDigits += L'0';
-              swDigits += L'.';
-              bDigitExist = TRUE;
-            }
-
-            bDot = TRUE;
-            break;
-          }
-        case 'e':
-        case 'E':
-          if (!bKXJS) {
-            p++;
-            c = *p;
-            if (c == '+' || c == '-') {
-              bKXJS = TRUE;
-              swDigits += 'e';
-              swDigits += c;
-            }
-            break;
-          }
-        case L'-':
-          if (!bDigitExist && !bSign) {
-            swDigits += c;
-            bSign = TRUE;
-            break;
-          }
-        default:
-          bAllDigits = FALSE;
-
-          if (p != pStart && !bDot && bDigitExist) {
-            swDigits += L'.';
-            bDot = TRUE;
-          } else {
-            bDot = FALSE;
-            bDigitExist = FALSE;
-            swDigits = L"";
-          }
-          break;
-      }
-    }
-
-    p++;
-  }
-
-  if (swDigits.GetLength() > 0 && swDigits.GetLength() < 17) {
-    CFX_ByteString sDigits = swDigits.UTF8Encode();
-
-    if (bKXJS) {
-      dRet = atof(sDigits);
-    } else {
-      if (bDot) {
-        char* pStopString;
-        dRet = ::strtod(sDigits, &pStopString);
-      } else {
-        dRet = atol(sDigits);
-      }
-    }
-  }
-
-  return dRet;
-}
-
-double CJS_PublicMethods::ParseStringToNumber(const FX_WCHAR* swSource) {
-  FX_BOOL bAllDigits = FALSE;
-  FX_BOOL bDot = FALSE;
-  FX_BOOL bSign = FALSE;
-  FX_BOOL bKXJS = FALSE;
-
-  return ParseNumber(swSource, bAllDigits, bDot, bSign, bKXJS);
-}
-
-FX_BOOL CJS_PublicMethods::ConvertStringToNumber(const FX_WCHAR* swSource,
-                                                 double& dRet,
-                                                 FX_BOOL& bDot) {
-  FX_BOOL bAllDigits = FALSE;
-  FX_BOOL bSign = FALSE;
-  FX_BOOL bKXJS = FALSE;
-
-  dRet = ParseNumber(swSource, bAllDigits, bDot, bSign, bKXJS);
-
-  return bAllDigits;
-}
-
 CJS_Array CJS_PublicMethods::AF_MakeArrayFromList(CJS_Runtime* pRuntime,
                                                   CJS_Value val) {
   CJS_Array StrArray(pRuntime);
@@ -1344,71 +1201,57 @@ FX_BOOL CJS_PublicMethods::AFDate_FormatEx(IJS_Context* cc,
 }
 
 double CJS_PublicMethods::MakeInterDate(CFX_WideString strValue) {
-  int nHour;
-  int nMin;
-  int nSec;
-  int nYear;
-  int nMonth;
-  int nDay;
-
   CFX_WideStringArray wsArray;
-  CFX_WideString sMonth = L"";
   CFX_WideString sTemp = L"";
-  int nSize = strValue.GetLength();
-
-  for (int i = 0; i < nSize; i++) {
+  for (int i = 0; i < strValue.GetLength(); ++i) {
     FX_WCHAR c = strValue.GetAt(i);
     if (c == L' ' || c == L':') {
       wsArray.Add(sTemp);
       sTemp = L"";
       continue;
     }
-
     sTemp += c;
   }
-
   wsArray.Add(sTemp);
   if (wsArray.GetSize() != 8)
     return 0;
 
+  int nMonth = 1;
   sTemp = wsArray[1];
   if (sTemp.Compare(L"Jan") == 0)
     nMonth = 1;
-  if (sTemp.Compare(L"Feb") == 0)
+  else if (sTemp.Compare(L"Feb") == 0)
     nMonth = 2;
-  if (sTemp.Compare(L"Mar") == 0)
+  else if (sTemp.Compare(L"Mar") == 0)
     nMonth = 3;
-  if (sTemp.Compare(L"Apr") == 0)
+  else if (sTemp.Compare(L"Apr") == 0)
     nMonth = 4;
-  if (sTemp.Compare(L"May") == 0)
+  else if (sTemp.Compare(L"May") == 0)
     nMonth = 5;
-  if (sTemp.Compare(L"Jun") == 0)
+  else if (sTemp.Compare(L"Jun") == 0)
     nMonth = 6;
-  if (sTemp.Compare(L"Jul") == 0)
+  else if (sTemp.Compare(L"Jul") == 0)
     nMonth = 7;
-  if (sTemp.Compare(L"Aug") == 0)
+  else if (sTemp.Compare(L"Aug") == 0)
     nMonth = 8;
-  if (sTemp.Compare(L"Sep") == 0)
+  else if (sTemp.Compare(L"Sep") == 0)
     nMonth = 9;
-  if (sTemp.Compare(L"Oct") == 0)
+  else if (sTemp.Compare(L"Oct") == 0)
     nMonth = 10;
-  if (sTemp.Compare(L"Nov") == 0)
+  else if (sTemp.Compare(L"Nov") == 0)
     nMonth = 11;
-  if (sTemp.Compare(L"Dec") == 0)
+  else if (sTemp.Compare(L"Dec") == 0)
     nMonth = 12;
 
-  nDay = (int)ParseStringToNumber(wsArray[2].c_str());
-  nHour = (int)ParseStringToNumber(wsArray[3].c_str());
-  nMin = (int)ParseStringToNumber(wsArray[4].c_str());
-  nSec = (int)ParseStringToNumber(wsArray[5].c_str());
-  nYear = (int)ParseStringToNumber(wsArray[7].c_str());
-
+  int nDay = FX_atof(wsArray[2]);
+  int nHour = FX_atof(wsArray[3]);
+  int nMin = FX_atof(wsArray[4]);
+  int nSec = FX_atof(wsArray[5]);
+  int nYear = FX_atof(wsArray[7]);
   double dRet = JS_MakeDate(JS_MakeDay(nYear, nMonth - 1, nDay),
                             JS_MakeTime(nHour, nMin, nSec, 0));
-
-  if (JS_PortIsNan(dRet)) {
+  if (JS_PortIsNan(dRet))
     dRet = JS_DateParse(strValue.c_str());
-  }
 
   return dRet;
 }
@@ -1866,14 +1709,17 @@ FX_BOOL CJS_PublicMethods::AFMakeNumber(IJS_Context* cc,
                                         const std::vector<CJS_Value>& params,
                                         CJS_Value& vRet,
                                         CFX_WideString& sError) {
+  CJS_Context* pContext = (CJS_Context*)cc;
   if (params.size() != 1) {
-    CJS_Context* pContext = (CJS_Context*)cc;
-    ASSERT(pContext);
-
     sError = JSGetStringFromID(pContext, IDS_STRING_JSPARAMERROR);
     return FALSE;
   }
-  vRet = ParseStringToNumber(params[0].ToCFXWideString().c_str());
+  CFX_WideString ws = params[0].ToCFXWideString();
+  ws.Replace(L",", L".");
+  vRet = ws;
+  vRet.MaybeCoerceToNumber();
+  if (vRet.GetType() != CJS_Value::VT_number)
+    vRet = 0;
   return TRUE;
 }
 
@@ -1913,38 +1759,40 @@ FX_BOOL CJS_PublicMethods::AFSimple_Calculate(
     for (int j = 0, jsz = pInterForm->CountFields(wsFieldName); j < jsz; j++) {
       if (CPDF_FormField* pFormField = pInterForm->GetField(j, wsFieldName)) {
         double dTemp = 0.0;
-
         switch (pFormField->GetFieldType()) {
           case FIELDTYPE_TEXTFIELD:
           case FIELDTYPE_COMBOBOX: {
-            dTemp = ParseStringToNumber(pFormField->GetValue().c_str());
-            break;
-          }
+            CFX_WideString trimmed = pFormField->GetValue();
+            trimmed.TrimRight();
+            trimmed.TrimLeft();
+            dTemp = FX_atof(trimmed);
+          } break;
           case FIELDTYPE_PUSHBUTTON: {
             dTemp = 0.0;
-            break;
-          }
+          } break;
           case FIELDTYPE_CHECKBOX:
           case FIELDTYPE_RADIOBUTTON: {
             dTemp = 0.0;
             for (int c = 0, csz = pFormField->CountControls(); c < csz; c++) {
               if (CPDF_FormControl* pFormCtrl = pFormField->GetControl(c)) {
                 if (pFormCtrl->IsChecked()) {
-                  dTemp +=
-                      ParseStringToNumber(pFormCtrl->GetExportValue().c_str());
+                  CFX_WideString trimmed = pFormCtrl->GetExportValue();
+                  trimmed.TrimRight();
+                  trimmed.TrimLeft();
+                  dTemp = FX_atof(trimmed);
                   break;
                 }
               }
             }
-            break;
-          }
+          } break;
           case FIELDTYPE_LISTBOX: {
-            if (pFormField->CountSelectedItems() > 1)
-              break;
-
-            dTemp = ParseStringToNumber(pFormField->GetValue().c_str());
-            break;
-          }
+            if (pFormField->CountSelectedItems() <= 1) {
+              CFX_WideString trimmed = pFormField->GetValue();
+              trimmed.TrimRight();
+              trimmed.TrimLeft();
+              dTemp = FX_atof(trimmed);
+            }
+          } break;
           default:
             break;
         }
diff --git a/fpdfsdk/src/javascript/PublicMethods.h b/fpdfsdk/src/javascript/PublicMethods.h
index 013c4ce1a4..26640bc40b 100644
--- a/fpdfsdk/src/javascript/PublicMethods.h
+++ b/fpdfsdk/src/javascript/PublicMethods.h
@@ -144,18 +144,9 @@ class CJS_PublicMethods : public CJS_Object {
                                 bool* bWrongFormat);
   static CFX_WideString MakeFormatDate(double dDate,
                                        const CFX_WideString& format);
-  static FX_BOOL ConvertStringToNumber(const FX_WCHAR* swSource,
-                                       double& dRet,
-                                       FX_BOOL& bDot);
-  static double ParseStringToNumber(const FX_WCHAR* swSource);
   static double ParseNormalDate(const CFX_WideString& value,
                                 bool* bWrongFormat);
   static double MakeInterDate(CFX_WideString strValue);
-  static double ParseNumber(const FX_WCHAR* swSource,
-                            FX_BOOL& bAllDigits,
-                            FX_BOOL& bDot,
-                            FX_BOOL& bSign,
-                            FX_BOOL& bKXJS);
 
  public:
   static CFX_WideString StrLTrim(const FX_WCHAR* pStr);
diff --git a/testing/resources/javascript/bug_361_expected.txt b/testing/resources/javascript/bug_361_expected.txt
index f7f7a562bb..04d72bce20 100644
--- a/testing/resources/javascript/bug_361_expected.txt
+++ b/testing/resources/javascript/bug_361_expected.txt
@@ -12,31 +12,31 @@ Alert: Answer for "                          4
 Alert: Answer for "4 3 2 1" is: string 4 3 2 1
 Alert: Answer for "-4" is: number -4
 Alert: Answer for "23.00000001" is: number 23.00000001
-Alert: Answer for "23.00000000000000001" is: number 0
+Alert: Answer for "23.00000000000000001" is: number 23
 Alert: Answer for "4e+25" is: number 4e+25
-Alert: Answer for "40000000000000000000000000" is: number 0
-Alert: Answer for "25,5" is: number 25.5
+Alert: Answer for "40000000000000000000000000" is: number 4e+25
+Alert: Answer for "25,5" is: string 25,5
 Alert: Answer for "1e+5" is: number 100000
-Alert: Answer for "1e5" is: number 1
+Alert: Answer for "1e5" is: number 100000
 Alert: Answer for "1e-5" is: number 0.00001
 Alert: Answer for "-1e-5" is: number -0.00001
-Alert: Answer for "1.2e5" is: number 1.2
-Alert: Answer for "Infinity" is: string Infinity
-Alert: Answer for "Infinity" is: string Infinity
+Alert: Answer for "1.2e5" is: number 120000
+Alert: Answer for "Infinity" is: number Infinity
+Alert: Answer for "Infinity" is: number Infinity
 Alert: Answer for "INFINITY" is: string INFINITY
 Alert: Answer for "INF" is: string INF
-Alert: Answer for "NaN" is: string NaN
-Alert: Answer for "NaN" is: string NaN
+Alert: Answer for "NaN" is: number NaN
+Alert: Answer for "NaN" is: number NaN
 Alert: Answer for "NAN" is: string NAN
-Alert: Answer for "0x100" is: string 0x100
+Alert: Answer for "0x100" is: number 256
 Alert: Answer for "0x100.1" is: string 0x100.1
 Alert: Answer for "0x100,1" is: string 0x100,1
 Alert: Answer for "0x100x1" is: string 0x100x1
 Alert: Answer for "123x6" is: string 123x6
 Alert: Answer for "123xy6" is: string 123xy6
 Alert: Answer for "123.y6" is: string 123.y6
-Alert: Answer for "1,000,000" is: number 1
-Alert: Answer for "1.2.3" is: number 1.2
+Alert: Answer for "1,000,000" is: string 1,000,000
+Alert: Answer for "1.2.3" is: string 1.2.3
 Alert: Answer for "1-3" is: string 1-3
 Alert: Answer for "1+3" is: string 1+3
 Alert: Answer for "1.-3" is: string 1.-3