summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Sepez <tsepez@chromium.org>2016-01-20 11:34:01 -0800
committerTom Sepez <tsepez@chromium.org>2016-01-20 11:34:01 -0800
commitf13d510cf267c27f4c123494de67670ec201cedc (patch)
tree2b5e279bef4fb9eb4a3a7e963cae93c65bc80fba
parentb196c7bebad66c9938d2705ccf64961bcdd774e2 (diff)
downloadpdfium-f13d510cf267c27f4c123494de67670ec201cedc.tar.xz
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 .
-rw-r--r--core/include/fxcrt/fx_string.h11
-rw-r--r--fpdfsdk/src/javascript/Field.cpp50
-rw-r--r--fpdfsdk/src/javascript/JS_Value.cpp30
-rw-r--r--fpdfsdk/src/javascript/JS_Value.h5
-rw-r--r--fpdfsdk/src/javascript/PublicMethods.cpp240
-rw-r--r--fpdfsdk/src/javascript/PublicMethods.h9
-rw-r--r--testing/resources/javascript/bug_361_expected.txt24
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