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/src/utils/dbus_path_utils.cpp b/src/utils/dbus_path_utils.cpp
index 9d37cfc..5c1fddb 100644
--- a/src/utils/dbus_path_utils.cpp
+++ b/src/utils/dbus_path_utils.cpp
@@ -35,16 +35,48 @@
if (path.str.starts_with(constants::reportDirStr))
{
auto id = path.str.substr(constants::reportDirStr.length());
- if (std::cmp_greater(std::count(id.begin(), id.end(), '/'),
- constants::maxPrefixesInId))
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Too many prefixes in id");
- }
+ verifyIdPrefixes(id);
return id;
}
throw sdbusplus::exception::SdBusError(
static_cast<int>(std::errc::invalid_argument), "Invalid path prefix");
}
+
+void verifyIdPrefixes(std::string_view id)
+{
+ size_t pos_start = 0;
+ size_t pos_end = 0;
+ size_t prefix_cnt = 0;
+ while ((pos_end = id.find('/', pos_start)) != std::string::npos)
+ {
+ if (pos_start == pos_end)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid prefixes in id");
+ }
+
+ if (++prefix_cnt > constants::maxPrefixesInId)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Too many prefixes");
+ }
+
+ if (pos_end - pos_start > constants::maxPrefixLength)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Prefix too long");
+ }
+
+ pos_start = pos_end + 1;
+ }
+
+ if (id.length() - pos_start > constants::maxIdNameLength)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument), "Id too long");
+ }
+}
} // namespace utils
diff --git a/src/utils/dbus_path_utils.hpp b/src/utils/dbus_path_utils.hpp
index a1b5709..3798d98 100644
--- a/src/utils/dbus_path_utils.hpp
+++ b/src/utils/dbus_path_utils.hpp
@@ -19,6 +19,21 @@
constexpr std::string_view allowedCharactersInPath =
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_/";
constexpr size_t maxPrefixesInId = 1;
+constexpr size_t maxPrefixLength{TELEMETRY_MAX_PREFIX_LENGTH};
+constexpr size_t maxIdNameLength{TELEMETRY_MAX_ID_NAME_LENGTH};
+constexpr size_t maxDbusPathLength{TELEMETRY_MAX_DBUS_PATH_LENGTH};
+
+constexpr size_t maxTriggeFullIdLength{maxDbusPathLength -
+ triggerDirStr.length()};
+constexpr size_t maxReportFullIdLength{maxDbusPathLength -
+ reportDirStr.length()};
+
+static_assert(maxPrefixesInId * (maxPrefixLength + 1) + maxIdNameLength <=
+ maxTriggeFullIdLength,
+ "Misconfigured prefix/id/name lengths.");
+static_assert(maxPrefixesInId * (maxPrefixLength + 1) + maxIdNameLength <=
+ maxReportFullIdLength,
+ "Misconfigured prefix/id/name lengths.");
const sdbusplus::message::object_path triggerDirPath =
sdbusplus::message::object_path(std::string(triggerDirStr));
@@ -33,9 +48,22 @@
!path.ends_with('/');
}
+inline void verifyIdCharacters(std::string_view id)
+{
+ if (id.find_first_not_of(utils::constants::allowedCharactersInPath) !=
+ std::string::npos)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid character in id");
+ }
+}
+
sdbusplus::message::object_path pathAppend(sdbusplus::message::object_path path,
const std::string& appended);
std::string reportPathToId(const sdbusplus::message::object_path& path);
+void verifyIdPrefixes(std::string_view id);
+
} // namespace utils
diff --git a/src/utils/generate_id.cpp b/src/utils/generate_id.cpp
deleted file mode 100644
index 42c9155..0000000
--- a/src/utils/generate_id.cpp
+++ /dev/null
@@ -1,135 +0,0 @@
-#include "utils/generate_id.hpp"
-
-#include "utils/dbus_path_utils.hpp"
-
-#include <sdbusplus/exception.hpp>
-
-#include <algorithm>
-#include <system_error>
-
-namespace utils
-{
-namespace details
-{
-
-size_t countDigits(size_t value)
-{
- size_t result = 1;
- while (value >= 10)
- {
- ++result;
- value /= 10;
- }
- return result;
-}
-
-std::string generateId(std::string_view id, std::string_view name,
- const std::vector<std::string>& conflictIds,
- size_t maxLength)
-{
- verifyIdCharacters(id);
-
- if (id.starts_with('/'))
- {
- id = id.substr(1);
- }
-
- if ((id.length() > maxLength) ||
- (id.ends_with('/') && id.length() >= maxLength))
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument), "Id too long");
- }
-
- if (!id.empty() && !id.ends_with('/'))
- {
- if (std::find(conflictIds.begin(), conflictIds.end(), id) !=
- conflictIds.end())
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::file_exists), "Duplicated id");
- }
- return std::string(id);
- }
-
- std::string strippedId(name);
- strippedId.erase(
- std::remove_if(
- strippedId.begin(), strippedId.end(),
- [](char c) {
- return c == '/' ||
- utils::constants::allowedCharactersInPath.find(c) ==
- std::string_view::npos;
- }),
- strippedId.end());
- strippedId = std::string(id) + strippedId;
-
- size_t idx = 0;
- std::string tmpId = strippedId.substr(0, maxLength);
-
- while (std::find(conflictIds.begin(), conflictIds.end(), tmpId) !=
- conflictIds.end() ||
- tmpId.empty())
- {
- size_t digitsInIdx = countDigits(idx);
-
- if (digitsInIdx > maxLength)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::file_exists),
- "Unique indices are depleted");
- }
-
- tmpId =
- strippedId.substr(0, maxLength - digitsInIdx) + std::to_string(idx);
- ++idx;
- }
-
- return tmpId;
-}
-
-} // namespace details
-
-void verifyIdCharacters(std::string_view id)
-{
- if (id.find_first_not_of(utils::constants::allowedCharactersInPath) !=
- std::string::npos)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Invalid character in id");
- }
-
- if (auto pos = id.find_first_of("/");
- pos != std::string::npos && pos != id.find_last_of("/"))
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Too many '/' in id");
- }
-}
-
-std::pair<std::string, std::string> generateId(
- std::string_view id, std::string_view name, std::string_view defaultName,
- const std::vector<std::string>& conflictIds, const size_t maxLength)
-{
- if (name.empty() && !id.ends_with('/'))
- {
- name = id;
-
- if (auto pos = name.find_last_of("/"); pos != std::string::npos)
- {
- name = name.substr(pos + 1);
- }
- }
-
- if (name.empty())
- {
- name = defaultName;
- }
-
- return std::make_pair(details::generateId(id, name, conflictIds, maxLength),
- std::string{name});
-}
-
-} // namespace utils
diff --git a/src/utils/generate_id.hpp b/src/utils/generate_id.hpp
deleted file mode 100644
index 94952dc..0000000
--- a/src/utils/generate_id.hpp
+++ /dev/null
@@ -1,15 +0,0 @@
-#pragma once
-
-#include <string>
-#include <string_view>
-#include <vector>
-
-namespace utils
-{
-
-void verifyIdCharacters(std::string_view id);
-std::pair<std::string, std::string> generateId(
- std::string_view id, std::string_view name, std::string_view defaultName,
- const std::vector<std::string>& conflictIds, const size_t maxLength);
-
-} // namespace utils
diff --git a/src/utils/make_id_name.cpp b/src/utils/make_id_name.cpp
new file mode 100644
index 0000000..0df6c17
--- /dev/null
+++ b/src/utils/make_id_name.cpp
@@ -0,0 +1,122 @@
+#include "utils/make_id_name.hpp"
+
+#include "utils/dbus_path_utils.hpp"
+
+#include <sdbusplus/exception.hpp>
+
+#include <algorithm>
+#include <system_error>
+
+namespace utils
+{
+namespace details
+{
+
+size_t countDigits(size_t value)
+{
+ size_t result = 1;
+ while (value >= 10)
+ {
+ ++result;
+ value /= 10;
+ }
+ return result;
+}
+
+std::string generateId(std::string_view idIn, std::string_view nameIn,
+ std::string_view defaultName,
+ const std::vector<std::string>& conflictIds)
+{
+ verifyIdCharacters(idIn);
+ verifyIdPrefixes(idIn);
+
+ if (!idIn.empty() && !idIn.ends_with('/'))
+ {
+ if (std::find(conflictIds.begin(), conflictIds.end(), idIn) !=
+ conflictIds.end())
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::file_exists), "Duplicated id");
+ }
+ return std::string(idIn);
+ }
+
+ const std::string prefixes(idIn);
+
+ std::string strippedId(nameIn);
+ if (strippedId.find_first_of(utils::constants::allowedCharactersInPath) ==
+ std::string::npos)
+ {
+ strippedId = defaultName;
+ }
+ strippedId.erase(
+ std::remove_if(
+ strippedId.begin(), strippedId.end(),
+ [](char c) {
+ return c == '/' ||
+ utils::constants::allowedCharactersInPath.find(c) ==
+ std::string_view::npos;
+ }),
+ strippedId.end());
+
+ size_t idx = 0;
+ std::string tmpId =
+ prefixes + strippedId.substr(0, constants::maxIdNameLength);
+
+ while (std::find(conflictIds.begin(), conflictIds.end(), tmpId) !=
+ conflictIds.end())
+ {
+ size_t digitsInIdx = countDigits(idx);
+
+ if (digitsInIdx > constants::maxIdNameLength)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::file_exists),
+ "Unique indices are depleted");
+ }
+
+ tmpId = prefixes +
+ strippedId.substr(0, constants::maxIdNameLength - digitsInIdx) +
+ std::to_string(idx);
+ ++idx;
+ }
+
+ return tmpId;
+}
+
+} // namespace details
+
+std::pair<std::string, std::string>
+ makeIdName(std::string_view id, std::string_view name,
+ std::string_view defaultName,
+ const std::vector<std::string>& conflictIds)
+{
+ if (name.length() > constants::maxIdNameLength)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument), "Name too long");
+ }
+
+ if (name.empty() && !id.ends_with('/'))
+ {
+ name = id;
+
+ if (auto pos = name.find_last_of("/"); pos != std::string::npos)
+ {
+ name = name.substr(pos + 1);
+ }
+
+ name = name.substr(0, constants::maxIdNameLength);
+ }
+
+ if (name.empty())
+ {
+ name = defaultName;
+ }
+
+ return std::make_pair(
+ details::generateId(id, name, defaultName, conflictIds),
+ std::string{name});
+}
+
+} // namespace utils
diff --git a/src/utils/make_id_name.hpp b/src/utils/make_id_name.hpp
new file mode 100644
index 0000000..2c4ce75
--- /dev/null
+++ b/src/utils/make_id_name.hpp
@@ -0,0 +1,15 @@
+#pragma once
+
+#include <string>
+#include <string_view>
+#include <vector>
+
+namespace utils
+{
+
+std::pair<std::string, std::string>
+ makeIdName(std::string_view id, std::string_view name,
+ std::string_view defaultName,
+ const std::vector<std::string>& conflictIds);
+
+} // namespace utils