Skip to content

Commit

Permalink
iox-#1036 Remove deprecated function, refactor isValidPathEntry to re…
Browse files Browse the repository at this point in the history 
…duce code duplication, add further tests

Signed-off-by: Christian Eltzschig <me@elchris.org>
  • Loading branch information
@elfenpiff
elfenpiff committed Jun 29, 2022
1 parent 5d844a2 commit b6ebbc0
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 59 deletions.
13 changes: 8 additions & 5 deletions iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <uint64_t StringCapacity>
bool isValidPathEntry(const string<StringCapacity>& name) noexcept;
bool isValidPathEntry(const string<StringCapacity>& name,
const RelativePathComponents& relativePathComponents) noexcept;

/// @brief checks if the given string is a valid filename
/// @param[in] name the string to verify
Expand All @@ -268,10 +275,6 @@ bool isValidFileName(const string<StringCapacity>& name) noexcept;
template <uint64_t StringCapacity>
bool isValidPathToFile(const string<StringCapacity>& name) noexcept;

template <uint64_t StringCapacity>
[[deprecated("removed in 4.0, use isValidPathToFile")]] bool
isValidFilePath(const string<StringCapacity>& name) noexcept;

/// @brief returns true if the provided name is a valid path, otherwise false
/// @param[in] name the string to verify
template <uint64_t StringCapacity>
Expand Down
34 changes: 6 additions & 28 deletions iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/helplets.inl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,22 @@ namespace iox
namespace cxx
{
template <uint64_t StringCapacity>
inline bool isValidPathEntry(const string<StringCapacity>& name) noexcept
inline bool isValidPathEntry(const string<StringCapacity>& name,
const RelativePathComponents& relativePathComponents) noexcept
{
const string<StringCapacity> currentDirectory(".");
const string<StringCapacity> 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
Expand All @@ -64,25 +65,8 @@ inline bool isValidFileName(const string<StringCapacity>& name) noexcept
return false;
}

uint64_t nameSize = name.size();

const string<StringCapacity> currentDirectory(".");
const string<StringCapacity> 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 <uint64_t StringCapacity>
Expand All @@ -100,12 +84,6 @@ inline bool isValidPathToFile(const string<StringCapacity>& name) noexcept
return (pathPart.empty() || isValidPathToDirectory(pathPart)) && isValidFileName(filePart);
}

template <uint64_t StringCapacity>
inline bool isValidFilePath(const string<StringCapacity>& name) noexcept
{
return isValidPathToFile(name);
}

template <uint64_t StringCapacity>
inline bool isValidPathToDirectory(const string<StringCapacity>& name) noexcept
{
Expand Down Expand Up @@ -151,7 +129,7 @@ inline bool isValidPathToDirectory(const string<StringCapacity>& 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);
}
}

Expand Down
107 changes: 82 additions & 25 deletions iceoryx_hoofs/test/moduletests/test_cxx_helplets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ TEST(Helplets_test_isValidPathToFile, MultipleSlashsAreValidFilePath)
EXPECT_TRUE(isValidPathToFile(string<FILE_PATH_LENGTH>("//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<FILE_PATH_LENGTH>("../some.file")));
Expand Down Expand Up @@ -460,7 +460,8 @@ TEST(Helplets_test_isValidPathToFile, EmptyFilePathIsInvalid)
EXPECT_FALSE(isValidPathToFile(string<FILE_PATH_LENGTH>("")));
}

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";
Expand Down Expand Up @@ -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));
}
}

Expand All @@ -539,7 +544,7 @@ TEST(Helplets_test_isValidPathToDirectory, MultipleSlashsAreValidPath)
EXPECT_TRUE(isValidPathToDirectory(string<FILE_PATH_LENGTH>("//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<FILE_PATH_LENGTH>("../some.file")));
Expand Down Expand Up @@ -684,52 +689,104 @@ TEST(Helplets_test_doesEndWithPathSeparator, MultiCharacterStringEndingWithPathS

TEST(Helplets_test_isValidPathEntry, EmptyPathEntryIsValid)
{
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("")));
EXPECT_TRUE(
isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(""), iox::cxx::RelativePathComponents::ACCEPT));
}

TEST(Helplets_test_isValidPathEntry, PathEntryWithOnlyValidCharactersIsValid)
{
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("a")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("agc")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("a.213jkgc")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("a"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("agc"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("a.213jkgc"),
iox::cxx::RelativePathComponents::ACCEPT));
}

TEST(Helplets_test_isValidPathEntry, RelativePathEntriesAreValid)
{
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(".")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("."),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(".."),
iox::cxx::RelativePathComponents::ACCEPT));
}

TEST(Helplets_test_isValidPathEntry, EntriesWithEndingDotAreInvalid)
{
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("abc.")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("19283912asdb..")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..19283912asdb..")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..192839.12a.sdb..")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("abc."),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("19283912asdb.."),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..19283912asdb.."),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..192839.12a.sdb.."),
iox::cxx::RelativePathComponents::ACCEPT));
}

TEST(Helplets_test_isValidPathEntry, EntriesWithDotsNotAtTheEndAreValid)
{
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(".abc")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(".19283912asdb")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..19283912asdb")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..192839.12a.sdb")));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(".abc"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(".19283912asdb"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..19283912asdb"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("..192839.12a.sdb"),
iox::cxx::RelativePathComponents::ACCEPT));
}

TEST(Helplets_test_isValidPathEntry, StringContainingAllValidCharactersIsValid)
{
EXPECT_TRUE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>(
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-.:_")));
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-.:_"),
iox::cxx::RelativePathComponents::ACCEPT));
}

TEST(Helplets_test_isValidPathEntry, StringWithSlashIsInvalid)
{
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("/fuuuu/")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("fuu/uu")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("/fuuuu")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("uuuubbuu/")));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("/fuuuu/"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("fuu/uu"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("/fuuuu"),
iox::cxx::RelativePathComponents::ACCEPT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("uuuubbuu/"),
iox::cxx::RelativePathComponents::ACCEPT));
}

TEST(Helplets_test_isValidPathEntry, StringWithRelativeComponentsIsInvalidWhenItContainsRelativeComponents)
{
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("../to/be"),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("../../or/not"),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("to/../be"),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("that/../../is/the/question"),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("whether/tis/nobler/.."),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("in/the/mind/to/suffer//../.."),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("../the/slings/and/arrows/../.."),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("../of/../outrageous/fortune/../.."),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("./or/to/take/../arms/../.."),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("./agains/a/see/./of/troubles/../.."),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("./and/by/../opposing/./."),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("./end/them"),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("to/./die"),
iox::cxx::RelativePathComponents::REJECT));
EXPECT_FALSE(isValidPathEntry(string<iox::platform::IOX_MAX_FILENAME_LENGTH>("to/./sleep/."),
iox::cxx::RelativePathComponents::REJECT));
}


TEST(Helplets_test_from, fromWorksAsConstexpr)
{
::testing::Test::RecordProperty("TEST_ID", "5b7cac32-c0ef-4f29-8314-59ed8850d1f5");
Expand Down
2 changes: 1 addition & 1 deletion iceoryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit b6ebbc0

Please sign in to comment.