Add length limit for user-defined values
Previously, the only limit for most user-defined values like id or name
was the one enforced by dbus, which was fairly big. In order to save
space used by persistent data, new meson options were added, with length
limit for those fields.
This directly impacts following dbus interfaces:
- TriggerManager.AddTrigger:
- Id
- Name
- Reports
- Thresholds (only name of discrete threshold)
- Trigger.Name
- Trigger.Reports
- Trigger.Thresholds (only name of discrete threshold)
- ReportManager.AddReport(FutureVersion):
- Id
- Name
- MetricParameters (metricId)
- Report.Name
- Report.ReadingParameters (metricId)
For Id fields we support 'prefixes', which also are limited, but those
limit are separate. So if limit for prefix is set to 5 and limit for
id/name is set to 5, following Ids are fine:
- 12345/12345
- 12345
and following are not:
- 123456/1234
- 1/123456
Testing done:
- UTs are passing.
- new limits are reflected when calling mentioned dbus interfaces.
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Change-Id: I29291a1cc56a344d92fb65491c9186fdb90a8529
diff --git a/tests/src/test_make_id_name.cpp b/tests/src/test_make_id_name.cpp
new file mode 100644
index 0000000..e88a8ac
--- /dev/null
+++ b/tests/src/test_make_id_name.cpp
@@ -0,0 +1,189 @@
+#include "helpers.hpp"
+#include "utils/dbus_path_utils.hpp"
+#include "utils/make_id_name.hpp"
+#include "utils/string_utils.hpp"
+
+#include <sdbusplus/exception.hpp>
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+using namespace std::literals::string_literals;
+using namespace utils::string_utils;
+
+class ScenarioBase : public Test
+{
+ public:
+ std::string_view defaultName = "defName";
+ std::vector<std::string> conflicts;
+};
+
+class ScenarioNameProvided : public ScenarioBase
+{
+ public:
+ auto makeIdName(std::string_view id, std::string_view name) const
+ {
+ return utils::makeIdName(id, name, defaultName, conflicts);
+ }
+};
+
+TEST_F(ScenarioNameProvided, throwsWhenProvidedNameIsTooLong)
+{
+ EXPECT_THROW(this->makeIdName("", getTooLongName()),
+ sdbusplus::exception::SdBusError);
+}
+
+class TestMakeIdNameNameProvided : public ScenarioNameProvided
+{};
+
+TEST_F(TestMakeIdNameNameProvided, usesIdWhenProvided)
+{
+ const std::string name = "name";
+
+ EXPECT_THAT(this->makeIdName("id0", name), Eq(std::pair{"id0"s, name}));
+ EXPECT_THAT(this->makeIdName("prefix/id2", name),
+ Eq(std::pair{"prefix/id2"s, name}));
+}
+
+TEST_F(TestMakeIdNameNameProvided, usedDefaultWhenNothingProvided)
+{
+ this->defaultName = "def";
+
+ EXPECT_THAT(this->makeIdName("", ""), Eq(std::pair{"def"s, "def"s}));
+ EXPECT_THAT(this->makeIdName("abc/", ""),
+ Eq(std::pair{"abc/def"s, "def"s}));
+}
+
+TEST_F(TestMakeIdNameNameProvided, usedDefaultWhenNameContainsNoIdChars)
+{
+ this->defaultName = "def";
+ const std::string name = " !";
+
+ EXPECT_THAT(this->makeIdName("", name), Eq(std::pair{"def"s, name}));
+ EXPECT_THAT(this->makeIdName("prefix/", name),
+ Eq(std::pair{"prefix/def"s, name}));
+}
+
+class ScenarioNameNotProvided : public ScenarioBase
+{
+ public:
+ auto makeIdName(std::string_view id, std::string_view name) const
+ {
+ return utils::makeIdName(id, "", name, conflicts);
+ }
+};
+
+class TestMakeIdNameNameNotProvided : public ScenarioNameNotProvided
+{};
+
+TEST_F(TestMakeIdNameNameNotProvided, usesIdAsNameWhenProvided)
+{
+ EXPECT_THAT(this->makeIdName("id0", defaultName),
+ Eq(std::pair{"id0"s, "id0"s}));
+ EXPECT_THAT(this->makeIdName("prefix/id2", defaultName),
+ Eq(std::pair{"prefix/id2"s, "id2"s}));
+}
+
+template <class Scenario>
+class TestMakeIdName : public Scenario
+{};
+
+using TestScenarios =
+ ::testing::Types<ScenarioNameProvided, ScenarioNameNotProvided>;
+TYPED_TEST_SUITE(TestMakeIdName, TestScenarios);
+
+TYPED_TEST(TestMakeIdName, throwsWhenProvidedIdContainsIncorrectCharacters)
+{
+ EXPECT_THROW(this->makeIdName("Id%@%!@%!%()%fooo/Id", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("Id/Id%@%!@%!%()%fooo", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("/123", "trigger"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("/123/", "trigger"),
+ sdbusplus::exception::SdBusError);
+}
+
+TYPED_TEST(TestMakeIdName, throwsWhenProvidedIdContainsTooLongSegment)
+{
+ std::string longPrefix = getTooLongPrefix();
+ std::string longSuffix = getTooLongId();
+ EXPECT_THROW(this->makeIdName(longPrefix + "/", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName(longPrefix + "/Id", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName(longPrefix + "/" + longSuffix, "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("Prefix/" + longSuffix, "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName(longSuffix, "name"),
+ sdbusplus::exception::SdBusError);
+}
+
+TYPED_TEST(TestMakeIdName, throwsWhenProvidedIdOrPrefixTooLong)
+{
+ EXPECT_THROW(this->makeIdName(getTooLongId(), "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName(getTooLongPrefix() + "/Id", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("Prefix/" + getTooLongId(), "trigger"),
+ sdbusplus::exception::SdBusError);
+}
+
+TYPED_TEST(TestMakeIdName, throwsWhenIdContainsMoreThanOneSlash)
+{
+ EXPECT_THROW(this->makeIdName("/12/", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("12//", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("12//123", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("12/12/123", "name"),
+ sdbusplus::exception::SdBusError);
+}
+
+TYPED_TEST(TestMakeIdName, usesNameWhenThereAreConflicts)
+{
+ this->conflicts = {"trigger"};
+ EXPECT_THAT(this->makeIdName("", "trigger"),
+ Eq(std::pair{"trigger0"s, "trigger"s}));
+
+ this->conflicts = {"trigger", "trigger0"};
+ EXPECT_THAT(this->makeIdName("", "trigger"),
+ Eq(std::pair{"trigger1"s, "trigger"s}));
+
+ this->conflicts = {getMaxId()};
+ std::string expectedId = getMaxId();
+ expectedId[expectedId.length() - 1] = '0';
+ EXPECT_THAT(this->makeIdName("", getMaxId()),
+ Eq(std::pair{expectedId, getMaxId()}));
+}
+
+TYPED_TEST(TestMakeIdName, throwsWhenProvidedIdIsTaken)
+{
+ this->conflicts = {"id", "prefix/id"};
+
+ EXPECT_THROW(this->makeIdName("id", "name"),
+ sdbusplus::exception::SdBusError);
+ EXPECT_THROW(this->makeIdName("prefix/id", "name"),
+ sdbusplus::exception::SdBusError);
+}
+
+TYPED_TEST(TestMakeIdName, usesNameWhenIdNotProvided)
+{
+ EXPECT_THAT(this->makeIdName("", "name"), Eq(std::pair{"name"s, "name"s}));
+ EXPECT_THAT(this->makeIdName("abc/", "name"),
+ Eq(std::pair{"abc/name"s, "name"s}));
+ EXPECT_THAT(this->makeIdName("123/", "name"),
+ Eq(std::pair{"123/name"s, "name"s}));
+}
+
+TYPED_TEST(TestMakeIdName, usesNameWithoutInvalidCharactersWhenIdNotProvided)
+{
+ EXPECT_THAT(this->makeIdName("", "n#a$/m@e"),
+ Eq(std::pair{"name"s, "n#a$/m@e"s}));
+ EXPECT_THAT(this->makeIdName("", "n!^aŹ/me"),
+ Eq(std::pair{"name"s, "n!^aŹ/me"s}));
+ EXPECT_THAT(this->makeIdName("p/", "n!^aŹ/m*(e"),
+ Eq(std::pair{"p/name"s, "n!^aŹ/m*(e"s}));
+}