Add url type safety to message registry
There are a number of places where we use message registry messages
incorrectly. This patchset attempts to fix them, and invoke some type
safety when they're used such that they're more obvious to use.
Namely, it changes a number of the message registry methods to accept a
boost::urls::url_view for its argument instead of a const std::string&.
This forces the calling code to correctly encode a URL to use the
method, which should make it obvious that it's not for an ID, a property
name, or anything else. In the course of doing this, several places
were found to be using the first argument incorrectly.
Tested:
curl --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Chassis/foobar
Returns:
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type #Chassis.v1_16_0.Chassis named foobar was not found.",
"MessageArgs": [
"#Chassis.v1_16_0.Chassis",
"foobar"
],
"MessageId": "Base.1.8.1.ResourceNotFound",
"MessageSeverity": "Critical",
"Resolution": "Provide a valid resource identifier and resubmit the request."
}
],
"code": "Base.1.8.1.ResourceNotFound",
"message": "The requested resource of type #Chassis.v1_16_0.Chassis named foobar was not found."
}
Identically to previously.
Also tested with IDs that contained % encoded characters, like
foobar%10, which gave the same result.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Icbb3bce5d190a260610087c9ef35e7becc5a50c7
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index 94de5a9..154de95 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -328,7 +328,8 @@
indexStr.data(), indexStr.data() + indexStr.size(), index);
if (ec != std::errc())
{
- messages::resourceMissingAtURI(asyncResp->res, entryID);
+ messages::resourceMissingAtURI(
+ asyncResp->res, crow::utility::urlFromPieces(entryID));
return false;
}
}
@@ -337,7 +338,8 @@
std::from_chars(tsStr.data(), tsStr.data() + tsStr.size(), timestamp);
if (ec != std::errc())
{
- messages::resourceMissingAtURI(asyncResp->res, entryID);
+ messages::resourceMissingAtURI(asyncResp->res,
+ crow::utility::urlFromPieces(entryID));
return false;
}
return true;
@@ -812,8 +814,7 @@
(*diagnosticDataType != "OEM"))
{
BMCWEB_LOG_ERROR << "Wrong parameter values passed";
- messages::invalidObject(asyncResp->res,
- "System Dump creation parameters");
+ messages::internalError(asyncResp->res);
return;
}
}
@@ -831,8 +832,7 @@
{
BMCWEB_LOG_ERROR
<< "Wrong parameter value passed for 'DiagnosticDataType'";
- messages::invalidObject(asyncResp->res,
- "BMC Dump creation parameters");
+ messages::internalError(asyncResp->res);
return;
}
}
@@ -1332,7 +1332,8 @@
}
}
// Requested ID was not found
- messages::resourceMissingAtURI(asyncResp->res, targetID);
+ messages::resourceMissingAtURI(
+ asyncResp->res, crow::utility::urlFromPieces(targetID));
});
}
@@ -1976,7 +1977,7 @@
app, "/redfish/v1/Systems/system/LogServices/HostLogger/Entries/<str>/")
.privileges(redfish::privileges::getLogEntry)
.methods(boost::beast::http::verb::get)(
- [](const crow::Request&,
+ [](const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::string& param) {
const std::string& targetID = param;
@@ -1989,12 +1990,12 @@
auto [ptr, ec] = std::from_chars(targetID.data(), end, idInt);
if (ec == std::errc::invalid_argument)
{
- messages::resourceMissingAtURI(asyncResp->res, targetID);
+ messages::resourceMissingAtURI(asyncResp->res, req.urlView);
return;
}
if (ec == std::errc::result_out_of_range)
{
- messages::resourceMissingAtURI(asyncResp->res, targetID);
+ messages::resourceMissingAtURI(asyncResp->res, req.urlView);
return;
}
@@ -2026,7 +2027,7 @@
}
// Requested ID was not found
- messages::resourceMissingAtURI(asyncResp->res, targetID);
+ messages::resourceMissingAtURI(asyncResp->res, req.urlView);
});
}
@@ -2253,7 +2254,7 @@
"/redfish/v1/Managers/bmc/LogServices/Journal/Entries/<str>/")
.privileges(redfish::privileges::getLogEntry)
.methods(boost::beast::http::verb::get)(
- [](const crow::Request&,
+ [](const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::string& entryID) {
// Convert the unique ID back to a timestamp to find the entry
@@ -2304,7 +2305,7 @@
// Confirm that the entry ID matches what was requested
if (idStr != entryID)
{
- messages::resourceMissingAtURI(asyncResp->res, entryID);
+ messages::resourceMissingAtURI(asyncResp->res, req.urlView);
return;
}
@@ -2642,7 +2643,8 @@
if (filename.empty() || timestamp.empty())
{
- messages::resourceMissingAtURI(asyncResp->res, logID);
+ messages::resourceMissingAtURI(
+ asyncResp->res, crow::utility::urlFromPieces(logID));
return;
}
@@ -2774,11 +2776,12 @@
"/redfish/v1/Systems/system/LogServices/Crashdump/Entries/<str>/<str>/")
.privileges(redfish::privileges::getLogEntry)
.methods(boost::beast::http::verb::get)(
- [](const crow::Request&,
+ [](const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::string& logID, const std::string& fileName) {
auto getStoredLogCallback =
- [asyncResp, logID, fileName](
+ [asyncResp, logID, fileName,
+ url(boost::urls::url(req.urlView))](
const boost::system::error_code ec,
const std::vector<std::pair<
std::string, dbus::utility::DbusVariantType>>&
@@ -2801,23 +2804,20 @@
if (dbusFilename.empty() || dbusTimestamp.empty() ||
dbusFilepath.empty())
{
- messages::resourceMissingAtURI(asyncResp->res,
- fileName);
+ messages::resourceMissingAtURI(asyncResp->res, url);
return;
}
// Verify the file name parameter is correct
if (fileName != dbusFilename)
{
- messages::resourceMissingAtURI(asyncResp->res,
- fileName);
+ messages::resourceMissingAtURI(asyncResp->res, url);
return;
}
if (!std::filesystem::exists(dbusFilepath))
{
- messages::resourceMissingAtURI(asyncResp->res,
- fileName);
+ messages::resourceMissingAtURI(asyncResp->res, url);
return;
}
std::ifstream ifs(dbusFilepath,
@@ -3456,7 +3456,7 @@
app, "/redfish/v1/Systems/system/LogServices/PostCodes/Entries/<str>/")
.privileges(redfish::privileges::getLogEntry)
.methods(boost::beast::http::verb::get)(
- [](const crow::Request&,
+ [](const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::string& targetID) {
uint16_t bootIndex = 0;
@@ -3464,7 +3464,7 @@
if (!parsePostCode(targetID, codeIndex, bootIndex))
{
// Requested ID was not found
- messages::resourceMissingAtURI(asyncResp->res, targetID);
+ messages::resourceMissingAtURI(asyncResp->res, req.urlView);
return;
}
if (bootIndex == 0 || codeIndex == 0)
diff --git a/redfish-core/lib/managers.hpp b/redfish-core/lib/managers.hpp
index b3796a6..a5e8c23 100644
--- a/redfish-core/lib/managers.hpp
+++ b/redfish-core/lib/managers.hpp
@@ -802,7 +802,9 @@
if (managedItem == nullptr)
{
BMCWEB_LOG_ERROR << "Failed to get chassis from config patch";
- messages::invalidObject(response->res, it.key());
+ messages::invalidObject(response->res,
+ crow::utility::urlFromPieces(
+ "redfish", "v1", "Chassis", chassis));
return CreatePIDRet::fail;
}
}
@@ -916,7 +918,9 @@
findChassis(managedObj, zonesStr[0], chassis) == nullptr)
{
BMCWEB_LOG_ERROR << "Failed to get chassis from config patch";
- messages::invalidObject(response->res, it.key());
+ messages::invalidObject(
+ response->res, crow::utility::urlFromPieces(
+ "redfish", "v1", "Chassis", chassis));
return CreatePIDRet::fail;
}
@@ -978,7 +982,8 @@
{
BMCWEB_LOG_ERROR << "Invalid setpointoffset "
<< *setpointOffset;
- messages::invalidObject(response->res, it.key());
+ messages::propertyValueNotInList(response->res, it.key(),
+ "SetPointOffset");
return CreatePIDRet::fail;
}
}
@@ -1033,7 +1038,9 @@
if (!dbus::utility::getNthStringFromPath(chassisId, 3, chassis))
{
BMCWEB_LOG_ERROR << "Got invalid path " << chassisId;
- messages::invalidObject(response->res, chassisId);
+ messages::invalidObject(
+ response->res, crow::utility::urlFromPieces(
+ "redfish", "v1", "Chassis", chassisId));
return CreatePIDRet::fail;
}
}
@@ -1081,7 +1088,9 @@
findChassis(managedObj, zonesStrs[0], chassis) == nullptr)
{
BMCWEB_LOG_ERROR << "Failed to get chassis from config patch";
- messages::invalidObject(response->res, it.key());
+ messages::invalidObject(
+ response->res, crow::utility::urlFromPieces(
+ "redfish", "v1", "Chassis", chassis));
return CreatePIDRet::fail;
}
output["Zones"] = std::move(zonesStrs);
@@ -1592,7 +1601,8 @@
if (createNewObject && it.value() == nullptr)
{
// can't delete a non-existent object
- messages::invalidObject(response->res, name);
+ messages::propertyValueNotInList(response->res, it.value(),
+ name);
continue;
}
@@ -1655,7 +1665,7 @@
if (chassis.empty())
{
BMCWEB_LOG_ERROR << "Failed to get chassis from config";
- messages::invalidObject(response->res, name);
+ messages::internalError(response->res);
return;
}
@@ -1673,7 +1683,9 @@
{
BMCWEB_LOG_ERROR << "Failed to find chassis on dbus";
messages::resourceMissingAtURI(
- response->res, "/redfish/v1/Chassis/" + chassis);
+ response->res,
+ crow::utility::urlFromPieces("redfish", "v1",
+ "Chassis", chassis));
return;
}
diff --git a/redfish-core/lib/redfish_sessions.hpp b/redfish-core/lib/redfish_sessions.hpp
index 1568c00..a66f460 100644
--- a/redfish-core/lib/redfish_sessions.hpp
+++ b/redfish-core/lib/redfish_sessions.hpp
@@ -19,6 +19,7 @@
#include "persistent_data.hpp"
#include <app.hpp>
+#include <http/utility.hpp>
#include <registries/privilege_registry.hpp>
namespace redfish
@@ -175,7 +176,7 @@
if ((pamrc != PAM_SUCCESS) && !isConfigureSelfOnly)
{
messages::resourceAtUriUnauthorized(
- asyncResp->res, std::string(req.url),
+ asyncResp->res, req.urlView,
"Invalid username or password");
return;
}
@@ -212,8 +213,9 @@
if (session->isConfigureSelfOnly)
{
messages::passwordChangeRequired(
- asyncResp->res, "/redfish/v1/AccountService/Accounts/" +
- session->username);
+ asyncResp->res, crow::utility::urlFromPieces(
+ "redfish", "v1", "AccountService",
+ "Accounts", req.session->username));
}
fillSessionObject(asyncResp->res, *session);
diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp
index 1288815..6c61e40 100644
--- a/redfish-core/lib/update_service.hpp
+++ b/redfish-core/lib/update_service.hpp
@@ -931,8 +931,9 @@
<< "Input swID " + *swId + " not found!";
messages::resourceMissingAtURI(
asyncResp->res,
- "/redfish/v1/UpdateService/FirmwareInventory/" +
- *swId);
+ crow::utility::urlFromPieces(
+ "redfish", "v1", "UpdateService",
+ "FirmwareInventory", *swId));
return;
}
asyncResp->res.jsonValue["@odata.type"] =
diff --git a/redfish-core/lib/virtual_media.hpp b/redfish-core/lib/virtual_media.hpp
index 049cc56..b0e3b38 100644
--- a/redfish-core/lib/virtual_media.hpp
+++ b/redfish-core/lib/virtual_media.hpp
@@ -306,16 +306,9 @@
*
*/
inline std::optional<TransferProtocol>
- getTransferProtocolFromUri(const std::string& imageUri)
+ getTransferProtocolFromUri(const boost::urls::url_view& imageUri)
{
- boost::urls::result<boost::urls::url_view> url =
- boost::urls::parse_uri(boost::string_view(imageUri));
- if (!url)
- {
- return {};
- }
-
- boost::string_view scheme = url->scheme();
+ boost::string_view scheme = imageUri.scheme();
if (scheme == "smb")
{
return TransferProtocol::smb;
@@ -421,9 +414,15 @@
return false;
}
-
+ boost::urls::result<boost::urls::url_view> url =
+ boost::urls::parse_uri(boost::string_view(imageUrl));
+ if (!url)
+ {
+ messages::resourceAtUriInUnknownFormat(res, *url);
+ return {};
+ }
std::optional<TransferProtocol> uriTransferProtocolType =
- getTransferProtocolFromUri(imageUrl);
+ getTransferProtocolFromUri(*url);
std::optional<TransferProtocol> paramTransferProtocolType =
getTransferProtocolFromParam(transferProtocolType);
@@ -435,7 +434,7 @@
"contain specified protocol type from list: "
"(smb, https).";
- messages::resourceAtUriInUnknownFormat(res, imageUrl);
+ messages::resourceAtUriInUnknownFormat(res, *url);
return false;
}
@@ -460,7 +459,7 @@
"contain specified protocol type or param "
"TransferProtocolType must be provided.";
- messages::resourceAtUriInUnknownFormat(res, imageUrl);
+ messages::resourceAtUriInUnknownFormat(res, *url);
return false;
}