From 9f445fc7355255f091782c40a8a42ceea3b5dc40 Mon Sep 17 00:00:00 2001
From: orbitcowboy <orbitcowboy@web.de>
Date: Fri, 12 Jun 2020 16:06:43 +0200
Subject: [PATCH] Library: simplified code and added test cases for validating
 <valid>-tag expressions

---
 lib/library.cpp      | 81 ++++++++++++++++++++++++--------------------
 lib/library.h        |  2 ++
 test/testlibrary.cpp | 21 ++++++++++++
 3 files changed, 67 insertions(+), 37 deletions(-)

diff --git a/lib/library.cpp b/lib/library.cpp
index cb955063e..ea9d0bbe7 100644
--- a/lib/library.cpp
+++ b/lib/library.cpp
@@ -708,46 +708,11 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
                 else if (argnodename == "valid") {
                     // Validate the validation expression
                     const char *p = argnode->GetText();
-                    bool error = false;
-                    bool range = false;
-                    bool has_dot = false;
-                    bool has_E = false;
-
-                    if (!p)
-                        return Error(BAD_ATTRIBUTE_VALUE, "\"\"");
-
-                    error = *p == '.';
-                    for (; *p; p++) {
-                        if (std::isdigit(*p))
-                            error |= (*(p+1) == '-');
-                        else if (*p == ':') {
-                            error |= range | (*(p+1) == '.');
-                            range = true;
-                            has_dot = false;
-                            has_E = false;
-                        } else if (*p == '-')
-                            error |= (!std::isdigit(*(p+1)));
-                        else if (*p == ',') {
-                            range = false;
-                            error |= *(p+1) == '.';
-                            has_dot = false;
-                            has_E = false;
-                        } else if (*p == '.') {
-                            error |= has_dot | (!std::isdigit(*(p+1)));
-                            has_dot = true;
-                        } else if (*p == 'E' || *p == 'e') {
-                            error |= has_E;
-                            has_E = true;
-                        } else
-                            error = true;
-                    }
-                    if (error)
-                        return Error(BAD_ATTRIBUTE_VALUE, argnode->GetText());
-
+                    if (!isCompliantValidationExpression(p))
+                        return Error(BAD_ATTRIBUTE_VALUE, (!p ? "\"\"" : argnode->GetText()));
                     // Set validation expression
                     ac.valid = argnode->GetText();
                 }
-
                 else if (argnodename == "minsize") {
                     const char *typeattr = argnode->Attribute("type");
                     if (!typeattr)
@@ -1242,6 +1207,48 @@ const Library::WarnInfo* Library::getWarnInfo(const Token* ftok) const
     return &i->second;
 }
 
+bool Library::isCompliantValidationExpression(const char* p)
+{
+    if (!p)
+        return false;
+
+    bool error = false;
+    bool range = false;
+    bool has_dot = false;
+    bool has_E = false;
+
+    error = *p == '.';
+    for (; *p; p++) {
+        if (std::isdigit(*p))
+            error |= (*(p + 1) == '-');
+        else if (*p == ':') {
+            error |= range | (*(p + 1) == '.');
+            range = true;
+            has_dot = false;
+            has_E = false;
+        }
+        else if ((*p == '-')|| (*p == '+'))
+            error |= (!std::isdigit(*(p + 1)));
+        else if (*p == ',') {
+            range = false;
+            error |= *(p + 1) == '.';
+            has_dot = false;
+            has_E = false;
+        }
+        else if (*p == '.') {
+            error |= has_dot | (!std::isdigit(*(p + 1)));
+            has_dot = true;
+        }
+        else if (*p == 'E' || *p == 'e') {
+            error |= has_E;
+            has_E = true;
+        }
+        else
+            return false;
+    }
+    return !error;
+}
+
 bool Library::formatstr_function(const Token* ftok) const
 {
     if (isNotLibraryFunction(ftok))
diff --git a/lib/library.h b/lib/library.h
index a383b2317..f7975faa9 100644
--- a/lib/library.h
+++ b/lib/library.h
@@ -142,6 +142,8 @@ public:
         mNoReturn[funcname] = noreturn;
     }
 
+    static bool isCompliantValidationExpression(const char* p);
+
     /** is allocation type memory? */
     static bool ismemory(const int id) {
         return ((id > 0) && ((id & 1) == 0));
diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp
index a24c034a4..fe33ce9b0 100644
--- a/test/testlibrary.cpp
+++ b/test/testlibrary.cpp
@@ -40,6 +40,7 @@ private:
     Settings settings;
 
     void run() OVERRIDE {
+        TEST_CASE(isCompliantValidationExpression);
         TEST_CASE(empty);
         TEST_CASE(function);
         TEST_CASE(function_match_scope);
@@ -72,6 +73,26 @@ private:
         return library.load(doc);
     }
 
+    void isCompliantValidationExpression()
+    {
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1:"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression(":1"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1,42"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1,-42"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1.0:42.0"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1.175494e-38:3.402823e+38"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1.175494e-38,3.402823e+38"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1.175494e-38:"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression(":1.175494e-38"));
+        ASSERT_EQUALS(true, Library::isCompliantValidationExpression(":42.0"));
+
+        // Robustness tests
+        ASSERT_EQUALS(false, Library::isCompliantValidationExpression(nullptr));
+        ASSERT_EQUALS(false, Library::isCompliantValidationExpression("x"));
+    }
+
     void empty() const {
         // Reading an empty library file is considered to be OK
         const char xmldata[] = "<?xml version=\"1.0\"?>\n<def/>";