Adding element id validity checks

CMakeLists.txt

@@ -83,8 +83,8 @@ ENDIF (MSVC)
 
 
 # configure file groups
-file(GLOB EPANET_SOURCES RELATIVE ${PROJECT_SOURCE_DIR} src/*.c)
+file(GLOB EPANET_SOURCES RELATIVE ${PROJECT_SOURCE_DIR} src/*.c src/util/cstr_helper.c)
-file(GLOB EPANET_LIB_ALL RELATIVE ${PROJECT_SOURCE_DIR} src/*)
+file(GLOB EPANET_LIB_ALL RELATIVE ${PROJECT_SOURCE_DIR} src/* src/util/cstr_helper.*)
 # exclude epanet python API from the default build
 list(REMOVE_ITEM EPANET_LIB_ALL "src/epanet_py.c")
 source_group("Library" FILES ${EPANET_LIB_ALL})

src/epanet.c

@@ -30,6 +30,8 @@
 #include "text.h"
 #include "enumstxt.h"
 
+#include "util/cstr_helper.h"
+
 #ifdef _WIN32
 #define snprintf _snprintf
 #endif
@@ -1731,6 +1733,9 @@ int DLLEXPORT EN_addnode(EN_Project p, char *id, int nodeType)
     if (!p->Openflag) return 102;
     if (hyd->OpenHflag || qual->OpenQflag) return 262;
 
+    // Check if id contains invalid characters
+    if (!cstr_isvalid(id)) return 250;
+
     // Check if a node with same id already exists
     if (EN_getnodeindex(p, id, &i) == 0) return 215;
 
@@ -2939,6 +2944,9 @@ int DLLEXPORT EN_addlink(EN_Project p, char *id, int linkType,
     if (!p->Openflag) return 102;
     if (p->hydraul.OpenHflag || p->quality.OpenQflag) return 262;
 
+    // Check if id contains invalid characters
+    if (!cstr_isvalid(id)) return 250;
+
     // Check if a link with same id already exists
     if (EN_getlinkindex(p, id, &i) == 0) return 215;
 
@@ -3951,6 +3959,9 @@ int DLLEXPORT EN_addpattern(EN_Project p, char *id)
     if (!p->Openflag) return 102;
     if (EN_getpatternindex(p, id, &i) == 0) return 215;
 
+    // Check is id name contains invalid characters
+    if (!cstr_isvalid(id)) return 250;
+
     // Check that id name is not too long
     if (strlen(id) > MAXID) return 250;
 
@@ -4219,6 +4230,9 @@ int DLLEXPORT EN_addcurve(EN_Project p, char *id)
     if (!p->Openflag) return 102;
     if (EN_getcurveindex(p, id, &i) == 0) return 215;
 
+    // Check is id name contains invalid characters
+    if (!cstr_isvalid(id)) return 250;
+
     // Check that id name is not too long
     if (strlen(id) > MAXID) return 250;
 

src/util/cstr_helper.c

@@ -36,11 +36,12 @@ int cstr_duplicate(char **dest, const char *source)
 }
 
 
-bool cstr_validate_id(const char *element_id)
+bool cstr_isvalid(const char *element_id)
 // Determines if invalid characters are present in an element id string
 {
     const char *invalid_chars = " \";";
 
+    // if invalid char is present a pointer to it is returned else NULL
     if (strpbrk(element_id, invalid_chars))
         return false;
     else

src/util/cstr_helper.h

@@ -25,7 +25,7 @@ extern "C" {
 
 int cstr_duplicate(char **dest, const char *source);
 
-bool cstr_validate_id(const char *element_id);
+bool cstr_isvalid(const char *element_id);
 
 bool cstr_isnullterm(const char *source);
 

tests/test_curve.cpp

@@ -66,4 +66,21 @@ BOOST_FIXTURE_TEST_CASE(test_curve_comments, FixtureOpenClose)
     }
 }
 
+
+BOOST_FIXTURE_TEST_CASE(test_curve_id_isvalid, FixtureInitClose)
+{
+    error = EN_addcurve(ph, "C1");
+    BOOST_REQUIRE(error == 0);
+
+    error = EN_addcurve(ph, "C 2");
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addcurve(ph, "C\"2");
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addcurve(ph, "C;2");
+    BOOST_REQUIRE(error == 250);
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

tests/test_link.cpp

@@ -55,6 +55,26 @@ BOOST_FIXTURE_TEST_CASE(test_adddelete_link, FixtureInitClose)
 
 }
 
+BOOST_FIXTURE_TEST_CASE(test_link_isvalid, FixtureInitClose)
+{
+    // Build a network
+    EN_addnode(ph, (char *)"N1", EN_JUNCTION);
+    EN_addnode(ph, (char *)"N2", EN_JUNCTION);
+    EN_addnode(ph, (char *)"N3", EN_RESERVOIR);
+
+    error = EN_addlink(ph, (char *)"L1", EN_PUMP, (char *)"N1", (char *)"N2");
+    BOOST_REQUIRE(error == 0);
+
+    error = EN_addlink(ph, (char *)"L 2", EN_PIPE, (char *)"N1", (char *)"N2");
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addlink(ph, (char *)"L\"2", EN_PIPE, (char *)"N1", (char *)"N2");
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addlink(ph, (char *)"L;2", EN_PIPE, (char *)"N1", (char *)"N2");
+    BOOST_REQUIRE(error == 250);
+}
+
 BOOST_AUTO_TEST_CASE(test_setlinktype)
 {
     int error = 0;

tests/test_node.cpp

@@ -47,6 +47,22 @@ BOOST_FIXTURE_TEST_CASE(test_adddelete_node, FixtureInitClose)
 
 }
 
+BOOST_FIXTURE_TEST_CASE(test_node_validate_id, FixtureInitClose)
+{
+    error = EN_addnode(ph, (char *)"N2", EN_JUNCTION);
+    BOOST_REQUIRE(error == 0);
+
+    error = EN_addnode(ph, (char *)"N 2", EN_JUNCTION);
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addnode(ph, (char *)"N\"2", EN_JUNCTION);
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addnode(ph, (char *)"N;2", EN_JUNCTION);
+    BOOST_REQUIRE(error == 250);
+}
+
+
 BOOST_FIXTURE_TEST_CASE(test_junc_props, FixtureOpenClose)
 {
     int index;

tests/test_pattern.cpp

@@ -147,4 +147,20 @@ BOOST_FIXTURE_TEST_CASE(test_pattern_comments, FixtureOpenClose)
     BOOST_CHECK(check_string(comment, (char *)"Time Pattern 1"));
 }
 
+BOOST_FIXTURE_TEST_CASE(test_pat_isvalid_id, FixtureInitClose)
+{
+    error = EN_addpattern(ph, "P1");
+    BOOST_REQUIRE(error == 0);
+
+    error = EN_addpattern(ph, "P 2");
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addpattern(ph, "P\"2");
+    BOOST_REQUIRE(error == 250);
+
+    error = EN_addpattern(ph, "P;2");
+    BOOST_REQUIRE(error == 250);
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

tests/util/test_cstrhelper.cpp

@@ -44,13 +44,13 @@ BOOST_AUTO_TEST_CASE(test_duplicate) {
 }
 
 
-BOOST_AUTO_TEST_CASE(test_validate_id) {
+BOOST_AUTO_TEST_CASE(test_isvalid) {
 
-    BOOST_CHECK(cstr_validate_id("big tank") == false);
+    BOOST_CHECK(cstr_isvalid("big tank") == false);
-    BOOST_CHECK(cstr_validate_id("big\"tank") == false);
+    BOOST_CHECK(cstr_isvalid("big\"tank") == false);
-    BOOST_CHECK(cstr_validate_id("big;tank") == false);
+    BOOST_CHECK(cstr_isvalid("big;tank") == false);
 
-    BOOST_CHECK(cstr_validate_id("big-tank") == true);
+    BOOST_CHECK(cstr_isvalid("big-tank") == true);
 }