Add extra input validation for AddReport
Now AddReport checks if ReadingParams exceeds limits that is set
to 200. AddReport checks if interval is less than allowed interval
only for Periodic reports. AddReport verify if only single sensor
for each reading parameters is provided.
Tested:
- Ran unit-tests with success.
- Added few reports in OpenBMC environment to check if results are
as expected.
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
Change-Id: Ife4f8a4cf6a8dc3fa526b66e4c698a57a733de48
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index f9840d3..21f0ac1 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -57,7 +57,9 @@
}
void ReportManager::verifyAddReport(const std::string& reportName,
- std::chrono::milliseconds interval)
+ const std::string& reportingType,
+ std::chrono::milliseconds interval,
+ const ReadingParameters& readingParams)
{
if (reports.size() >= maxReports)
{
@@ -75,11 +77,37 @@
}
}
- if (interval < minInterval)
+ auto found = std::find(supportedReportingType.begin(),
+ supportedReportingType.end(), reportingType);
+ if (found == supportedReportingType.end())
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Invalid reportingType");
+ }
+
+ if (reportingType == "Periodic" && interval < minInterval)
{
throw sdbusplus::exception::SdBusError(
static_cast<int>(std::errc::invalid_argument), "Invalid interval");
}
+
+ for (const auto& param : readingParams)
+ {
+ const auto& sensors = std::get<0>(param);
+ if (sensors.size() != 1)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::not_supported),
+ "Only single sensor per metric is allowed");
+ }
+ }
+ if (readingParams.size() >= maxReadingParams)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::argument_list_too_long),
+ "Too many reading parameters");
+ }
}
interfaces::Report& ReportManager::addReport(
@@ -88,7 +116,7 @@
const bool logToMetricReportsCollection, std::chrono::milliseconds interval,
ReadingParameters metricParams)
{
- verifyAddReport(reportName, interval);
+ verifyAddReport(reportName, reportingType, interval, metricParams);
reports.emplace_back(reportFactory->make(
yield, reportName, reportingType, emitsReadingsUpdate,
@@ -103,8 +131,6 @@
std::chrono::milliseconds interval,
std::vector<LabeledMetricParameters> labeledMetricParams)
{
- verifyAddReport(reportName, interval);
-
auto metricParams = utils::transform(
labeledMetricParams, [](const LabeledMetricParameters& param) {
using namespace utils::tstring;
@@ -118,6 +144,8 @@
param.at_index<1>(), param.at_index<2>(), param.at_index<3>());
});
+ verifyAddReport(reportName, reportingType, interval, metricParams);
+
reports.emplace_back(reportFactory->make(
reportName, reportingType, emitsReadingsUpdate,
logToMetricReportsCollection, interval, std::move(metricParams), *this,
diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index 0e37144..85358cc 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -9,6 +9,7 @@
#include <chrono>
#include <memory>
+#include <string>
#include <vector>
class ReportManager : public interfaces::ReportManager
@@ -36,7 +37,9 @@
std::vector<std::unique_ptr<interfaces::Report>> reports;
void verifyAddReport(const std::string& reportName,
- std::chrono::milliseconds interval);
+ const std::string& reportingType,
+ std::chrono::milliseconds interval,
+ const ReadingParameters& readingParams);
interfaces::Report& addReport(boost::asio::yield_context& yield,
const std::string& reportName,
const std::string& reportingType,
@@ -53,9 +56,12 @@
public:
static constexpr uint32_t maxReports{20};
+ static constexpr uint32_t maxReadingParams{200};
static constexpr std::chrono::milliseconds minInterval{1000};
static constexpr const char* reportManagerIfaceName =
"xyz.openbmc_project.Telemetry.ReportManager";
static constexpr const char* reportManagerPath =
"/xyz/openbmc_project/Telemetry/Reports";
+ static constexpr std::array<std::string_view, 2> supportedReportingType = {
+ "Periodic", "OnRequest"};
};
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index c780556..1a81844 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -120,6 +120,7 @@
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
.Times(0);
+ reportParams.reportingType("Periodic");
reportParams.interval(reportParams.interval() - 1ms);
auto [ec, path] = addReport(reportParams);
@@ -127,6 +128,39 @@
EXPECT_THAT(path, Eq(std::string()));
}
+TEST_F(TestReportManager, DISABLED_failToAddReportWithInvalidReportingType)
+{
+ reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
+ .Times(0);
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
+ .Times(0);
+
+ reportParams.reportingType("Invalid");
+
+ 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_failToAddReportWithMoreSensorsThanExpected)
+{
+ reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))
+ .Times(0);
+ reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock), _)
+ .Times(0);
+
+ auto readingParams = reportParams.readingParameters();
+ for (size_t i = 0; i < ReportManager::maxReadingParams + 1; i++)
+ {
+ readingParams.push_back(readingParams.front());
+ }
+ 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()));
+}
+
TEST_F(TestReportManager, DISABLED_failToAddReportWhenMaxReportIsReached)
{
reportFactoryMock.expectMake(_, std::nullopt, Ref(*sut), Ref(storageMock))