Resolves: tdf#127013 differentiate non-/array separators and restrict

Change-Id: I9ff6b816b0c7af9081954fa407a4d516a576a338
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117068
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins

2 files changed, 59 insertions, 26 deletions

diff --git a/sc/source/ui/inc/tpformula.hxx b/sc/source/ui/inc/tpformula.hxx
index 6f8a4c601cdf..abc14f4fde58 100644
--- a/sc/source/ui/inc/tpformula.hxx
+++ b/sc/source/ui/inc/tpformula.hxx
@@ -43,7 +43,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(ToggleHdl, weld::Toggleable&, void);
diff --git a/sc/source/ui/optdlg/tpformula.cxx b/sc/source/ui/optdlg/tpformula.cxx
index 24d91000867f..2f24e0714e71 100644
--- a/sc/source/ui/optdlg/tpformula.cxx
+++ b/sc/source/ui/optdlg/tpformula.cxx
@@ -131,40 +131,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;
 }
@@ -189,7 +222,7 @@ IMPL_LINK( ScTpFormulaOptions, ToggleHdl, weld::Toggleable&, 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;
@@ -198,7 +231,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;
 }
@@ -206,7 +239,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;
 }