Skip to content

Commit

Permalink
log_service: Fix behavior of getting single PostCode entry
Browse files Browse the repository at this point in the history 
Currently getting single PostCode entry returns a LogEntryCollection
with the specified LogEntry in its Members. Since Redfish Service
Validator does not follow the links in LogServiceCollection[1], such
unexpected behavior passes the validator. This commit makes it return
the LogEntry itself (or 404 Not Found) when requesting it.

Fixes Github issue #236 (#236)

[1] DMTF/Redfish-Service-Validator#519

Tested:
* Confirmed getting a valid PostCode entry now returns a LogEntry, and
  getting invalid entries like B0-1, B1-0, B1-999 or 123 (Not properly-
  formatted ID) responds with 404 Not Found.
* Get PostCode log entries collection still returns LogEntryCollection
  containing first 1000 PostCode entries by default.
* Redfish Service Validator passed.

Change-Id: Ice6b8742caea96ad3d436d57898202fe7362b150
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
  • Loading branch information
@jiaqingz-intel @edtanous
jiaqingz-intel authored and edtanous committed Dec 7, 2022
1 parent 2d6cb56 commit 6f284d2
Showing 1 changed file with 82 additions and 78 deletions.
160 changes: 82 additions & 78 deletions redfish-core/lib/log_services.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3559,7 +3559,47 @@ inline void requestRoutesPostCodesClear(App& app)
});
}

static void fillPostCodeEntry(
/**
* @brief Parse post code ID and get the current value and index value
* eg: postCodeID=B1-2, currentValue=1, index=2
*
* @param[in] postCodeID Post Code ID
* @param[out] currentValue Current value
* @param[out] index Index value
*
* @return bool true if the parsing is successful, false the parsing fails
*/
inline static bool parsePostCode(const std::string& postCodeID,
uint64_t& currentValue, uint16_t& index)
{
std::vector<std::string> split;
boost::algorithm::split(split, postCodeID, boost::is_any_of("-"));
if (split.size() != 2 || split[0].length() < 2 || split[0].front() != 'B')
{
return false;
}

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
const char* start = split[0].data() + 1;
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
const char* end = split[0].data() + split[0].size();
auto [ptrIndex, ecIndex] = std::from_chars(start, end, index);

if (ptrIndex != end || ecIndex != std::errc())
{
return false;
}

start = split[1].data();

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
end = split[1].data() + split[1].size();
auto [ptrValue, ecValue] = std::from_chars(start, end, currentValue);

return ptrValue == end && ecValue == std::errc();
}

static bool fillPostCodeEntry(
const std::shared_ptr<bmcweb::AsyncResp>& aResp,
const boost::container::flat_map<
uint64_t, std::tuple<uint64_t, std::vector<uint8_t>>>& postcode,
Expand All @@ -3571,8 +3611,6 @@ static void fillPostCodeEntry(
registries::getMessage("OpenBMC.0.2.BIOSPOSTCode");

uint64_t currentCodeIndex = 0;
nlohmann::json& logEntryArray = aResp->res.jsonValue["Members"];

uint64_t firstCodeTimeUs = 0;
for (const std::pair<uint64_t, std::tuple<uint64_t, std::vector<uint8_t>>>&
code : postcode)
Expand Down Expand Up @@ -3660,9 +3698,8 @@ static void fillPostCodeEntry(
severity = message->messageSeverity;
}

// add to AsyncResp
logEntryArray.push_back({});
nlohmann::json& bmcLogEntry = logEntryArray.back();
// Format entry
nlohmann::json::object_t bmcLogEntry;
bmcLogEntry["@odata.type"] = "#LogEntry.v1_9_0.LogEntry";
bmcLogEntry["@odata.id"] =
"/redfish/v1/Systems/system/LogServices/PostCodes/Entries/" +
Expand All @@ -3681,15 +3718,44 @@ static void fillPostCodeEntry(
"/redfish/v1/Systems/system/LogServices/PostCodes/Entries/" +
postcodeEntryID + "/attachment";
}

// codeIndex is only specified when querying single entry, return only
// that entry in this case
if (codeIndex != 0)
{
aResp->res.jsonValue.update(bmcLogEntry);
return true;
}

nlohmann::json& logEntryArray = aResp->res.jsonValue["Members"];
logEntryArray.push_back(std::move(bmcLogEntry));
}

// Return value is always false when querying multiple entries
return false;
}

static void getPostCodeForEntry(const std::shared_ptr<bmcweb::AsyncResp>& aResp,
const uint16_t bootIndex,
const uint64_t codeIndex)
const std::string& entryId)
{
uint16_t bootIndex = 0;
uint64_t codeIndex = 0;
if (!parsePostCode(entryId, codeIndex, bootIndex))
{
// Requested ID was not found
messages::resourceNotFound(aResp->res, "LogEntry", entryId);
return;
}

if (bootIndex == 0 || codeIndex == 0)
{
// 0 is an invalid index
messages::resourceNotFound(aResp->res, "LogEntry", entryId);
return;
}

crow::connections::systemBus->async_method_call(
[aResp, bootIndex,
[aResp, entryId, bootIndex,
codeIndex](const boost::system::error_code ec,
const boost::container::flat_map<
uint64_t, std::tuple<uint64_t, std::vector<uint8_t>>>&
Expand All @@ -3701,16 +3767,17 @@ static void getPostCodeForEntry(const std::shared_ptr<bmcweb::AsyncResp>& aResp,
return;
}

// skip the empty postcode boots
if (postcode.empty())
{
messages::resourceNotFound(aResp->res, "LogEntry", entryId);
return;
}

fillPostCodeEntry(aResp, postcode, bootIndex, codeIndex);

aResp->res.jsonValue["Members@odata.count"] =
aResp->res.jsonValue["Members"].size();
if (!fillPostCodeEntry(aResp, postcode, bootIndex, codeIndex))
{
messages::resourceNotFound(aResp->res, "LogEntry", entryId);
return;
}
},
"xyz.openbmc_project.State.Boot.PostCode0",
"/xyz/openbmc_project/State/Boot/PostCode0",
Expand Down Expand Up @@ -3839,46 +3906,6 @@ inline void requestRoutesPostCodesEntryCollection(App& app)
});
}

/**
* @brief Parse post code ID and get the current value and index value
* eg: postCodeID=B1-2, currentValue=1, index=2
*
* @param[in] postCodeID Post Code ID
* @param[out] currentValue Current value
* @param[out] index Index value
*
* @return bool true if the parsing is successful, false the parsing fails
*/
inline static bool parsePostCode(const std::string& postCodeID,
uint64_t& currentValue, uint16_t& index)
{
std::vector<std::string> split;
boost::algorithm::split(split, postCodeID, boost::is_any_of("-"));
if (split.size() != 2 || split[0].length() < 2 || split[0].front() != 'B')
{
return false;
}

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
const char* start = split[0].data() + 1;
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
const char* end = split[0].data() + split[0].size();
auto [ptrIndex, ecIndex] = std::from_chars(start, end, index);

if (ptrIndex != end || ecIndex != std::errc())
{
return false;
}

start = split[1].data();

// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
end = split[1].data() + split[1].size();
auto [ptrValue, ecValue] = std::from_chars(start, end, currentValue);

return ptrValue == end && ecValue == std::errc();
}

inline void requestRoutesPostCodesEntryAdditionalData(App& app)
{
BMCWEB_ROUTE(
Expand Down Expand Up @@ -3988,30 +4015,7 @@ inline void requestRoutesPostCodesEntry(App& app)
return;
}

uint16_t bootIndex = 0;
uint64_t codeIndex = 0;
if (!parsePostCode(targetID, codeIndex, bootIndex))
{
// Requested ID was not found
messages::resourceNotFound(asyncResp->res, "LogEntry", targetID);
return;
}
if (bootIndex == 0 || codeIndex == 0)
{
BMCWEB_LOG_DEBUG << "Get Post Code invalid entry string "
<< targetID;
}

asyncResp->res.jsonValue["@odata.type"] = "#LogEntry.v1_9_0.LogEntry";
asyncResp->res.jsonValue["@odata.id"] =
"/redfish/v1/Systems/system/LogServices/PostCodes/Entries";
asyncResp->res.jsonValue["Name"] = "BIOS POST Code Log Entries";
asyncResp->res.jsonValue["Description"] =
"Collection of POST Code Log Entries";
asyncResp->res.jsonValue["Members"] = nlohmann::json::array();
asyncResp->res.jsonValue["Members@odata.count"] = 0;

getPostCodeForEntry(asyncResp, bootIndex, codeIndex);
getPostCodeForEntry(asyncResp, targetID);
});
}

Expand Down

0 comments on commit 6f284d2

Please sign in to comment.