Make log services use parameter delegation
The commit prior to this one added support for delegation of $expand and
$only query param types; This commit adds support for delegation of top
and skip (which we already have a few handlers for) and moves them to
the new style.
Note, this makes top and skip query params NOT below the
insecure-enable-redfish-query. top and skip have existed for a while,
and are unlikely to have security issues, as they're relatively simple
transforms.
Tested:
curl --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Managers/bmc/LogServices/Journal/Entries\?\$top\=3\&\$skip\=0
With varying $top between 1-5 and $skip between 0-5 gave the expected
number of log results.
Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia213a5e929c40579825eaf251e4b9159bc84c802
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 67595cc..042451f 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -32,6 +32,12 @@
// Expand
uint8_t expandLevel = 0;
ExpandType expandType = ExpandType::None;
+
+ // Skip
+ size_t skip = 0;
+
+ // Top
+ size_t top = std::numeric_limits<size_t>::max();
};
// The struct defines how resource handlers in redfish-core/lib/ can handle
@@ -40,6 +46,8 @@
struct QueryCapabilities
{
bool canDelegateOnly = false;
+ bool canDelegateTop = false;
+ bool canDelegateSkip = false;
uint8_t canDelegateExpandLevel = 0;
};
@@ -73,6 +81,20 @@
delegated.expandLevel = queryCapabilities.canDelegateExpandLevel;
}
}
+
+ // delegate top
+ if (queryCapabilities.canDelegateTop)
+ {
+ delegated.top = query.top;
+ query.top = std::numeric_limits<size_t>::max();
+ }
+
+ // delegate skip
+ if (queryCapabilities.canDelegateSkip)
+ {
+ delegated.skip = query.skip;
+ query.skip = 0;
+ }
return delegated;
}
@@ -121,6 +143,54 @@
return value == ")";
}
+enum class QueryError
+{
+ Ok,
+ OutOfRange,
+ ValueFormat,
+};
+
+inline QueryError getNumericParam(std::string_view value, size_t& param)
+{
+ std::from_chars_result r =
+ std::from_chars(value.data(), value.data() + value.size(), param);
+
+ // If the number wasn't representable in the type, it's out of range
+ if (r.ec == std::errc::result_out_of_range)
+ {
+ return QueryError::OutOfRange;
+ }
+ // All other errors are value format
+ if (r.ec != std::errc())
+ {
+ return QueryError::ValueFormat;
+ }
+ return QueryError::Ok;
+}
+
+inline QueryError getSkipParam(std::string_view value, Query& query)
+{
+ return getNumericParam(value, query.skip);
+}
+
+static constexpr size_t maxEntriesPerPage = 1000;
+inline QueryError getTopParam(std::string_view value, Query& query)
+{
+ QueryError ret = getNumericParam(value, query.top);
+ if (ret != QueryError::Ok)
+ {
+ return ret;
+ }
+
+ // Range check for sanity.
+ if (query.top > maxEntriesPerPage)
+ {
+ return QueryError::OutOfRange;
+ }
+
+ return QueryError::Ok;
+}
+
inline std::optional<Query>
parseParameters(const boost::urls::params_view& urlParams,
crow::Response& res)
@@ -147,6 +217,38 @@
return std::nullopt;
}
}
+ else if (key == "$top")
+ {
+ QueryError topRet = getTopParam(value, ret);
+ if (topRet == QueryError::ValueFormat)
+ {
+ messages::queryParameterValueFormatError(res, value, key);
+ return std::nullopt;
+ }
+ if (topRet == QueryError::OutOfRange)
+ {
+ messages::queryParameterOutOfRange(
+ res, value, "$top",
+ "1-" + std::to_string(maxEntriesPerPage));
+ return std::nullopt;
+ }
+ }
+ else if (key == "$skip")
+ {
+ QueryError topRet = getSkipParam(value, ret);
+ if (topRet == QueryError::ValueFormat)
+ {
+ messages::queryParameterValueFormatError(res, value, key);
+ return std::nullopt;
+ }
+ if (topRet == QueryError::OutOfRange)
+ {
+ messages::queryParameterOutOfRange(
+ res, value, key,
+ "1-" + std::to_string(std::numeric_limits<size_t>::max()));
+ return std::nullopt;
+ }
+ }
else
{
// Intentionally ignore other errors Redfish spec, 7.3.1
diff --git a/redfish-core/include/utils/query_param_test.cpp b/redfish-core/include/utils/query_param_test.cpp
index 1a0cf15..7897807 100644
--- a/redfish-core/include/utils/query_param_test.cpp
+++ b/redfish-core/include/utils/query_param_test.cpp
@@ -65,6 +65,52 @@
EXPECT_EQ(delegated.expandType, ExpandType::None);
}
+TEST(Delegate, TopNegative)
+{
+ Query query{
+ .top = 42,
+ };
+ Query delegated = delegate(QueryCapabilities{}, query);
+ EXPECT_EQ(delegated.top, std::numeric_limits<size_t>::max());
+ EXPECT_EQ(query.top, 42);
+}
+
+TEST(Delegate, TopPositive)
+{
+ Query query{
+ .top = 42,
+ };
+ QueryCapabilities capabilities{
+ .canDelegateTop = false,
+ };
+ Query delegated = delegate(capabilities, query);
+ EXPECT_EQ(delegated.top, std::numeric_limits<size_t>::max());
+ EXPECT_EQ(query.top, 42);
+}
+
+TEST(Delegate, SkipNegative)
+{
+ Query query{
+ .skip = 42,
+ };
+ Query delegated = delegate(QueryCapabilities{}, query);
+ EXPECT_EQ(delegated.skip, 0);
+ EXPECT_EQ(query.skip, 42);
+}
+
+TEST(Delegate, SkipPositive)
+{
+ Query query{
+ .skip = 42,
+ };
+ QueryCapabilities capabilities{
+ .canDelegateSkip = false,
+ };
+ Query delegated = delegate(capabilities, query);
+ EXPECT_EQ(delegated.skip, 0);
+ EXPECT_EQ(query.skip, 42);
+}
+
} // namespace
} // namespace redfish::query_param
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index 49911f3..3a8ce10 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -180,49 +180,6 @@
return true;
}
-static bool getSkipParam(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req, uint64_t& skip)
-{
- boost::urls::params_view::iterator it = req.urlView.params().find("$skip");
- if (it != req.urlView.params().end())
- {
- std::from_chars_result r = std::from_chars(
- (*it).value.data(), (*it).value.data() + (*it).value.size(), skip);
- if (r.ec != std::errc())
- {
- messages::queryParameterValueTypeError(asyncResp->res, "", "$skip");
- return false;
- }
- }
- return true;
-}
-
-static constexpr const uint64_t maxEntriesPerPage = 1000;
-static bool getTopParam(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req, uint64_t& top)
-{
- boost::urls::params_view::iterator it = req.urlView.params().find("$top");
- if (it != req.urlView.params().end())
- {
- std::from_chars_result r = std::from_chars(
- (*it).value.data(), (*it).value.data() + (*it).value.size(), top);
- if (r.ec != std::errc())
- {
- messages::queryParameterValueTypeError(asyncResp->res, "", "$top");
- return false;
- }
- if (top < 1U || top > maxEntriesPerPage)
- {
-
- messages::queryParameterOutOfRange(
- asyncResp->res, std::to_string(top), "$top",
- "1-" + std::to_string(maxEntriesPerPage));
- return false;
- }
- }
- return true;
-}
-
inline static bool getUniqueEntryID(sd_journal* journal, std::string& entryID,
const bool firstEntry = true)
{
@@ -1200,17 +1157,13 @@
const std::shared_ptr<
bmcweb::AsyncResp>&
asyncResp) {
- if (!redfish::setUpRedfishRoute(app, req, asyncResp->res))
- {
- return;
- }
- uint64_t skip = 0;
- uint64_t top = maxEntriesPerPage; // Show max entries by default
- if (!getSkipParam(asyncResp, req, skip))
- {
- return;
- }
- if (!getTopParam(asyncResp, req, top))
+ query_param::QueryCapabilities capabilities = {
+ .canDelegateTop = true,
+ .canDelegateSkip = true,
+ };
+ query_param::Query delegatedQuery;
+ if (!redfish::setUpRedfishRouteWithDelegation(
+ app, req, asyncResp->res, delegatedQuery, capabilities))
{
return;
}
@@ -1252,7 +1205,8 @@
// Handle paging using skip (number of entries to skip
// from the start) and top (number of entries to
// display)
- if (entryCount <= skip || entryCount > skip + top)
+ if (entryCount <= delegatedQuery.skip ||
+ entryCount > delegatedQuery.skip + delegatedQuery.top)
{
continue;
}
@@ -1279,11 +1233,11 @@
}
}
asyncResp->res.jsonValue["Members@odata.count"] = entryCount;
- if (skip + top < entryCount)
+ if (delegatedQuery.skip + delegatedQuery.top < entryCount)
{
asyncResp->res.jsonValue["Members@odata.nextLink"] =
"/redfish/v1/Systems/system/LogServices/EventLog/Entries?$skip=" +
- std::to_string(skip + top);
+ std::to_string(delegatedQuery.skip + delegatedQuery.top);
}
});
}
@@ -1861,7 +1815,7 @@
inline bool
getHostLoggerEntries(std::vector<std::filesystem::path>& hostLoggerFiles,
- uint64_t& skip, uint64_t& top,
+ uint64_t skip, uint64_t top,
std::vector<std::string>& logEntries, size_t& logCount)
{
GzFileReader logFile;
@@ -1940,19 +1894,13 @@
const std::shared_ptr<
bmcweb::AsyncResp>&
asyncResp) {
- if (!redfish::setUpRedfishRoute(app, req, asyncResp->res))
- {
- return;
- }
- uint64_t skip = 0;
- uint64_t top = maxEntriesPerPage; // Show max 1000 entries by
- // default, allow range 1 to
- // 1000 entries per page.
- if (!getSkipParam(asyncResp, req, skip))
- {
- return;
- }
- if (!getTopParam(asyncResp, req, top))
+ query_param::QueryCapabilities capabilities = {
+ .canDelegateTop = true,
+ .canDelegateSkip = true,
+ };
+ query_param::Query delegatedQuery;
+ if (!redfish::setUpRedfishRouteWithDelegation(
+ app, req, asyncResp->res, delegatedQuery, capabilities))
{
return;
}
@@ -1978,8 +1926,8 @@
// This vector only store the entries we want to expose that
// control by skip and top.
std::vector<std::string> logEntries;
- if (!getHostLoggerEntries(hostLoggerFiles, skip, top, logEntries,
- logCount))
+ if (!getHostLoggerEntries(hostLoggerFiles, delegatedQuery.skip,
+ delegatedQuery.top, logEntries, logCount))
{
messages::internalError(asyncResp->res);
return;
@@ -1997,16 +1945,18 @@
{
logEntryArray.push_back({});
nlohmann::json& hostLogEntry = logEntryArray.back();
- fillHostLoggerEntryJson(std::to_string(skip + i),
- logEntries[i], hostLogEntry);
+ fillHostLoggerEntryJson(
+ std::to_string(delegatedQuery.skip + i), logEntries[i],
+ hostLogEntry);
}
asyncResp->res.jsonValue["Members@odata.count"] = logCount;
- if (skip + top < logCount)
+ if (delegatedQuery.skip + delegatedQuery.top < logCount)
{
asyncResp->res.jsonValue["Members@odata.nextLink"] =
"/redfish/v1/Systems/system/LogServices/HostLogger/Entries?$skip=" +
- std::to_string(skip + top);
+ std::to_string(delegatedQuery.skip +
+ delegatedQuery.top);
}
}
});
@@ -2219,18 +2169,13 @@
const std::shared_ptr<
bmcweb::AsyncResp>&
asyncResp) {
- if (!redfish::setUpRedfishRoute(app, req, asyncResp->res))
- {
- return;
- }
- static constexpr const long maxEntriesPerPage = 1000;
- uint64_t skip = 0;
- uint64_t top = maxEntriesPerPage; // Show max entries by default
- if (!getSkipParam(asyncResp, req, skip))
- {
- return;
- }
- if (!getTopParam(asyncResp, req, top))
+ query_param::QueryCapabilities capabilities = {
+ .canDelegateTop = true,
+ .canDelegateSkip = true,
+ };
+ query_param::Query delegatedQuery;
+ if (!redfish::setUpRedfishRouteWithDelegation(
+ app, req, asyncResp->res, delegatedQuery, capabilities))
{
return;
}
@@ -2268,7 +2213,8 @@
entryCount++;
// Handle paging using skip (number of entries to skip from
// the start) and top (number of entries to display)
- if (entryCount <= skip || entryCount > skip + top)
+ if (entryCount <= delegatedQuery.skip ||
+ entryCount > delegatedQuery.skip + delegatedQuery.top)
{
continue;
}
@@ -2294,11 +2240,11 @@
}
}
asyncResp->res.jsonValue["Members@odata.count"] = entryCount;
- if (skip + top < entryCount)
+ if (delegatedQuery.skip + delegatedQuery.top < entryCount)
{
asyncResp->res.jsonValue["Members@odata.nextLink"] =
"/redfish/v1/Managers/bmc/LogServices/Journal/Entries?$skip=" +
- std::to_string(skip + top);
+ std::to_string(delegatedQuery.skip + delegatedQuery.top);
}
});
}
@@ -3478,7 +3424,13 @@
.methods(boost::beast::http::verb::get)(
[&app](const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) {
- if (!redfish::setUpRedfishRoute(app, req, asyncResp->res))
+ query_param::QueryCapabilities capabilities = {
+ .canDelegateTop = true,
+ .canDelegateSkip = true,
+ };
+ query_param::Query delegatedQuery;
+ if (!redfish::setUpRedfishRouteWithDelegation(
+ app, req, asyncResp->res, delegatedQuery, capabilities))
{
return;
}
@@ -3492,17 +3444,8 @@
asyncResp->res.jsonValue["Members"] = nlohmann::json::array();
asyncResp->res.jsonValue["Members@odata.count"] = 0;
- uint64_t skip = 0;
- uint64_t top = maxEntriesPerPage; // Show max entries by default
- if (!getSkipParam(asyncResp, req, skip))
- {
- return;
- }
- if (!getTopParam(asyncResp, req, top))
- {
- return;
- }
- getCurrentBootNumber(asyncResp, skip, top);
+ getCurrentBootNumber(asyncResp, delegatedQuery.skip,
+ delegatedQuery.top);
});
}