From cac38ba47f6ac541dded4f985e2b6dded2841705 Mon Sep 17 00:00:00 2001 From: Eike Rathke Date: Thu, 5 Jun 2014 16:34:08 +0200 Subject: unify the handling of string position arguments, fdo#75971 related Only two text functions had the full set of checks as introduced by the fix for fdo#75971. Let all text functions check their arguments in the same way. Additionally this will ease a transition to accept string lengths >64k in spreadsheet functions as now we have only one place that checks for SAL_MAX_UINT16 instead of having that scattered all over the place. (cherry picked from commit 14ce27cc045bd9bcbbfa3eac56cd99f2be08de1f) Conflicts: sc/source/core/tool/interpr1.cxx Change-Id: I454e617a59d0b3c2ca725047e7f8c7370bc0bb1f Reviewed-on: https://gerrit.libreoffice.org/9657 Tested-by: David Tardon Reviewed-by: David Tardon --- sc/source/core/inc/interpre.hxx | 48 +++++++++++++++++++++++++++++++++++ sc/source/core/tool/interpr1.cxx | 54 ++++++++++++++++++++++------------------ 2 files changed, 78 insertions(+), 24 deletions(-) (limited to 'sc') diff --git a/sc/source/core/inc/interpre.hxx b/sc/source/core/inc/interpre.hxx index cc675808a50e..aa2dd73982c6 100644 --- a/sc/source/core/inc/interpre.hxx +++ b/sc/source/core/inc/interpre.hxx @@ -365,6 +365,24 @@ void ScErrCell(); // special handling for void SetMaxIterationCount(sal_uInt16 n); inline void CurFmtToFuncFmt() { nFuncFmtType = nCurFmtType; nFuncFmtIndex = nCurFmtIndex; } + +/** Check if a double is suitable as string position or length argument. + + If fVal is Inf or NaN it is changed to -1, if it is less than 0 it is + sanitized to 0, if it is greater than some implementation defined max + string length it is sanitized to that max. + + @return TRUE if double value fVal is suitable as string argument and was + not sanitized. + FALSE if not and fVal was adapted. + */ +inline bool CheckStringPositionArgument( double & fVal ); + +/** Obtain a double suitable as string position or length argument. + Returns -1 if the number is Inf or NaN or less than 0 or greater than some + implementation defined max string length. */ +inline double GetStringPositionArgument(); + // Check for String overflow of rResult+rAdd and set error and erase rResult // if so. Return true if ok, false if overflow inline bool CheckStringResultLen( OUString& rResult, const OUString& rAdd ); @@ -900,6 +918,36 @@ inline bool ScInterpreter::MustHaveParamCountMin( short nAct, short nMin ) return false; } +inline bool ScInterpreter::CheckStringPositionArgument( double & fVal ) +{ + if (!rtl::math::isFinite( fVal)) + { + fVal = -1.0; + return false; + } + else if (fVal < 0.0) + { + fVal = 0.0; + return false; + } + else if (fVal > SAL_MAX_UINT16) + { + fVal = static_cast(SAL_MAX_UINT16); + return false; + } + return true; +} + +inline double ScInterpreter::GetStringPositionArgument() +{ + double fVal = rtl::math::approxFloor( GetDouble()); + if (!CheckStringPositionArgument( fVal)) + { + fVal = -1.0; + } + return fVal; +} + inline bool ScInterpreter::CheckStringResultLen( OUString& rResult, const OUString& rAdd ) { if ( (sal_uLong) rResult.getLength() + rAdd.getLength() > STRING_MAXLEN ) diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx index c4e6c56224b1..758ce156e057 100644 --- a/sc/source/core/tool/interpr1.cxx +++ b/sc/source/core/tool/interpr1.cxx @@ -7673,11 +7673,10 @@ void ScInterpreter::ScReplace() if ( MustHaveParamCount( GetByte(), 4 ) ) { OUString aNewStr = GetString().getString(); - double fCount = ::rtl::math::approxFloor( GetDouble()); - double fPos = ::rtl::math::approxFloor( GetDouble()); + double fCount = GetStringPositionArgument(); + double fPos = GetStringPositionArgument(); OUString aOldStr = GetString().getString(); - if (fPos < 1.0 || fPos > static_cast(STRING_MAXLEN) - || fCount < 0.0 || fCount > static_cast(STRING_MAXLEN)) + if (fPos < 1.0 || fCount < 0.0) PushIllegalArgument(); else { @@ -7800,8 +7799,8 @@ void ScInterpreter::ScLeft() sal_Int32 n; if (nParamCount == 2) { - double nVal = ::rtl::math::approxFloor(GetDouble()); - if ( rtl::math::isNan(nVal) || nVal < 0.0 || nVal > STRING_MAXLEN ) + double nVal = GetStringPositionArgument(); + if (nVal < 0.0) { PushIllegalArgument(); return ; @@ -7908,8 +7907,8 @@ void ScInterpreter::ScRightB() sal_Int32 n; if (nParamCount == 2) { - double nVal = ::rtl::math::approxFloor(GetDouble()); - if ( rtl::math::isNan(nVal) || nVal < 0.0 || nVal > STRING_MAXLEN ) + double nVal = GetStringPositionArgument(); + if ( nVal < 0.0 ) { PushIllegalArgument(); return ; @@ -7959,8 +7958,8 @@ void ScInterpreter::ScLeftB() sal_Int32 n; if (nParamCount == 2) { - double nVal = ::rtl::math::approxFloor(GetDouble()); - if ( nVal < 0.0 || nVal > STRING_MAXLEN ) + double nVal = GetStringPositionArgument(); + if ( nVal < 0.0 ) { PushIllegalArgument(); return ; @@ -7978,10 +7977,10 @@ void ScInterpreter::ScMidB() { if ( MustHaveParamCount( GetByte(), 3 ) ) { - double fAnz = ::rtl::math::approxFloor(GetDouble()); - double fAnfang = ::rtl::math::approxFloor(GetDouble()); + double fAnz = GetStringPositionArgument(); + double fAnfang = GetStringPositionArgument(); OUString aStr = GetString().getString(); - if (fAnfang < 1.0 || fAnz < 0.0 || fAnfang > double(STRING_MAXLEN) || fAnz > double(STRING_MAXLEN)) + if (fAnfang < 1.0 || fAnz < 0.0) PushIllegalArgument(); else { @@ -8002,8 +8001,8 @@ void ScInterpreter::ScRight() sal_Int32 n; if (nParamCount == 2) { - double nVal = ::rtl::math::approxFloor(GetDouble()); - if ( nVal < 0.0 || nVal > STRING_MAXLEN ) + double nVal = GetStringPositionArgument(); + if (nVal < 0.0) { PushIllegalArgument(); return ; @@ -8029,8 +8028,15 @@ void ScInterpreter::ScSearch() double fAnz; if (nParamCount == 3) { - fAnz = ::rtl::math::approxFloor(GetDouble()); - if (fAnz > double(STRING_MAXLEN)) + // This should use GetStringPositionArgument() but old versions up + // to LibreOffice 4.2.5 allowed and ignored 0 and negative values. + // It is unnecessary to break existing documents that "rely" on + // that behavior. Though ODFF constrains Start to be >=1. + /* TODO: fix this and possibly break those broken documents? */ + fAnz = rtl::math::approxFloor( GetDouble()); + if (fAnz < 1.0) + fAnz = 1.0; + else if (!CheckStringPositionArgument( fAnz)) { PushIllegalArgument(); return; @@ -8064,10 +8070,10 @@ void ScInterpreter::ScMid() { if ( MustHaveParamCount( GetByte(), 3 ) ) { - double fAnz = ::rtl::math::approxFloor(GetDouble()); - double fAnfang = ::rtl::math::approxFloor(GetDouble()); + double fAnz = GetStringPositionArgument(); + double fAnfang = GetStringPositionArgument(); OUString aStr = GetString().getString(); - if (fAnfang < 1.0 || fAnz < 0.0 || fAnfang > double(STRING_MAXLEN) || fAnz > double(STRING_MAXLEN)) + if (fAnfang < 1.0 || fAnz < 0.0) PushIllegalArgument(); else { @@ -8162,8 +8168,8 @@ void ScInterpreter::ScSubstitute() sal_Int32 nAnz; if (nParamCount == 4) { - double fAnz = ::rtl::math::approxFloor(GetDouble()); - if( fAnz < 1 || fAnz > STRING_MAXLEN ) + double fAnz = GetStringPositionArgument(); + if( fAnz < 1 ) { PushIllegalArgument(); return; @@ -8212,11 +8218,11 @@ void ScInterpreter::ScRept() { if ( MustHaveParamCount( GetByte(), 2 ) ) { - double fAnz = ::rtl::math::approxFloor(GetDouble()); + double fAnz = GetStringPositionArgument(); OUString aStr = GetString().getString(); if ( fAnz < 0.0 ) PushIllegalArgument(); - else if ( fAnz * aStr.getLength() > STRING_MAXLEN ) + else if ( fAnz * aStr.getLength() > SAL_MAX_UINT16 ) { PushError( errStringOverflow ); } -- cgit