[Spreadsheet] fix isValidAlias() (FreeCAD#18567)

mwganson · web-flow · commit c0210c2d3c37 · 2024-12-23T14:18:30.000-03:00
diff --git a/src/Mod/Spreadsheet/App/PropertySheet.cpp b/src/Mod/Spreadsheet/App/PropertySheet.cpp
@@ -129,34 +129,36 @@ const Cell* PropertySheet::getValueFromAlias(const std::string& alias) const
 
 bool PropertySheet::isValidCellAddressName(const std::string& candidate)
 {
-    static const boost::regex gen("^[A-Za-z][_A-Za-z0-9]*$");
-    boost::cmatch cm;
-
     /* Check if it matches a cell reference */
-    if (boost::regex_match(candidate.c_str(), cm, gen)) {
-        static const boost::regex e("\\${0,1}([A-Z]{1,2})\\${0,1}([0-9]{1,5})");
+    static const boost::regex e("\\${0,1}([A-Z]{1,2})\\${0,1}([0-9]{1,5})");
+    boost::cmatch cm;
 
-        if (boost::regex_match(candidate.c_str(), cm, e)) {
-            const boost::sub_match<const char*> colstr = cm[1];
-            const boost::sub_match<const char*> rowstr = cm[2];
+    if (boost::regex_match(candidate.c_str(), cm, e)) {
+        const boost::sub_match<const char*> colstr = cm[1];
+        const boost::sub_match<const char*> rowstr = cm[2];
 
-            if (App::validRow(rowstr.str()) >= 0 && App::validColumn(colstr.str())) {
-                return true;
-            }
+        if (App::validRow(rowstr.str()) >= 0 && App::validColumn(colstr.str())) {
+            return true;
         }
     }
     return false;
 }
 
 bool PropertySheet::isValidAlias(const std::string& candidate)
 {
+    /* Ensure it only contains allowed characters */
+    static const boost::regex gen("^[A-Za-z][_A-Za-z0-9]*$");
+    boost::cmatch cm;
+    if (!boost::regex_match(candidate.c_str(), cm, gen)) {
+        return false;
+    }
 
     /* Check if it is used before */
     if (getValueFromAlias(candidate)) {
         return false;
     }
 
-    /* check if it would be a valid cell address name, e.g. "A2" or "C3" */
+    /* Check if it would be a valid cell address name, e.g. "A2" or "C3" */
     if (isValidCellAddressName(candidate)) {
         return false;
     }
diff --git a/tests/src/Mod/Spreadsheet/App/PropertySheet.cpp b/tests/src/Mod/Spreadsheet/App/PropertySheet.cpp
@@ -76,7 +76,12 @@ TEST_F(PropertySheetTest, validAliases)  // NOLINT
 
 TEST_F(PropertySheetTest, invalidAliases)  // NOLINT
 {
-    std::vector<std::string> invalidAliases {"A1", "ZZ1234", "mm"};
+    std::vector<std::string> invalidAliases {"A1",
+                                             "ZZ1234",
+                                             "mm",
+                                             "no spaces allowed",
+                                             "\'NoLeadingQuotes"};
+
     for (const auto& name : invalidAliases) {
         EXPECT_FALSE(propertySheet()->isValidAlias(name))
             << "\"" << name << "\" was accepted as an alias name, and should not be";
