diff --git a/.gitignore b/.gitignore index 8566bb4..6e469d5 100755 --- a/.gitignore +++ b/.gitignore @@ -223,6 +223,8 @@ temp/ # Testing Stuff nrtestsuite/ tests/data/ +tests/outfile/data/en* + #Cmake stuff __cmake_systeminformation/ diff --git a/CMakeLists.txt b/CMakeLists.txt index 6892dcc..cc15f07 100755 --- a/CMakeLists.txt +++ b/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_LIB_ALL RELATIVE ${PROJECT_SOURCE_DIR} src/* src/util/list.*) +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/*) # 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}) diff --git a/src/epanet.c b/src/epanet.c index 6b27c16..6a9c546 100644 --- a/src/epanet.c +++ b/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++) { diff --git a/src/errors.dat b/src/errors.dat index 52922c7..52baec3 100644 --- a/src/errors.dat +++ b/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") diff --git a/src/util/cstr_helper.c b/src/util/cstr_helper.c index 7d005a0..e02239b 100644 --- a/src/util/cstr_helper.c +++ b/src/util/cstr_helper.c @@ -17,32 +17,43 @@ #include "cstr_helper.h" -int copy_cstr(const char *source, char **dest) -// Determines length, allocates memory, and returns a null terminated copy -// Be Aware: caller is responsible for freeing memory +int cstr_duplicate(char **dest, const char *source) +// Duplicates source string { - size_t size; - - size = 1 + strlen(source); + size_t 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')) - return true; - else - return false; + 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 + 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; } diff --git a/src/util/cstr_helper.h b/src/util/cstr_helper.h index c344042..05399f0 100644 --- a/src/util/cstr_helper.h +++ b/src/util/cstr_helper.h @@ -18,8 +18,21 @@ #include -int copy_cstr(const char *source, char **destination); -bool isnullterm_cstr(const char *source); +#if defined(__cplusplus) +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_ */ diff --git a/src/util/filemanager.c b/src/util/filemanager.c index eb9c4cd..79a45d2 100644 --- a/src/util/filemanager.c +++ b/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; diff --git a/tests/test_curve.cpp b/tests/test_curve.cpp index 6eb5a63..6e80cd5 100644 --- a/tests/test_curve.cpp +++ b/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() diff --git a/tests/test_link.cpp b/tests/test_link.cpp index 3f91167..2a24512 100644 --- a/tests/test_link.cpp +++ b/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; diff --git a/tests/test_node.cpp b/tests/test_node.cpp index d0642ea..e7139ae 100644 --- a/tests/test_node.cpp +++ b/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; diff --git a/tests/test_pattern.cpp b/tests/test_pattern.cpp index bedad48..ef2eb71 100644 --- a/tests/test_pattern.cpp +++ b/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() diff --git a/tests/util/CMakeLists.txt b/tests/util/CMakeLists.txt index 59463e9..c707e0b 100644 --- a/tests/util/CMakeLists.txt +++ b/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/) diff --git a/tests/util/test_cstrhelper.cpp b/tests/util/test_cstrhelper.cpp new file mode 100644 index 0000000..2688a74 --- /dev/null +++ b/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 + +#define BOOST_TEST_MODULE cstr_helper +#include + +#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()