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@redhat.com>
Reviewed-by: David Tardon <dtardon@redhat.com>

2 files changed, 78 insertions, 24 deletions

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<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 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<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 );
         }