Merge branch 'dev' of https://github.com/OpenWaterAnalytics/EPANET into dev-list

.gitignore

@@ -223,6 +223,8 @@ temp/
 # Testing Stuff
 nrtestsuite/
 tests/data/
+tests/outfile/data/en*
+
 
 #Cmake stuff
 __cmake_systeminformation/

CMakeLists.txt

@@ -83,8 +83,8 @@ ENDIF (MSVC)
 
 
 # configure file groups
-file(GLOB EPANET_SOURCES RELATIVE ${PROJECT_SOURCE_DIR} src/*.c src/util/list.c)
+file(GLOB EPANET_SOURCES RELATIVE ${PROJECT_SOURCE_DIR} src/*.c src/util/*.c)
-file(GLOB EPANET_LIB_ALL RELATIVE ${PROJECT_SOURCE_DIR} src/* src/util/list.*)
+file(GLOB EPANET_LIB_ALL RELATIVE ${PROJECT_SOURCE_DIR} src/* src/util/*)
 # 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 252;
+
     // Check if a node with same id already exists
     if (EN_getnodeindex(p, id, &i) == 0) return 215;
 
@@ -2016,7 +2021,7 @@ int DLLEXPORT EN_setnodeid(EN_Project p, int index, char *newid)
     if (index <= 0 || index > net->Nnodes) return 203;
     n = strlen(newid);
     if (n < 1 || n > MAXID) return 209;
-    if (strcspn(newid, " ;") < n) return 209;
+    if (!cstr_isvalid(newid)) return 252;
 
     // Check if another node with same name exists
     if (hashtable_find(net->NodeHashTable, newid) > 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 252;
+
     // Check if a link with same id already exists
     if (EN_getlinkindex(p, id, &i) == 0) return 215;
 
@@ -3200,7 +3208,7 @@ int DLLEXPORT EN_setlinkid(EN_Project p, int index, char *newid)
     if (index <= 0 || index > net->Nlinks) return 204;
     n = strlen(newid);
     if (n < 1 || n > MAXID) return 211;
-    if (strcspn(newid, " ;") < n) return 211;
+    if (!cstr_isvalid(newid)) return 252;
 
     // Check if another link with same name exists
     if (hashtable_find(net->LinkHashTable, newid) > 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 252;
+
     // Check that id name is not too long
     if (strlen(id) > MAXID) return 250;
 
@@ -4074,7 +4085,12 @@ int DLLEXPORT EN_setpatternid(EN_Project p, int index, char *id)
 
     if (!p->Openflag) return 102;
     if (index < 1 || index > p->network.Npats) return 205;
+
+    // Check is id name contains invalid characters
+    if (!cstr_isvalid(id)) return 252;
+
     if (strlen(id) > MAXID) return 250;
+
     for (i = 1; i <= p->network.Npats; i++)
     {
         if (i != index && strcmp(id, p->network.Pattern[i].ID) == 0) return 215;
@@ -4219,6 +4235,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 252;
+
     // Check that id name is not too long
     if (strlen(id) > MAXID) return 250;
 
@@ -4338,6 +4357,10 @@ int DLLEXPORT EN_setcurveid(EN_Project p, int index, char *id)
 
     if (!p->Openflag) return 102;
     if (index < 1 || index > p->network.Ncurves) return 205;
+
+    // Check is id name contains invalid characters
+    if (!cstr_isvalid(id)) return 252;
+
     if (strlen(id) > MAXID) return 250;
     for (i = 1; i <= p->network.Ncurves; i++)
     {

src/errors.dat

@@ -49,6 +49,7 @@ DAT(240,"nonexistent source")
 DAT(241,"nonexistent control")
 DAT(250,"invalid format")
 DAT(251,"invalid parameter code")
+DAT(252,"invalid ID name")
 DAT(253,"nonexistent demand category")
 DAT(254,"node with no coordinates")
 DAT(257,"nonexistent rule")

src/util/cstr_helper.c

@@ -17,32 +17,43 @@
 #include "cstr_helper.h"
 
 
-int copy_cstr(const char *source, char **dest)
+int cstr_duplicate(char **dest, const char *source)
-// Determines length, allocates memory, and returns a null terminated copy
+// Duplicates source string
-// Be Aware: caller is responsible for freeing memory
 {
-    size_t size;
+    size_t size = 1 + strlen(source);
-
-    size = 1 + strlen(source);
     *dest = (char *) calloc(size, sizeof(char));
 
     if (*dest == NULL)
         return -1;
     else {
 #ifdef _MSC_VER
-		strncpy_s(*dest, size, source, size);
+        strncpy_s(*dest, size, source, size);
 #else
-		strncpy(*dest, source, size);
+        strncpy(*dest, source, size);
 #endif
     }
     return 0;
 }
 
 
-bool isnullterm_cstr(const char *source)
+bool cstr_isvalid(const char *element_id)
+// Determines if invalid characters are present in an element id string
 {
-	if (strchr(source, '\0'))
+    const char *invalid_chars = " \";";
-		return true;
+
-	else
+    // if invalid char is present a pointer to it is returned else NULL
-		return false;
+    if (strpbrk(element_id, invalid_chars))
+        return false;
+    else
+        return true;
+}
+
+
+bool cstr_isnullterm(const char *source)
+// Determines if the string passed is null terminated or not
+{
+    if (strchr(source, '\0'))
+        return true;
+    else
+        return false;
 }

src/util/cstr_helper.h

@@ -18,8 +18,21 @@
 #include <stdbool.h>
 
 
-int copy_cstr(const char *source, char **destination);
+#if defined(__cplusplus)
-bool isnullterm_cstr(const char *source);
+extern "C" {
+#endif
+
+
+int cstr_duplicate(char **dest, const char *source);
+
+bool cstr_isvalid(const char *element_id);
+
+bool cstr_isnullterm(const char *source);
+
+
+#if defined(__cplusplus)
+}
+#endif
 
 
 #endif /* CSTR_HELPER_H_ */

src/util/filemanager.c

@@ -63,7 +63,7 @@ int get_filename(file_handle_t *file_handle, char **filename)
 // BE AWARE: The memory allocated here must be freed by the caller
 //
 {
-    return copy_cstr(file_handle->filename, filename);
+    return cstr_duplicate(filename, file_handle->filename);
 }
 
 
@@ -73,7 +73,7 @@ int open_file(file_handle_t *file_handle, const char *filename, const char *file
     if (filename == NULL)
         _get_temp_filename(&(file_handle->filename));
     else
-        copy_cstr(filename, &(file_handle->filename));
+        cstr_duplicate(&(file_handle->filename), filename);
 
     if (file_mode == NULL)
         error = -1;
@@ -156,7 +156,7 @@ int remove_file(file_handle_t *file_handle) {
 bool is_valid(file_handle_t *file_handle)
 {
 	if ((file_handle->filename == NULL && file_handle->file == NULL) ||
-		(isnullterm_cstr(file_handle->filename) && file_handle != NULL))
+		(cstr_isnullterm(file_handle->filename) && file_handle != NULL))
 		return true;
 	else
 		return false;
@@ -195,7 +195,7 @@ int _get_temp_filename(char **tempname)
         return error;
     }
     else
-        copy_cstr(name, tempname);
+        cstr_duplicate(tempname, name);
 
     // --- free the pointer returned by _tempnam
     if (name)
@@ -204,7 +204,7 @@ int _get_temp_filename(char **tempname)
     // --- for non-Windows systems:
 #else
     // --- use system function mkstemp() to create a temporary file name
-    copy_cstr("enXXXXXX", tempname);
+    cstr_duplicate(tempname, "enXXXXXX");
     error = mkstemp(*tempname);
 #endif
     return error;

tests/test_curve.cpp

@@ -66,4 +66,27 @@ BOOST_FIXTURE_TEST_CASE(test_curve_comments, FixtureOpenClose)
     }
 }
 
+
+BOOST_FIXTURE_TEST_CASE(test_curve_id_isvalid, FixtureInitClose)
+{
+    int index;
+
+    error = EN_addcurve(ph, (char *)"C1");
+    BOOST_REQUIRE(error == 0);
+
+    error = EN_addcurve(ph, (char *)"C 2");
+    BOOST_REQUIRE(error == 252);
+
+    error = EN_addcurve(ph, (char *)"C\"2");
+    BOOST_REQUIRE(error == 252);
+
+    error = EN_addcurve(ph, (char *)"C;2");
+    BOOST_REQUIRE(error == 252);
+
+    EN_getcurveindex(ph, (char *)"C1", &index);
+    error = EN_setcurveid(ph, index, (char *)"C;2");
+    BOOST_REQUIRE(error == 252);
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

tests/test_link.cpp

@@ -55,6 +55,32 @@ BOOST_FIXTURE_TEST_CASE(test_adddelete_link, FixtureInitClose)
 
 }
 
+BOOST_FIXTURE_TEST_CASE(test_link_id_isvalid, FixtureInitClose)
+{
+    int index;
+
+    // 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 == 252);
+
+    error = EN_addlink(ph, (char *)"L\"2", EN_PIPE, (char *)"N1", (char *)"N2");
+    BOOST_REQUIRE(error == 252);
+
+    error = EN_addlink(ph, (char *)"L;2", EN_PIPE, (char *)"N1", (char *)"N2");
+    BOOST_REQUIRE(error == 252);
+
+    EN_getlinkindex(ph, (char *)"L1", &index);
+    error = EN_setlinkid(ph, index, (char *)"L;1");
+    BOOST_REQUIRE(error == 252);
+}
+
 BOOST_AUTO_TEST_CASE(test_setlinktype)
 {
     int error = 0;

tests/test_node.cpp

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

tests/test_pattern.cpp

@@ -147,4 +147,27 @@ BOOST_FIXTURE_TEST_CASE(test_pattern_comments, FixtureOpenClose)
     BOOST_CHECK(check_string(comment, (char *)"Time Pattern 1"));
 }
 
+BOOST_FIXTURE_TEST_CASE(test_pat_id_isvalid, FixtureInitClose)
+{
+    int index;
+
+    error = EN_addpattern(ph, (char *)"P1");
+    BOOST_REQUIRE(error == 0);
+
+    error = EN_addpattern(ph, (char *)"P 2");
+    BOOST_REQUIRE(error == 252);
+
+    error = EN_addpattern(ph, (char *)"P\"2");
+    BOOST_REQUIRE(error == 252);
+
+    error = EN_addpattern(ph, (char *)"P;2");
+    BOOST_REQUIRE(error == 252);
+
+    EN_getpatternindex(ph, (char *)"P1", &index);
+    error = EN_setpatternid(ph, index, (char *)"P;1");
+    BOOST_REQUIRE(error == 252);
+
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

tests/util/CMakeLists.txt

@@ -7,6 +7,12 @@ set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
 
 
+add_executable(test_cstrhelper ./test_cstrhelper.cpp
+                               ../../src/util/cstr_helper.c)
+target_include_directories(test_cstrhelper PUBLIC ../../src/)
+target_link_libraries(test_cstrhelper ${Boost_LIBRARIES})
+
+
 add_executable(test_errormanager ./test_errormanager.cpp
                                  ../../src/util/errormanager.c)
 target_include_directories(test_errormanager PUBLIC ../../src/)

tests/util/test_cstrhelper.cpp

@@ -0,0 +1,57 @@
+/*
+ ******************************************************************************
+ Project:      OWA EPANET
+ Version:      2.2
+ Module:       test_cstrhelper.cpp
+ Description:  tests for C string helper functions
+ Authors:      see AUTHORS
+ Copyright:    see AUTHORS
+ License:      see LICENSE
+ Last Updated: 04/16/2019
+ ******************************************************************************
+*/
+
+#include <string>
+
+#define BOOST_TEST_MODULE cstr_helper
+#include <boost/test/unit_test.hpp>
+
+#include "util/cstr_helper.h"
+
+
+boost::test_tools::predicate_result check_string(std::string test, std::string ref)
+{
+    if (ref.compare(test) == 0)
+        return true;
+    else
+        return false;
+}
+
+
+BOOST_AUTO_TEST_SUITE(test_cstrhelper)
+
+
+BOOST_AUTO_TEST_CASE(test_duplicate) {
+
+    std::string source = "I will be rewarded for good behavior.";
+    char *dest = NULL;
+
+    cstr_duplicate(&dest, source.c_str());
+    BOOST_CHECK(check_string(dest, source));
+    BOOST_CHECK(cstr_isnullterm(dest) == true);
+
+    free(dest);
+}
+
+
+BOOST_AUTO_TEST_CASE(test_isvalid) {
+
+    BOOST_CHECK(cstr_isvalid("big tank") == false);
+    BOOST_CHECK(cstr_isvalid("big\"tank") == false);
+    BOOST_CHECK(cstr_isvalid("big;tank") == false);
+
+    BOOST_CHECK(cstr_isvalid("big-tank") == true);
+}
+
+
+BOOST_AUTO_TEST_SUITE_END()