[Libreoffice-commits] core.git: Branch 'libreoffice-4-2' - sc/source
Eike Rathke
erack at redhat.com
Mon Jun 9 09:21:13 PDT 2014
sc/source/core/inc/interpre.hxx | 48 ++++++++++++++++++++++++++++++++++
sc/source/core/tool/interpr1.cxx | 54 +++++++++++++++++++++------------------
2 files changed, 78 insertions(+), 24 deletions(-)
New commits:
commit 9e49e49709935d594fa49cbccc4ffbcb4e90400f
Author: Eike Rathke <erack at redhat.com>
Date: Thu Jun 5 16:34:08 2014 +0200
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 <dtardon at redhat.com>
Reviewed-by: David Tardon <dtardon at redhat.com>
diff --git a/sc/source/core/inc/interpre.hxx b/sc/source/core/inc/interpre.hxx
index cc67580..aa2dd73 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<double>(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 c4e6c56..758ce15 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<double>(STRING_MAXLEN)
- || fCount < 0.0 || fCount > static_cast<double>(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 );
}
More information about the Libreoffice-commits
mailing list