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@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@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@allotropia.de>

2 files changed, 61 insertions, 28 deletions

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