[Libreoffice-commits] core.git: Branch 'libreoffice-7-1' - sc/source

[Eike Rathke (via logerrit)]

 sc/source/ui/inc/tpformula.hxx    |    2 
 sc/source/ui/optdlg/tpformula.cxx |   87 ++++++++++++++++++++++++++------------
 2 files changed, 61 insertions(+), 28 deletions(-)

New commits:
commit 304d64cc27a729646522c7e16ad3d1d0aa2b1a12
Author:     Eike Rathke <erack at redhat.com>
AuthorDate: Fri Jun 11 19:02:24 2021 +0200
Commit:     Michael Stahl <michael.stahl at allotropia.de>
CommitDate: Mon Jun 14 11:15:02 2021 +0200

    Resolves: tdf#127013 differentiate non-/array separators and restrict
    
     This is a combination of 2 commits.
    
    Resolves: tdf#127013 differentiate non-/array separators and restrict
    
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117068
    Reviewed-by: Eike Rathke <erack at redhat.com>
    Tested-by: Jenkins
    (cherry picked from commit bb54d6d8241a06a6772052b77b67d6a4f686426c)
    
    Related: tdf#127013 Check loaded separators for validity
    
    ... not only if something is present at all.
    That way future restrictions can reset separators.
    
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117072
    Reviewed-by: Eike Rathke <erack at redhat.com>
    Tested-by: Jenkins
    (cherry picked from commit 381bc9d9acd461415cf81eef71ec3dd935a07b62)
    
    I738bcb9e052e8dbecb0a6cd07bbae8f4a8ea1c35
    
    Change-Id: I9ff6b816b0c7af9081954fa407a4d516a576a338
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117075
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>

diff --git a/sc/source/ui/inc/tpformula.hxx b/sc/source/ui/inc/tpformula.hxx
index 78d93c798f05..25077921b15c 100644
--- a/sc/source/ui/inc/tpformula.hxx
+++ b/sc/source/ui/inc/tpformula.hxx
@@ -44,7 +44,7 @@ private:
     void UpdateCustomCalcRadioButtons(bool bDefault);
     void LaunchCustomCalcSettings();
 
-    bool IsValidSeparator(const OUString& rSep) const;
+    bool IsValidSeparator(const OUString& rSep, bool bArray) const;
 
     DECL_LINK(ButtonHdl, weld::Button&, void);
     DECL_LINK(SepInsertTextHdl, OUString&, bool);
diff --git a/sc/source/ui/optdlg/tpformula.cxx b/sc/source/ui/optdlg/tpformula.cxx
index 440a509f853d..b4c0045d6469 100644
--- a/sc/source/ui/optdlg/tpformula.cxx
+++ b/sc/source/ui/optdlg/tpformula.cxx
@@ -129,40 +129,73 @@ void ScTpFormulaOptions::LaunchCustomCalcSettings()
     }
 }
 
-bool ScTpFormulaOptions::IsValidSeparator(const OUString& rSep) const
+bool ScTpFormulaOptions::IsValidSeparator(const OUString& rSep, bool bArray) const
 {
     if (rSep.getLength() != 1)
         // Must be one-character long.
         return false;
 
-    if (rSep.compareToAscii("a") >= 0 && rSep.compareToAscii("z") <= 0)
+    const sal_Unicode c = rSep[0];
+
+    if (c == mnDecSep)
+        // decimal separator is not allowed.
+        return false;
+
+    if (c <= 0x20 || c == 0x7f)
+        // Disallow non-printables including space and DEL.
         return false;
 
-    if (rSep.compareToAscii("A") >= 0 && rSep.compareToAscii("Z") <= 0)
+    if (('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || ('0' <= c && c <= '9'))
+        // Disallow alphanumeric.
         return false;
 
-    sal_Unicode c = rSep[0];
-    switch (c)
+    if (bArray)
     {
-        case '+':
-        case '-':
-        case '/':
-        case '*':
-        case '<':
-        case '>':
-        case '[':
-        case ']':
-        case '(':
-        case ')':
-        case '"':
-        case '\'':
-            // Disallowed characters.  Anything else we want to disallow ?
-            return false;
+        switch (c)
+        {
+            case '+':
+            case '-':
+            case '{':
+            case '}':
+            case '"':
+                // All following just to prevent confusion, they are not
+                // evaluated in inline arrays and theoretically would be
+                // possible.
+            case '%':
+            case '/':
+            case '*':
+            case '=':
+            case '<':
+            case '>':
+            case '[':
+            case ']':
+            case '(':
+            case ')':
+            case '\'':
+                // Disallowed characters.  Anything else we want to disallow ?
+                return false;
+        }
     }
-
-    if (c == mnDecSep)
-        // decimal separator is not allowed.
+    else if (c <= 0x7f)
+    {
+        switch (c)
+        {
+            default:
+                // Anything bad except the knowns.
+                return false;
+            case ';':
+            case ',':
+                ; // nothing
+        }
+    }
+    else
+    {
+        // Any Unicode character, would have to ask the compiler's localized
+        // symbol map whether it's a known symbol but not a separator
+        // (ocSep,ocArrayRowSep,ocArrayColSep), which we're about to set here.
+        // But really..
         return false;
+    }
 
     return true;
 }
@@ -181,7 +214,7 @@ IMPL_LINK( ScTpFormulaOptions, ButtonHdl, weld::Button&, rBtn, void )
 
 IMPL_LINK(ScTpFormulaOptions, SepInsertTextHdl, OUString&, rTest, bool)
 {
-    if (!IsValidSeparator(rTest) && !maOldSepValue.isEmpty())
+    if (!IsValidSeparator(rTest, false) && !maOldSepValue.isEmpty())
         // Invalid separator.  Restore the old value.
         rTest = maOldSepValue;
     return true;
@@ -190,7 +223,7 @@ IMPL_LINK(ScTpFormulaOptions, SepInsertTextHdl, OUString&, rTest, bool)
 IMPL_LINK(ScTpFormulaOptions, RowSepInsertTextHdl, OUString&, rTest, bool)
 {
     // Invalid separator or same as ColStr - Restore the old value.
-    if ((!IsValidSeparator(rTest) || rTest == mxEdSepArrayCol->get_text()) && !maOldSepValue.isEmpty())
+    if ((!IsValidSeparator(rTest, true) || rTest == mxEdSepArrayCol->get_text()) && !maOldSepValue.isEmpty())
         rTest = maOldSepValue;
     return true;
 }
@@ -198,7 +231,7 @@ IMPL_LINK(ScTpFormulaOptions, RowSepInsertTextHdl, OUString&, rTest, bool)
 IMPL_LINK(ScTpFormulaOptions, ColSepInsertTextHdl, OUString&, rTest, bool)
 {
     // Invalid separator or same as RowStr - Restore the old value.
-    if ((!IsValidSeparator(rTest) || rTest == mxEdSepArrayRow->get_text()) && !maOldSepValue.isEmpty())
+    if ((!IsValidSeparator(rTest, true) || rTest == mxEdSepArrayRow->get_text()) && !maOldSepValue.isEmpty())
         rTest = maOldSepValue;
     return true;
 }
@@ -331,9 +364,9 @@ void ScTpFormulaOptions::Reset(const SfxItemSet* rCoreSet)
     OUString aSepArrayRow = aOpt.GetFormulaSepArrayRow();
     OUString aSepArrayCol = aOpt.GetFormulaSepArrayCol();
 
-    if (aSep.getLength() == 1 && aSepArrayRow.getLength() == 1 && aSepArrayCol.getLength() == 1)
+    if (IsValidSeparator(aSep, false) && IsValidSeparator(aSepArrayRow, true) && IsValidSeparator(aSepArrayCol, true))
     {
-        // Each separator must be one character long.
+        // Each and all separators must be valid.
         mxEdSepFuncArg->set_text(aSep);
         mxEdSepArrayCol->set_text(aSepArrayCol);
         mxEdSepArrayRow->set_text(aSepArrayRow);