Add limit for report name length
Added limit for report name length, parametrized with
max-report-name-length option, because we cannot
remove reports with too long report name.
Tested:
- Confirmed that report with name length equal to 4096
cannot be generated via bmcweb (POST
redfish/v1/TelemetryService/MetricReportDefinitions
fails with code 500)
- Confirmed that report with name length equal to 4095
can be generated and removed via bmcweb
- Added unit-test that test that report with name
length equal to max-report-name-length + 1 cannot
be generated
- Added unit-test that test that report with name
length equal to max-report-name-length can
be generated
Change-Id: I6868320f831079af903f3624d1beff648059e351
Signed-off-by: Karol Niczyj <karol.niczyj@intel.com>
diff --git a/meson.build b/meson.build
index 90f1799..145805a 100644
--- a/meson.build
+++ b/meson.build
@@ -65,6 +65,8 @@
get_option('max-reading-parameters').to_string(),
'-DTELEMETRY_MIN_INTERVAL=' + get_option('min-interval').to_string(),
'-DTELEMETRY_MAX_TRIGGERS=' + get_option('max-triggers').to_string(),
+ '-DTELEMETRY_MAX_REPORT_NAME_LENGTH=' +
+ get_option('max-report-name-length').to_string(),
language: 'cpp'
)
diff --git a/meson_options.txt b/meson_options.txt
index cad7253..1a7896e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -7,3 +7,5 @@
description: 'Minimal value of interval in milliseconds')
option('max-triggers', type: 'integer', min: 1, value: 10,
description: 'Max number of Triggers')
+option('max-report-name-length', type: 'integer', min: 1, value: 4095,
+ description: 'Max length of Report name')
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 9ea6026..24087c3 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -28,6 +28,10 @@
"MaxReports", size_t{}, sdbusplus::vtable::property_::const_,
[](const auto&) { return maxReports; });
dbusIface.register_property_r(
+ "MaxReportNameLength", size_t{},
+ sdbusplus::vtable::property_::const_,
+ [](const auto&) { return maxReportNameLength; });
+ dbusIface.register_property_r(
"MinInterval", uint64_t{}, sdbusplus::vtable::property_::const_,
[](const auto&) -> uint64_t { return minInterval.count(); });
@@ -57,6 +61,16 @@
reports.end());
}
+void ReportManager::verifyReportNameLength(const std::string& reportName)
+{
+ if (reportName.length() > maxReportNameLength)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Report name exceed maximum length");
+ }
+}
+
void ReportManager::verifyAddReport(const std::string& reportName,
const std::string& reportingType,
std::chrono::milliseconds interval,
@@ -69,6 +83,8 @@
"Reached maximal report count");
}
+ verifyReportNameLength(reportName);
+
for (const auto& report : reports)
{
if (report->getName() == reportName)
diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index bb4d395..e66a614 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -5,6 +5,8 @@
#include "interfaces/report_factory.hpp"
#include "interfaces/report_manager.hpp"
+#include <systemd/sd-bus-protocol.h>
+
#include <sdbusplus/asio/object_server.hpp>
#include <chrono>
@@ -36,6 +38,7 @@
std::unique_ptr<sdbusplus::asio::dbus_interface> reportManagerIface;
std::vector<std::unique_ptr<interfaces::Report>> reports;
+ void verifyReportNameLength(const std::string& reportName);
void verifyAddReport(const std::string& reportName,
const std::string& reportingType,
std::chrono::milliseconds interval,
@@ -57,6 +60,8 @@
public:
static constexpr size_t maxReports{TELEMETRY_MAX_REPORTS};
static constexpr size_t maxReadingParams{TELEMETRY_MAX_READING_PARAMS};
+ static constexpr size_t maxReportNameLength{
+ TELEMETRY_MAX_REPORT_NAME_LENGTH};
static constexpr std::chrono::milliseconds minInterval{
TELEMETRY_MIN_INTERVAL};
static constexpr const char* reportManagerIfaceName =
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 519606f..4ad5479 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -84,6 +84,16 @@
});
return DbusEnvironment::waitForFuture(std::move(propertyFuture));
}
+
+ static std::string prepareReportNameWithLength(size_t length)
+ {
+ std::stringstream reportNameStream;
+ for (size_t i = 0; i < length; ++i)
+ {
+ reportNameStream << "z";
+ }
+ return reportNameStream.str();
+ }
};
TEST_F(TestReportManager, minInterval)
@@ -108,6 +118,35 @@
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
+TEST_F(TestReportManager, addReportWithMaxLengthName)
+{
+ std::string reportName =
+ prepareReportNameWithLength(ReportManager::maxReportNameLength);
+ reportParams.reportName(reportName);
+ reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock));
+
+ auto [ec, path] = addReport(reportParams);
+
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
+ EXPECT_THAT(path, Eq("/"s + reportName));
+}
+
+TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongName)
+{
+ reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
+ .Times(0);
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
+ .Times(0);
+
+ reportParams.reportName(
+ prepareReportNameWithLength(ReportManager::maxReportNameLength + 1));
+
+ auto [ec, path] = addReport(reportParams);
+
+ EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
+ EXPECT_THAT(path, Eq(std::string()));
+}
+
TEST_F(TestReportManager, DISABLED_failToAddReportTwice)
{
reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
@@ -116,6 +155,7 @@
addReport(reportParams);
auto [ec, path] = addReport(reportParams);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::file_exists));
EXPECT_THAT(path, Eq(std::string()));
}
@@ -131,6 +171,7 @@
reportParams.interval(reportParams.interval() - 1ms);
auto [ec, path] = addReport(reportParams);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(path, Eq(std::string()));
}
@@ -145,6 +186,7 @@
reportParams.reportingType("Invalid");
auto [ec, path] = addReport(reportParams);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(path, Eq(std::string()));
}
@@ -164,6 +206,7 @@
reportParams.readingParameters(std::move(readingParams));
auto [ec, path] = addReport(reportParams);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::argument_list_too_long));
EXPECT_THAT(path, Eq(std::string()));
}
@@ -184,6 +227,7 @@
reportParams.reportName(reportParams.reportName() +
std::to_string(ReportManager::maxReports));
auto [ec, path] = addReport(reportParams);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::too_many_files_open));
EXPECT_THAT(path, Eq(std::string()));
}
@@ -280,6 +324,7 @@
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
auto [ec, path] = addReport(reportParams);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
EXPECT_THAT(path, Eq("/"s + reportParams.reportName()));
}