From ac56971ef2fd929b59d3a393bd13c1327a15e66b Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 14:44:06 -0400 Subject: [PATCH 1/9] Work in progress Creating function for validateing element ID strings --- src/util/cstr_helper.c | 17 +++++++++++++++-- src/util/cstr_helper.h | 17 +++++++++++++++-- src/util/filemanager.c | 10 +++++----- tests/util/CMakeLists.txt | 6 ++++++ tests/util/test_cstrhelper.cpp | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 tests/util/test_cstrhelper.cpp diff --git a/src/util/cstr_helper.c b/src/util/cstr_helper.c index 7d005a0..ec51594 100644 --- a/src/util/cstr_helper.c +++ b/src/util/cstr_helper.c @@ -17,7 +17,7 @@ #include "cstr_helper.h" -int copy_cstr(const char *source, char **dest) +int cstr_copy(const char *source, char **dest) // Determines length, allocates memory, and returns a null terminated copy // Be Aware: caller is responsible for freeing memory { @@ -39,7 +39,20 @@ int copy_cstr(const char *source, char **dest) } -bool isnullterm_cstr(const char *source) +bool cstr_validate_id(const char *element_id) +// Determines if invalid characters are present in an element id string +{ + const char *invalid_chars = " \";"; + + 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; diff --git a/src/util/cstr_helper.h b/src/util/cstr_helper.h index c344042..6817f5d 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_copy(const char *source, char **destination); + +bool cstr_validate_id(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..13c59c0 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_copy(file_handle->filename, 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_copy(filename, &(file_handle->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_copy(name, tempname); // --- 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_copy("enXXXXXX", tempname); error = mkstemp(*tempname); #endif return error; 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..1209e99 --- /dev/null +++ b/tests/util/test_cstrhelper.cpp @@ -0,0 +1,33 @@ +/* + ****************************************************************************** + 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 + ****************************************************************************** +*/ + +#define BOOST_TEST_MODULE cstr_helper +#include + +#include "util/cstr_helper.h" + + +BOOST_AUTO_TEST_SUITE(test_cstrhelper) + + +BOOST_AUTO_TEST_CASE(test_validate_id){ + + BOOST_CHECK(cstr_validate_id("big tank") == false); + BOOST_CHECK(cstr_validate_id("big\"tank") == false); + BOOST_CHECK(cstr_validate_id("big;tank") == false); + + BOOST_CHECK(cstr_validate_id("big-tank") == true); +} + + +BOOST_AUTO_TEST_SUITE_END() From 02ec735c58461a85b79aef6b0f34a37ff5f2d62c Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 15:51:19 -0400 Subject: [PATCH 2/9] Work in progress Refactoring cstr_copy and adding test --- src/util/cstr_helper.c | 9 +++------ src/util/cstr_helper.h | 2 +- src/util/filemanager.c | 8 ++++---- tests/util/test_cstrhelper.cpp | 24 +++++++++++++++++++++++- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/util/cstr_helper.c b/src/util/cstr_helper.c index ec51594..fd91ff3 100644 --- a/src/util/cstr_helper.c +++ b/src/util/cstr_helper.c @@ -17,13 +17,10 @@ #include "cstr_helper.h" -int cstr_copy(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) diff --git a/src/util/cstr_helper.h b/src/util/cstr_helper.h index 6817f5d..ca3ce6d 100644 --- a/src/util/cstr_helper.h +++ b/src/util/cstr_helper.h @@ -23,7 +23,7 @@ extern "C" { #endif -int cstr_copy(const char *source, char **destination); +int cstr_duplicate(char **dest, const char *source); bool cstr_validate_id(const char *element_id); diff --git a/src/util/filemanager.c b/src/util/filemanager.c index 13c59c0..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 cstr_copy(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 - cstr_copy(filename, &(file_handle->filename)); + cstr_duplicate(&(file_handle->filename), filename); if (file_mode == NULL) error = -1; @@ -195,7 +195,7 @@ int _get_temp_filename(char **tempname) return error; } else - cstr_copy(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 - cstr_copy("enXXXXXX", tempname); + cstr_duplicate(tempname, "enXXXXXX"); error = mkstemp(*tempname); #endif return error; diff --git a/tests/util/test_cstrhelper.cpp b/tests/util/test_cstrhelper.cpp index 1209e99..6011573 100644 --- a/tests/util/test_cstrhelper.cpp +++ b/tests/util/test_cstrhelper.cpp @@ -17,10 +17,32 @@ #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_validate_id){ +BOOST_AUTO_TEST_CASE(test_duplicate) { + char source[] = "I will be rewarded for good behavior."; + char *dest = NULL; + + cstr_duplicate(&dest, source); + BOOST_CHECK(check_string(dest, source)); + BOOST_CHECK(cstr_isnullterm(dest) == true); + + free(dest); + free(source); +} + + +BOOST_AUTO_TEST_CASE(test_validate_id) { BOOST_CHECK(cstr_validate_id("big tank") == false); BOOST_CHECK(cstr_validate_id("big\"tank") == false); From 2d74851635f06ac153c3ae609f31ae7754669cdb Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 15:54:12 -0400 Subject: [PATCH 3/9] Update cstr_helper.c fixing indentation --- src/util/cstr_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/cstr_helper.c b/src/util/cstr_helper.c index fd91ff3..d8706d3 100644 --- a/src/util/cstr_helper.c +++ b/src/util/cstr_helper.c @@ -27,9 +27,9 @@ int cstr_duplicate(char **dest, const char *source) 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; From 9224ac4f09a51502f8cbb4fb68758fb354fbf1eb Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 15:54:50 -0400 Subject: [PATCH 4/9] Update cstr_helper.c Fixing indentation --- src/util/cstr_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/cstr_helper.c b/src/util/cstr_helper.c index d8706d3..34d4279 100644 --- a/src/util/cstr_helper.c +++ b/src/util/cstr_helper.c @@ -51,8 +51,8 @@ bool cstr_validate_id(const char *element_id) 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; + if (strchr(source, '\0')) + return true; + else + return false; } From ea8e0439e3f17b07bab12d80431f0b73c836c455 Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 16:06:44 -0400 Subject: [PATCH 5/9] Update test_cstrhelper.cpp Fixed mem leak --- tests/util/test_cstrhelper.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/util/test_cstrhelper.cpp b/tests/util/test_cstrhelper.cpp index 6011573..2ee0ecd 100644 --- a/tests/util/test_cstrhelper.cpp +++ b/tests/util/test_cstrhelper.cpp @@ -11,6 +11,8 @@ ****************************************************************************** */ +#include + #define BOOST_TEST_MODULE cstr_helper #include @@ -30,15 +32,15 @@ BOOST_AUTO_TEST_SUITE(test_cstrhelper) BOOST_AUTO_TEST_CASE(test_duplicate) { - char source[] = "I will be rewarded for good behavior."; + + std::string source = "I will be rewarded for good behavior."; char *dest = NULL; - cstr_duplicate(&dest, source); + cstr_duplicate(&dest, source.c_str()); BOOST_CHECK(check_string(dest, source)); BOOST_CHECK(cstr_isnullterm(dest) == true); free(dest); - free(source); } From 22a7993c8c79e13947bf3f25d9ca54c0b0b8063b Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 16:57:38 -0400 Subject: [PATCH 6/9] Adding element id validity checks --- CMakeLists.txt | 4 ++-- src/epanet.c | 14 ++++++++++++++ src/util/cstr_helper.c | 3 ++- src/util/cstr_helper.h | 2 +- tests/test_curve.cpp | 17 +++++++++++++++++ tests/test_link.cpp | 20 ++++++++++++++++++++ tests/test_node.cpp | 16 ++++++++++++++++ tests/test_pattern.cpp | 16 ++++++++++++++++ tests/util/test_cstrhelper.cpp | 10 +++++----- 9 files changed, 93 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 93fbda1..0eaf8fd 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) -file(GLOB EPANET_LIB_ALL RELATIVE ${PROJECT_SOURCE_DIR} src/*) +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/* 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}) diff --git a/src/epanet.c b/src/epanet.c index 6b27c16..49ee7b7 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 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; diff --git a/src/util/cstr_helper.c b/src/util/cstr_helper.c index 34d4279..e02239b 100644 --- a/src/util/cstr_helper.c +++ b/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 diff --git a/src/util/cstr_helper.h b/src/util/cstr_helper.h index ca3ce6d..05399f0 100644 --- a/src/util/cstr_helper.h +++ b/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); diff --git a/tests/test_curve.cpp b/tests/test_curve.cpp index 6eb5a63..db59ad5 100644 --- a/tests/test_curve.cpp +++ b/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() diff --git a/tests/test_link.cpp b/tests/test_link.cpp index 3f91167..51c1d1f 100644 --- a/tests/test_link.cpp +++ b/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; diff --git a/tests/test_node.cpp b/tests/test_node.cpp index d0642ea..d35062f 100644 --- a/tests/test_node.cpp +++ b/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; diff --git a/tests/test_pattern.cpp b/tests/test_pattern.cpp index bedad48..c2b3cc7 100644 --- a/tests/test_pattern.cpp +++ b/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() diff --git a/tests/util/test_cstrhelper.cpp b/tests/util/test_cstrhelper.cpp index 2ee0ecd..2688a74 100644 --- a/tests/util/test_cstrhelper.cpp +++ b/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_validate_id("big\"tank") == false); - BOOST_CHECK(cstr_validate_id("big;tank") == false); + BOOST_CHECK(cstr_isvalid("big tank") == false); + BOOST_CHECK(cstr_isvalid("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); } From 3186ec326ca3f982234e31ccec934a607e926186 Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 17:29:32 -0400 Subject: [PATCH 7/9] Adding element id validity check Adding checks for element set id functions --- src/epanet.c | 13 +++++++++++-- tests/test_curve.cpp | 6 ++++++ tests/test_link.cpp | 8 +++++++- tests/test_node.cpp | 12 +++++++++--- tests/test_pattern.cpp | 9 ++++++++- 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/epanet.c b/src/epanet.c index 49ee7b7..d1bb49c 100644 --- a/src/epanet.c +++ b/src/epanet.c @@ -2021,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 250; // Check if another node with same name exists if (hashtable_find(net->NodeHashTable, newid) > 0) return 215; @@ -3208,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 250; // Check if another link with same name exists if (hashtable_find(net->LinkHashTable, newid) > 0) return 215; @@ -4085,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 250; + 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; @@ -4352,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 250; + if (strlen(id) > MAXID) return 250; for (i = 1; i <= p->network.Ncurves; i++) { diff --git a/tests/test_curve.cpp b/tests/test_curve.cpp index db59ad5..7472c4f 100644 --- a/tests/test_curve.cpp +++ b/tests/test_curve.cpp @@ -69,6 +69,8 @@ BOOST_FIXTURE_TEST_CASE(test_curve_comments, FixtureOpenClose) BOOST_FIXTURE_TEST_CASE(test_curve_id_isvalid, FixtureInitClose) { + int index; + error = EN_addcurve(ph, "C1"); BOOST_REQUIRE(error == 0); @@ -80,6 +82,10 @@ BOOST_FIXTURE_TEST_CASE(test_curve_id_isvalid, FixtureInitClose) error = EN_addcurve(ph, "C;2"); BOOST_REQUIRE(error == 250); + + EN_getcurveindex(ph, "C1", &index); + error = EN_setcurveid(ph, index, "C;2"); + BOOST_REQUIRE(error == 250); } diff --git a/tests/test_link.cpp b/tests/test_link.cpp index 51c1d1f..aef009e 100644 --- a/tests/test_link.cpp +++ b/tests/test_link.cpp @@ -55,8 +55,10 @@ BOOST_FIXTURE_TEST_CASE(test_adddelete_link, FixtureInitClose) } -BOOST_FIXTURE_TEST_CASE(test_link_isvalid, 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); @@ -73,6 +75,10 @@ BOOST_FIXTURE_TEST_CASE(test_link_isvalid, FixtureInitClose) error = EN_addlink(ph, (char *)"L;2", EN_PIPE, (char *)"N1", (char *)"N2"); BOOST_REQUIRE(error == 250); + + EN_getlinkindex(ph, "L1", &index); + error = EN_setlinkid(ph, index, "L;1"); + BOOST_REQUIRE(error == 250); } BOOST_AUTO_TEST_CASE(test_setlinktype) diff --git a/tests/test_node.cpp b/tests/test_node.cpp index d35062f..ebe22a9 100644 --- a/tests/test_node.cpp +++ b/tests/test_node.cpp @@ -49,17 +49,23 @@ 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 2", EN_JUNCTION); + error = EN_addnode(ph, (char *)"N 3", EN_JUNCTION); BOOST_REQUIRE(error == 250); - error = EN_addnode(ph, (char *)"N\"2", EN_JUNCTION); + error = EN_addnode(ph, (char *)"N\"3", EN_JUNCTION); BOOST_REQUIRE(error == 250); - error = EN_addnode(ph, (char *)"N;2", EN_JUNCTION); + error = EN_addnode(ph, (char *)"N;3", EN_JUNCTION); BOOST_REQUIRE(error == 250); + + EN_getnodeindex(ph, "N2", &index); + error = EN_setnodeid(ph, index, "N;2"); + BOOST_REQUIRE(error = 250); } diff --git a/tests/test_pattern.cpp b/tests/test_pattern.cpp index c2b3cc7..129d213 100644 --- a/tests/test_pattern.cpp +++ b/tests/test_pattern.cpp @@ -147,8 +147,10 @@ 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) +BOOST_FIXTURE_TEST_CASE(test_pat_id_isvalid, FixtureInitClose) { + int index; + error = EN_addpattern(ph, "P1"); BOOST_REQUIRE(error == 0); @@ -160,6 +162,11 @@ BOOST_FIXTURE_TEST_CASE(test_pat_isvalid_id, FixtureInitClose) error = EN_addpattern(ph, "P;2"); BOOST_REQUIRE(error == 250); + + EN_getpatternindex(ph, "P1", &index); + error = EN_setpatternid(ph, index, "P;1"); + BOOST_REQUIRE(error == 250); + } From f161ecaa72156564058624e573af547fd26ba5b2 Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Tue, 16 Apr 2019 17:40:40 -0400 Subject: [PATCH 8/9] Fixing build warnings on gcc --- tests/test_curve.cpp | 12 ++++++------ tests/test_link.cpp | 4 ++-- tests/test_node.cpp | 4 ++-- tests/test_pattern.cpp | 12 ++++++------ 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/test_curve.cpp b/tests/test_curve.cpp index 7472c4f..8cb9705 100644 --- a/tests/test_curve.cpp +++ b/tests/test_curve.cpp @@ -71,20 +71,20 @@ BOOST_FIXTURE_TEST_CASE(test_curve_id_isvalid, FixtureInitClose) { int index; - error = EN_addcurve(ph, "C1"); + error = EN_addcurve(ph, (char *)"C1"); BOOST_REQUIRE(error == 0); - error = EN_addcurve(ph, "C 2"); + error = EN_addcurve(ph, (char *)"C 2"); BOOST_REQUIRE(error == 250); - error = EN_addcurve(ph, "C\"2"); + error = EN_addcurve(ph, (char *)"C\"2"); BOOST_REQUIRE(error == 250); - error = EN_addcurve(ph, "C;2"); + error = EN_addcurve(ph, (char *)"C;2"); BOOST_REQUIRE(error == 250); - EN_getcurveindex(ph, "C1", &index); - error = EN_setcurveid(ph, index, "C;2"); + EN_getcurveindex(ph, (char *)"C1", &index); + error = EN_setcurveid(ph, index, (char *)"C;2"); BOOST_REQUIRE(error == 250); } diff --git a/tests/test_link.cpp b/tests/test_link.cpp index aef009e..0863d96 100644 --- a/tests/test_link.cpp +++ b/tests/test_link.cpp @@ -76,8 +76,8 @@ BOOST_FIXTURE_TEST_CASE(test_link_id_isvalid, FixtureInitClose) error = EN_addlink(ph, (char *)"L;2", EN_PIPE, (char *)"N1", (char *)"N2"); BOOST_REQUIRE(error == 250); - EN_getlinkindex(ph, "L1", &index); - error = EN_setlinkid(ph, index, "L;1"); + EN_getlinkindex(ph, (char *)"L1", &index); + error = EN_setlinkid(ph, index, (char *)"L;1"); BOOST_REQUIRE(error == 250); } diff --git a/tests/test_node.cpp b/tests/test_node.cpp index ebe22a9..907ae4e 100644 --- a/tests/test_node.cpp +++ b/tests/test_node.cpp @@ -63,8 +63,8 @@ BOOST_FIXTURE_TEST_CASE(test_node_validate_id, FixtureInitClose) error = EN_addnode(ph, (char *)"N;3", EN_JUNCTION); BOOST_REQUIRE(error == 250); - EN_getnodeindex(ph, "N2", &index); - error = EN_setnodeid(ph, index, "N;2"); + EN_getnodeindex(ph, (char *)"N2", &index); + error = EN_setnodeid(ph, index, (char *)"N;2"); BOOST_REQUIRE(error = 250); } diff --git a/tests/test_pattern.cpp b/tests/test_pattern.cpp index 129d213..e370134 100644 --- a/tests/test_pattern.cpp +++ b/tests/test_pattern.cpp @@ -151,20 +151,20 @@ BOOST_FIXTURE_TEST_CASE(test_pat_id_isvalid, FixtureInitClose) { int index; - error = EN_addpattern(ph, "P1"); + error = EN_addpattern(ph, (char *)"P1"); BOOST_REQUIRE(error == 0); - error = EN_addpattern(ph, "P 2"); + error = EN_addpattern(ph, (char *)"P 2"); BOOST_REQUIRE(error == 250); - error = EN_addpattern(ph, "P\"2"); + error = EN_addpattern(ph, (char *)"P\"2"); BOOST_REQUIRE(error == 250); - error = EN_addpattern(ph, "P;2"); + error = EN_addpattern(ph, (char *)"P;2"); BOOST_REQUIRE(error == 250); - EN_getpatternindex(ph, "P1", &index); - error = EN_setpatternid(ph, index, "P;1"); + EN_getpatternindex(ph, (char *)"P1", &index); + error = EN_setpatternid(ph, index, (char *)"P;1"); BOOST_REQUIRE(error == 250); } From d1979e7ed09384131dc5784d632ac32d418a1a93 Mon Sep 17 00:00:00 2001 From: Michael Tryby Date: Wed, 17 Apr 2019 09:39:24 -0400 Subject: [PATCH 9/9] Update errror code from 250 to 252 --- .gitignore | 2 ++ src/epanet.c | 18 +++++++++--------- src/errors.dat | 1 + tests/test_curve.cpp | 8 ++++---- tests/test_link.cpp | 8 ++++---- tests/test_node.cpp | 8 ++++---- tests/test_pattern.cpp | 8 ++++---- 7 files changed, 28 insertions(+), 25 deletions(-) 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/src/epanet.c b/src/epanet.c index d1bb49c..6a9c546 100644 --- a/src/epanet.c +++ b/src/epanet.c @@ -1734,7 +1734,7 @@ int DLLEXPORT EN_addnode(EN_Project p, char *id, int nodeType) if (hyd->OpenHflag || qual->OpenQflag) return 262; // Check if id contains invalid characters - if (!cstr_isvalid(id)) return 250; + if (!cstr_isvalid(id)) return 252; // Check if a node with same id already exists if (EN_getnodeindex(p, id, &i) == 0) return 215; @@ -2021,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 (!cstr_isvalid(newid)) return 250; + if (!cstr_isvalid(newid)) return 252; // Check if another node with same name exists if (hashtable_find(net->NodeHashTable, newid) > 0) return 215; @@ -2945,7 +2945,7 @@ int DLLEXPORT EN_addlink(EN_Project p, char *id, int linkType, if (p->hydraul.OpenHflag || p->quality.OpenQflag) return 262; // Check if id contains invalid characters - if (!cstr_isvalid(id)) return 250; + if (!cstr_isvalid(id)) return 252; // Check if a link with same id already exists if (EN_getlinkindex(p, id, &i) == 0) return 215; @@ -3208,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 (!cstr_isvalid(newid)) return 250; + if (!cstr_isvalid(newid)) return 252; // Check if another link with same name exists if (hashtable_find(net->LinkHashTable, newid) > 0) return 215; @@ -3960,7 +3960,7 @@ int DLLEXPORT EN_addpattern(EN_Project p, char *id) if (EN_getpatternindex(p, id, &i) == 0) return 215; // Check is id name contains invalid characters - if (!cstr_isvalid(id)) return 250; + if (!cstr_isvalid(id)) return 252; // Check that id name is not too long if (strlen(id) > MAXID) return 250; @@ -4087,7 +4087,7 @@ int DLLEXPORT EN_setpatternid(EN_Project p, int index, char *id) if (index < 1 || index > p->network.Npats) return 205; // Check is id name contains invalid characters - if (!cstr_isvalid(id)) return 250; + if (!cstr_isvalid(id)) return 252; if (strlen(id) > MAXID) return 250; @@ -4236,7 +4236,7 @@ int DLLEXPORT EN_addcurve(EN_Project p, char *id) if (EN_getcurveindex(p, id, &i) == 0) return 215; // Check is id name contains invalid characters - if (!cstr_isvalid(id)) return 250; + if (!cstr_isvalid(id)) return 252; // Check that id name is not too long if (strlen(id) > MAXID) return 250; @@ -4359,8 +4359,8 @@ int DLLEXPORT EN_setcurveid(EN_Project p, int index, char *id) if (index < 1 || index > p->network.Ncurves) return 205; // Check is id name contains invalid characters - if (!cstr_isvalid(id)) return 250; - + 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/tests/test_curve.cpp b/tests/test_curve.cpp index 8cb9705..6e80cd5 100644 --- a/tests/test_curve.cpp +++ b/tests/test_curve.cpp @@ -75,17 +75,17 @@ BOOST_FIXTURE_TEST_CASE(test_curve_id_isvalid, FixtureInitClose) BOOST_REQUIRE(error == 0); error = EN_addcurve(ph, (char *)"C 2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addcurve(ph, (char *)"C\"2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addcurve(ph, (char *)"C;2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); EN_getcurveindex(ph, (char *)"C1", &index); error = EN_setcurveid(ph, index, (char *)"C;2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); } diff --git a/tests/test_link.cpp b/tests/test_link.cpp index 0863d96..2a24512 100644 --- a/tests/test_link.cpp +++ b/tests/test_link.cpp @@ -68,17 +68,17 @@ BOOST_FIXTURE_TEST_CASE(test_link_id_isvalid, FixtureInitClose) BOOST_REQUIRE(error == 0); error = EN_addlink(ph, (char *)"L 2", EN_PIPE, (char *)"N1", (char *)"N2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addlink(ph, (char *)"L\"2", EN_PIPE, (char *)"N1", (char *)"N2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addlink(ph, (char *)"L;2", EN_PIPE, (char *)"N1", (char *)"N2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); EN_getlinkindex(ph, (char *)"L1", &index); error = EN_setlinkid(ph, index, (char *)"L;1"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); } BOOST_AUTO_TEST_CASE(test_setlinktype) diff --git a/tests/test_node.cpp b/tests/test_node.cpp index 907ae4e..e7139ae 100644 --- a/tests/test_node.cpp +++ b/tests/test_node.cpp @@ -55,17 +55,17 @@ BOOST_FIXTURE_TEST_CASE(test_node_validate_id, FixtureInitClose) BOOST_REQUIRE(error == 0); error = EN_addnode(ph, (char *)"N 3", EN_JUNCTION); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addnode(ph, (char *)"N\"3", EN_JUNCTION); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addnode(ph, (char *)"N;3", EN_JUNCTION); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); EN_getnodeindex(ph, (char *)"N2", &index); error = EN_setnodeid(ph, index, (char *)"N;2"); - BOOST_REQUIRE(error = 250); + BOOST_REQUIRE(error = 252); } diff --git a/tests/test_pattern.cpp b/tests/test_pattern.cpp index e370134..ef2eb71 100644 --- a/tests/test_pattern.cpp +++ b/tests/test_pattern.cpp @@ -155,17 +155,17 @@ BOOST_FIXTURE_TEST_CASE(test_pat_id_isvalid, FixtureInitClose) BOOST_REQUIRE(error == 0); error = EN_addpattern(ph, (char *)"P 2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addpattern(ph, (char *)"P\"2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); error = EN_addpattern(ph, (char *)"P;2"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); EN_getpatternindex(ph, (char *)"P1", &index); error = EN_setpatternid(ph, index, (char *)"P;1"); - BOOST_REQUIRE(error == 250); + BOOST_REQUIRE(error == 252); }