From b6ebbc0b8b15741916b9a4b3e84521ddd8d1c517 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Wed, 29 Jun 2022 15:39:54 +0200 Subject: [PATCH] iox-#1036 Remove deprecated function, refactor isValidPathEntry to reduce code duplication, add further tests Signed-off-by: Christian Eltzschig --- .../include/iceoryx_hoofs/cxx/helplets.hpp | 13 ++- .../iceoryx_hoofs/internal/cxx/helplets.inl | 34 +----- .../test/moduletests/test_cxx_helplets.cpp | 107 ++++++++++++++---- .../include/iceoryx_posh/roudi/roudi_app.hpp | 2 +- 4 files changed, 97 insertions(+), 59 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp index d98fc58ea0..761b3766bb 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp @@ -249,12 +249,19 @@ constexpr bool isPowerOfTwo(const T n) noexcept return n && ((n & (n - 1U)) == 0U); } +enum class RelativePathComponents +{ + REJECT, + ACCEPT +}; + /// @brief checks if the given string is a valid path entry. A path entry is the string between /// two path separators. /// @param[in] name the path entry in question /// @return true if it is valid, otherwise false template -bool isValidPathEntry(const string& name) noexcept; +bool isValidPathEntry(const string& name, + const RelativePathComponents& relativePathComponents) noexcept; /// @brief checks if the given string is a valid filename /// @param[in] name the string to verify @@ -268,10 +275,6 @@ bool isValidFileName(const string& name) noexcept; template bool isValidPathToFile(const string& name) noexcept; -template -[[deprecated("removed in 4.0, use isValidPathToFile")]] bool -isValidFilePath(const string& name) noexcept; - /// @brief returns true if the provided name is a valid path, otherwise false /// @param[in] name the string to verify template diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/helplets.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/helplets.inl index b00a2a030c..bab8d5a52e 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/helplets.inl +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/helplets.inl @@ -23,21 +23,22 @@ namespace iox namespace cxx { template -inline bool isValidPathEntry(const string& name) noexcept +inline bool isValidPathEntry(const string& name, + const RelativePathComponents& relativePathComponents) noexcept { const string currentDirectory("."); const string parentDirectory(".."); if (name == currentDirectory || name == parentDirectory) { - return true; + return relativePathComponents == RelativePathComponents::ACCEPT; } const auto nameSize = name.size(); for (uint64_t i = 0; i < nameSize; ++i) { - const char c = name.c_str()[i]; + const char c = name[i]; if (!((internal::ASCII_A <= c && c <= internal::ASCII_Z) || (internal::ASCII_CAPITAL_A <= c && c <= internal::ASCII_CAPITAL_Z) || (internal::ASCII_0 <= c && c <= internal::ASCII_9) || c == internal::ASCII_MINUS @@ -64,25 +65,8 @@ inline bool isValidFileName(const string& name) noexcept return false; } - uint64_t nameSize = name.size(); - - const string currentDirectory("."); - const string parentDirectory(".."); - - if (name == currentDirectory || name == parentDirectory) - { - return false; - } - - // dot at the end is invalid to be compatible with windows api - const char lastCharacter = name.c_str()[nameSize - 1U]; - if (lastCharacter == '.') - { - return false; - } - // check if the file contains only valid characters - return isValidPathEntry(name); + return isValidPathEntry(name, RelativePathComponents::REJECT); } template @@ -100,12 +84,6 @@ inline bool isValidPathToFile(const string& name) noexcept return (pathPart.empty() || isValidPathToDirectory(pathPart)) && isValidFileName(filePart); } -template -inline bool isValidFilePath(const string& name) noexcept -{ - return isValidPathToFile(name); -} - template inline bool isValidPathToDirectory(const string& name) noexcept { @@ -151,7 +129,7 @@ inline bool isValidPathToDirectory(const string& name) noexcept // we reached the last entry, if its a valid file name the path is valid else if (!separatorPosition) { - return isValidPathEntry(temp); + return isValidPathEntry(temp, RelativePathComponents::ACCEPT); } } diff --git a/iceoryx_hoofs/test/moduletests/test_cxx_helplets.cpp b/iceoryx_hoofs/test/moduletests/test_cxx_helplets.cpp index 64c0291736..b941ab60c0 100644 --- a/iceoryx_hoofs/test/moduletests/test_cxx_helplets.cpp +++ b/iceoryx_hoofs/test/moduletests/test_cxx_helplets.cpp @@ -384,7 +384,7 @@ TEST(Helplets_test_isValidPathToFile, MultipleSlashsAreValidFilePath) EXPECT_TRUE(isValidPathToFile(string("//multi///slash////hypno"))); } -TEST(Helplets_test_isValidPathToFile, RelativePathElementsAreValid) +TEST(Helplets_test_isValidPathToFile, RelativePathComponentsAreValid) { ::testing::Test::RecordProperty("TEST_ID", "ec7d682f-ac7b-4173-a3f6-55969696ee92"); EXPECT_TRUE(isValidPathToFile(string("../some.file"))); @@ -460,7 +460,8 @@ TEST(Helplets_test_isValidPathToFile, EmptyFilePathIsInvalid) EXPECT_FALSE(isValidPathToFile(string(""))); } -TEST(Helplets_test_isValidPathToFile_and_isValidPath, WhenOneInvalidCharacterIsContainedPathIsInvalid) +TEST(Helplets_test_isValidPathToFile_isValidPathToDirectory_isValidPathEntry, + WhenOneInvalidCharacterIsContainedPathIsInvalid) { ::testing::Test::RecordProperty("TEST_ID", "a764cff3-2607-47bb-952b-4ca75f326721"); std::string validPath1 = "/hello"; @@ -518,9 +519,13 @@ TEST(Helplets_test_isValidPathToFile_and_isValidPath, WhenOneInvalidCharacterIsC EXPECT_FALSE(isValidPathToDirectory(invalidCharacterMiddleTest)); EXPECT_FALSE(isValidPathToDirectory(invalidCharacterEndTest)); - EXPECT_FALSE(isValidPathEntry(invalidCharacterFrontTest)); - EXPECT_FALSE(isValidPathEntry(invalidCharacterMiddleTest)); - EXPECT_FALSE(isValidPathEntry(invalidCharacterEndTest)); + EXPECT_FALSE(isValidPathEntry(invalidCharacterFrontTest, iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(invalidCharacterMiddleTest, iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(invalidCharacterEndTest, iox::cxx::RelativePathComponents::ACCEPT)); + + EXPECT_FALSE(isValidPathEntry(invalidCharacterFrontTest, iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(invalidCharacterMiddleTest, iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(invalidCharacterEndTest, iox::cxx::RelativePathComponents::REJECT)); } } @@ -539,7 +544,7 @@ TEST(Helplets_test_isValidPathToDirectory, MultipleSlashsAreValidPath) EXPECT_TRUE(isValidPathToDirectory(string("//multi///slash////hypno////"))); } -TEST(Helplets_test_isValidPathToDirectory, RelativePathElementsAreValid) +TEST(Helplets_test_isValidPathToDirectory, RelativePathComponentsAreValid) { ::testing::Test::RecordProperty("TEST_ID", "97c215ca-7f67-4ec1-9b17-d98b219a804d"); EXPECT_TRUE(isValidPathToDirectory(string("../some.file"))); @@ -684,52 +689,104 @@ TEST(Helplets_test_doesEndWithPathSeparator, MultiCharacterStringEndingWithPathS TEST(Helplets_test_isValidPathEntry, EmptyPathEntryIsValid) { - EXPECT_TRUE(isValidPathEntry(string(""))); + EXPECT_TRUE( + isValidPathEntry(string(""), iox::cxx::RelativePathComponents::ACCEPT)); } TEST(Helplets_test_isValidPathEntry, PathEntryWithOnlyValidCharactersIsValid) { - EXPECT_TRUE(isValidPathEntry(string("a"))); - EXPECT_TRUE(isValidPathEntry(string("agc"))); - EXPECT_TRUE(isValidPathEntry(string("a.213jkgc"))); + EXPECT_TRUE(isValidPathEntry(string("a"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_TRUE(isValidPathEntry(string("agc"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_TRUE(isValidPathEntry(string("a.213jkgc"), + iox::cxx::RelativePathComponents::ACCEPT)); } TEST(Helplets_test_isValidPathEntry, RelativePathEntriesAreValid) { - EXPECT_TRUE(isValidPathEntry(string("."))); - EXPECT_TRUE(isValidPathEntry(string(".."))); + EXPECT_TRUE(isValidPathEntry(string("."), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_TRUE(isValidPathEntry(string(".."), + iox::cxx::RelativePathComponents::ACCEPT)); } TEST(Helplets_test_isValidPathEntry, EntriesWithEndingDotAreInvalid) { - EXPECT_FALSE(isValidPathEntry(string("abc."))); - EXPECT_FALSE(isValidPathEntry(string("19283912asdb.."))); - EXPECT_FALSE(isValidPathEntry(string("..19283912asdb.."))); - EXPECT_FALSE(isValidPathEntry(string("..192839.12a.sdb.."))); + EXPECT_FALSE(isValidPathEntry(string("abc."), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(string("19283912asdb.."), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(string("..19283912asdb.."), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(string("..192839.12a.sdb.."), + iox::cxx::RelativePathComponents::ACCEPT)); } TEST(Helplets_test_isValidPathEntry, EntriesWithDotsNotAtTheEndAreValid) { - EXPECT_TRUE(isValidPathEntry(string(".abc"))); - EXPECT_TRUE(isValidPathEntry(string(".19283912asdb"))); - EXPECT_TRUE(isValidPathEntry(string("..19283912asdb"))); - EXPECT_TRUE(isValidPathEntry(string("..192839.12a.sdb"))); + EXPECT_TRUE(isValidPathEntry(string(".abc"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_TRUE(isValidPathEntry(string(".19283912asdb"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_TRUE(isValidPathEntry(string("..19283912asdb"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_TRUE(isValidPathEntry(string("..192839.12a.sdb"), + iox::cxx::RelativePathComponents::ACCEPT)); } TEST(Helplets_test_isValidPathEntry, StringContainingAllValidCharactersIsValid) { EXPECT_TRUE(isValidPathEntry(string( - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-.:_"))); + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-.:_"), + iox::cxx::RelativePathComponents::ACCEPT)); } TEST(Helplets_test_isValidPathEntry, StringWithSlashIsInvalid) { - EXPECT_FALSE(isValidPathEntry(string("/fuuuu/"))); - EXPECT_FALSE(isValidPathEntry(string("fuu/uu"))); - EXPECT_FALSE(isValidPathEntry(string("/fuuuu"))); - EXPECT_FALSE(isValidPathEntry(string("uuuubbuu/"))); + EXPECT_FALSE(isValidPathEntry(string("/fuuuu/"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(string("fuu/uu"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(string("/fuuuu"), + iox::cxx::RelativePathComponents::ACCEPT)); + EXPECT_FALSE(isValidPathEntry(string("uuuubbuu/"), + iox::cxx::RelativePathComponents::ACCEPT)); +} + +TEST(Helplets_test_isValidPathEntry, StringWithRelativeComponentsIsInvalidWhenItContainsRelativeComponents) +{ + EXPECT_FALSE(isValidPathEntry(string("../to/be"), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("../../or/not"), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("to/../be"), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("that/../../is/the/question"), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("whether/tis/nobler/.."), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("in/the/mind/to/suffer//../.."), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("../the/slings/and/arrows/../.."), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("../of/../outrageous/fortune/../.."), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("./or/to/take/../arms/../.."), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("./agains/a/see/./of/troubles/../.."), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("./and/by/../opposing/./."), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("./end/them"), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("to/./die"), + iox::cxx::RelativePathComponents::REJECT)); + EXPECT_FALSE(isValidPathEntry(string("to/./sleep/."), + iox::cxx::RelativePathComponents::REJECT)); } + TEST(Helplets_test_from, fromWorksAsConstexpr) { ::testing::Test::RecordProperty("TEST_ID", "5b7cac32-c0ef-4f29-8314-59ed8850d1f5"); diff --git a/iceoryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp b/iceoryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp index de6fcf2160..aa4e099d62 100644 --- a/iceoryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp +++ b/iceoryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp @@ -47,7 +47,7 @@ class RouDiApp protected: /// @brief waits for the next signal to RouDi daemon - [[deprecated("removed in 4.0, use iox::posix::waitForTerminationRequest() from " + [[deprecated("in 3.0, removed in 4.0, use iox::posix::waitForTerminationRequest() from " "'iceoryx_hoofs/posix_wrapper/signal_watcher.hpp'")]] bool waitForSignal() noexcept;