Added constrains to id generator
This commit prevents id with more than one '/'. Slash can be used to
create namespaces to avoid id conflicts between multiple users.
Changed generator logic to take provided id as a name if no name was
provided.
Tested:
- Tested that id and name generates correctly
- It is no longer possible to add id with more than one '/'
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: Ieef32ddb71b5a4870117aab0d624cbd46b5893e6
diff --git a/src/report.cpp b/src/report.cpp
index ed869b9..2b7134a 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -250,7 +250,6 @@
"ReportUpdates", std::string(),
sdbusplus::vtable::property_::emits_change,
[this](auto newVal, auto& oldVal) {
- ReportManager::verifyReportUpdates(utils::toReportUpdates(newVal));
setReportUpdates(utils::toReportUpdates(newVal));
oldVal = newVal;
return 1;
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 18636f8..7ac8537 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -124,16 +124,6 @@
reports.end());
}
-void ReportManager::verifyReportIdLength(const std::string& reportId)
-{
- if (reportId.length() > maxReportIdLength)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Report id exceeds maximum length");
- }
-}
-
void ReportManager::verifyAddReport(
const std::string& reportId, const std::string& reportName,
const ReportingType reportingType, Milliseconds interval,
@@ -154,20 +144,6 @@
"Reached maximal report count");
}
- verifyReportIdLength(reportId);
- utils::verifyIdCharacters(reportId);
-
- for (const auto& report : reports)
- {
- if (report->getId() == reportId)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::file_exists), "Duplicate report");
- }
- }
-
- verifyReportUpdates(reportUpdates);
-
if (reportingType == ReportingType::periodic && interval < minInterval)
{
throw sdbusplus::exception::SdBusError(
@@ -222,17 +198,11 @@
std::vector<LabeledMetricParameters> labeledMetricParams,
const bool enabled)
{
- std::string name = reportName;
- if (name.empty())
- {
- name = "Report";
- }
-
const auto existingReportIds = utils::transform(
reports, [](const auto& report) { return report->getId(); });
- std::string id =
- utils::generateId(reportId, name, existingReportIds, maxReportIdLength);
+ auto [id, name] = utils::generateId(reportId, reportName, reportNameDefault,
+ existingReportIds, maxReportIdLength);
verifyAddReport(id, name, reportingType, interval, reportUpdates,
labeledMetricParams);
@@ -304,14 +274,3 @@
}
}
}
-
-void ReportManager::verifyReportUpdates(const ReportUpdates reportUpdates)
-{
- if (std::find(supportedReportUpdates.begin(), supportedReportUpdates.end(),
- reportUpdates) == supportedReportUpdates.end())
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Invalid ReportUpdates");
- }
-}
diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index 5a850f0..34f98e0 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -39,7 +39,6 @@
std::unique_ptr<sdbusplus::asio::dbus_interface> reportManagerIface;
std::vector<std::unique_ptr<interfaces::Report>> reports;
- void verifyReportIdLength(const std::string& reportName);
void verifyAddReport(
const std::string& reportId, const std::string& reportName,
const ReportingType reportingType, Milliseconds interval,
@@ -70,9 +69,5 @@
"xyz.openbmc_project.Telemetry.ReportManager";
static constexpr const char* reportManagerPath =
"/xyz/openbmc_project/Telemetry/Reports";
- static constexpr std::array<ReportUpdates, 3> supportedReportUpdates = {
- ReportUpdates::overwrite, ReportUpdates::appendStopsWhenFull,
- ReportUpdates::appendWrapsWhenFull};
-
- static void verifyReportUpdates(const ReportUpdates reportUpdates);
+ static constexpr const char* reportNameDefault = "Report";
};
diff --git a/src/trigger_manager.cpp b/src/trigger_manager.cpp
index 358ee78..c0b20d7 100644
--- a/src/trigger_manager.cpp
+++ b/src/trigger_manager.cpp
@@ -61,7 +61,6 @@
"Reached maximal trigger count");
}
- verifyTriggerIdLength(triggerId);
utils::verifyIdCharacters(triggerId);
for (const auto& trigger : triggers)
@@ -74,26 +73,6 @@
}
}
-void TriggerManager::verifyTriggerIdLength(const std::string& triggerId)
-{
- if (triggerId.length() > maxTriggerIdLength)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Trigger id exceeds maximum length");
- }
-}
-
-std::string TriggerManager::generateId(const std::string& prefix,
- const std::string& triggerName) const
-{
- const auto existingTriggerIds = utils::transform(
- triggers, [](const auto& trigger) { return trigger->getId(); });
-
- return utils::generateId(prefix, triggerName, existingTriggerIds,
- maxTriggerIdLength);
-}
-
interfaces::Trigger& TriggerManager::addTrigger(
const std::string& triggerIdIn, const std::string& triggerNameIn,
const std::vector<std::string>& triggerActions,
@@ -101,19 +80,18 @@
const std::vector<std::string>& reportIds,
const LabeledTriggerThresholdParams& labeledThresholdParams)
{
- std::string triggerName = triggerNameIn;
- if (triggerName.empty())
- {
- triggerName = triggerNameDefault;
- }
+ const auto existingTriggerIds = utils::transform(
+ triggers, [](const auto& trigger) { return trigger->getId(); });
- std::string triggerId = generateId(triggerIdIn, triggerName);
+ auto [id, name] =
+ utils::generateId(triggerIdIn, triggerNameIn, triggerNameDefault,
+ existingTriggerIds, maxTriggerIdLength);
- verifyAddTrigger(triggerId, triggerName);
+ verifyAddTrigger(id, name);
triggers.emplace_back(triggerFactory->make(
- triggerId, triggerName, triggerActions, reportIds, *this,
- *triggerStorage, labeledThresholdParams, labeledSensorsInfo));
+ id, name, triggerActions, reportIds, *this, *triggerStorage,
+ labeledThresholdParams, labeledSensorsInfo));
return *triggers.back();
}
diff --git a/src/trigger_manager.hpp b/src/trigger_manager.hpp
index d11e57a..72b6274 100644
--- a/src/trigger_manager.hpp
+++ b/src/trigger_manager.hpp
@@ -33,9 +33,6 @@
void verifyAddTrigger(const std::string& triggerId,
const std::string& triggerName) const;
- std::string generateId(const std::string& prefix,
- const std::string& triggerName) const;
- static void verifyTriggerIdLength(const std::string& triggerId);
interfaces::Trigger&
addTrigger(const std::string& triggerId, const std::string& triggerName,
diff --git a/src/types/report_updates.hpp b/src/types/report_updates.hpp
index a47e8fb..2640bf0 100644
--- a/src/types/report_updates.hpp
+++ b/src/types/report_updates.hpp
@@ -13,8 +13,7 @@
{
overwrite,
appendStopsWhenFull,
- appendWrapsWhenFull,
- newReport
+ appendWrapsWhenFull
};
namespace utils
@@ -31,16 +30,13 @@
}
};
-constexpr std::array<std::pair<std::string_view, ReportUpdates>, 4>
- convDataReportUpdates = {
- {std::make_pair<std::string_view, ReportUpdates>(
- "Overwrite", ReportUpdates::overwrite),
- std::make_pair<std::string_view, ReportUpdates>(
- "AppendStopsWhenFull", ReportUpdates::appendStopsWhenFull),
- std::make_pair<std::string_view, ReportUpdates>(
- "AppendWrapsWhenFull", ReportUpdates::appendWrapsWhenFull),
- std::make_pair<std::string_view, ReportUpdates>(
- "NewReport", ReportUpdates::newReport)}};
+constexpr auto convDataReportUpdates =
+ std::array{std::make_pair<std::string_view, ReportUpdates>(
+ "Overwrite", ReportUpdates::overwrite),
+ std::make_pair<std::string_view, ReportUpdates>(
+ "AppendStopsWhenFull", ReportUpdates::appendStopsWhenFull),
+ std::make_pair<std::string_view, ReportUpdates>(
+ "AppendWrapsWhenFull", ReportUpdates::appendWrapsWhenFull)};
inline ReportUpdates
toReportUpdates(std::underlying_type_t<ReportUpdates> value)
diff --git a/src/utils/generate_id.cpp b/src/utils/generate_id.cpp
index adb1a5b..6631e69 100644
--- a/src/utils/generate_id.cpp
+++ b/src/utils/generate_id.cpp
@@ -12,28 +12,44 @@
static constexpr std::string_view allowedCharactersInId =
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_/";
-}
-
-void verifyIdCharacters(std::string_view triggerId)
+size_t countDigits(size_t value)
{
- if (triggerId.find_first_not_of(details::allowedCharactersInId) !=
- std::string::npos)
+ size_t result = 1;
+ while (value >= 10)
{
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::invalid_argument),
- "Invalid character in id");
+ ++result;
+ value /= 10;
}
+ return result;
}
-std::string generateId(std::string_view prefix, std::string_view name,
+std::string generateId(std::string_view id, std::string_view name,
const std::vector<std::string>& conflictIds,
size_t maxLength)
{
- verifyIdCharacters(prefix);
+ verifyIdCharacters(id);
- if (!prefix.empty() && !prefix.ends_with('/'))
+ if (id.starts_with('/'))
{
- return std::string(prefix);
+ 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);
@@ -45,7 +61,7 @@
std::string_view::npos;
}),
strippedId.end());
- strippedId = std::string(prefix) + strippedId;
+ strippedId = std::string(id) + strippedId;
size_t idx = 0;
std::string tmpId = strippedId.substr(0, maxLength);
@@ -54,11 +70,65 @@
conflictIds.end() ||
tmpId.empty())
{
- tmpId = strippedId.substr(0, maxLength - std::to_string(idx).length()) +
- std::to_string(idx);
+ 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(details::allowedCharactersInId) !=
+ 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
index aa9508e..94952dc 100644
--- a/src/utils/generate_id.hpp
+++ b/src/utils/generate_id.hpp
@@ -7,9 +7,9 @@
namespace utils
{
-void verifyIdCharacters(std::string_view triggerId);
-std::string generateId(std::string_view prefix, std::string_view triggerName,
- const std::vector<std::string>& conflictIds,
- size_t maxLength);
+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