Refactor tftp parser
This function in the next patch will be used for more than just TFTP, so
rename it to match intent, and refactor to use non-TFTP specific types.
Tested: Rename only. Need help on TFTP setups if we need it.
Change-Id: Ifc7485aa60ec53407c38b3d1bec530bdacf50075
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp
index a735567..df4043f 100644
--- a/redfish-core/lib/update_service.hpp
+++ b/redfish-core/lib/update_service.hpp
@@ -19,6 +19,7 @@
#include "app.hpp"
#include "dbus_utility.hpp"
+#include "generated/enums/update_service.hpp"
#include "multipart_parser.hpp"
#include "ossl_random.hpp"
#include "query.hpp"
@@ -441,16 +442,10 @@
std::bind_front(afterUpdateErrorMatcher, asyncResp, url));
}
-struct TftpUrl
-{
- std::string fwFile;
- std::string tftpServer;
-};
-
-inline std::optional<TftpUrl>
- parseTftpUrl(std::string imageURI,
- std::optional<std::string> transferProtocol,
- crow::Response& res)
+inline std::optional<boost::urls::url>
+ parseSimpleUpdateUrl(std::string imageURI,
+ std::optional<std::string> transferProtocol,
+ crow::Response& res)
{
if (imageURI.find("://") == std::string::npos)
{
@@ -467,7 +462,11 @@
return std::nullopt;
}
// OpenBMC currently only supports TFTP
- if (*transferProtocol != "TFTP")
+ if (*transferProtocol == "TFTP")
+ {
+ imageURI = "tftp://" + imageURI;
+ }
+ else
{
messages::actionParameterNotSupported(res, "TransferProtocol",
*transferProtocol);
@@ -475,7 +474,6 @@
*transferProtocol);
return std::nullopt;
}
- imageURI = "tftp://" + imageURI;
}
boost::system::result<boost::urls::url> url =
@@ -489,32 +487,52 @@
}
url->normalize();
- if (url->scheme() != "tftp")
+ if (url->scheme() == "tftp")
+ {
+ if (url->encoded_path().size() < 2)
+ {
+ messages::actionParameterNotSupported(res, "ImageURI",
+ url->buffer());
+ return std::nullopt;
+ }
+ }
+ else
{
messages::actionParameterNotSupported(res, "ImageURI", imageURI);
return std::nullopt;
}
- std::string path(url->encoded_path());
- if (path.size() < 2)
+
+ if (url->encoded_path().empty())
{
- messages::actionParameterNotSupported(res, "ImageURI", imageURI);
+ messages::actionParameterValueTypeError(res, imageURI, "ImageURI",
+ "UpdateService.SimpleUpdate");
return std::nullopt;
}
- path.erase(0, 1);
- std::string host(url->encoded_host_and_port());
- return TftpUrl{path, host};
+
+ return *url;
}
inline void doTftpUpdate(const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const TftpUrl& tftpUrl)
+ const boost::urls::url_view_base& url)
{
#ifndef BMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE
messages::actionParameterNotSupported(asyncResp->res, "ImageURI",
- tftpUrl.tftpServer);
+ url.buffer());
return;
#endif
- BMCWEB_LOG_DEBUG("Server: {} File: {}", tftpUrl.tftpServer, tftpUrl.fwFile);
+
+ std::string path(url.encoded_path());
+ if (path.size() < 2)
+ {
+ messages::actionParameterNotSupported(asyncResp->res, "ImageURI",
+ url.buffer());
+ return;
+ }
+ // TFTP expects a path without a /
+ path.erase(0, 1);
+ std::string host(url.encoded_host_and_port());
+ BMCWEB_LOG_DEBUG("Server: {} File: {}", host, path);
// Setup callback for when new software detected
// Give TFTP 10 minutes to complete
@@ -545,7 +563,7 @@
},
"xyz.openbmc_project.Software.Download",
"/xyz/openbmc_project/software", "xyz.openbmc_project.Common.TFTP",
- "DownloadViaTFTP", tftpUrl.fwFile, tftpUrl.tftpServer);
+ "DownloadViaTFTP", path, host);
}
inline void handleUpdateServiceSimpleUpdateAction(
@@ -575,15 +593,22 @@
return;
}
- std::optional<TftpUrl> ret = parseTftpUrl(imageURI, transferProtocol,
- asyncResp->res);
- if (!ret)
+ std::optional<boost::urls::url> url =
+ parseSimpleUpdateUrl(imageURI, transferProtocol, asyncResp->res);
+ if (!url)
{
return;
}
-
- BMCWEB_LOG_DEBUG("Server: {} File: {}", ret->tftpServer, ret->fwFile);
- doTftpUpdate(req, asyncResp, *ret);
+ if (url->scheme() == "tftp")
+ {
+ doTftpUpdate(req, asyncResp, *url);
+ }
+ else
+ {
+ messages::actionParameterNotSupported(asyncResp->res, "ImageURI",
+ url->buffer());
+ return;
+ }
BMCWEB_LOG_DEBUG("Exit UpdateService.SimpleUpdate doPost");
}
@@ -805,15 +830,21 @@
asyncResp->res.jsonValue["MaxImageSizeBytes"] = bmcwebHttpReqBodyLimitMb *
1024 * 1024;
-#ifdef BMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE
// Update Actions object.
nlohmann::json& updateSvcSimpleUpdate =
asyncResp->res.jsonValue["Actions"]["#UpdateService.SimpleUpdate"];
updateSvcSimpleUpdate["target"] =
"/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate";
- updateSvcSimpleUpdate["TransferProtocol@Redfish.AllowableValues"] = {
- "TFTP"};
+
+ nlohmann::json::array_t allowed;
+
+#ifdef BMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE
+ allowed.emplace_back(update_service::TransferProtocolType::TFTP);
#endif
+
+ updateSvcSimpleUpdate["TransferProtocol@Redfish.AllowableValues"] =
+ std::move(allowed);
+
// Get the current ApplyTime value
sdbusplus::asio::getProperty<std::string>(
*crow::connections::systemBus, "xyz.openbmc_project.Settings",
diff --git a/test/redfish-core/lib/update_service_test.cpp b/test/redfish-core/lib/update_service_test.cpp
index d56d709..5403131 100644
--- a/test/redfish-core/lib/update_service_test.cpp
+++ b/test/redfish-core/lib/update_service_test.cpp
@@ -16,38 +16,42 @@
crow::Response res;
{
// No protocol, schema on url
- std::optional<TftpUrl> ret = parseTftpUrl("tftp://1.1.1.1/path",
- std::nullopt, res);
+ std::optional<boost::urls::url> ret =
+ parseSimpleUpdateUrl("tftp://1.1.1.1/path", std::nullopt, res);
ASSERT_TRUE(ret);
if (!ret)
{
return;
}
- EXPECT_EQ(ret->tftpServer, "1.1.1.1");
- EXPECT_EQ(ret->fwFile, "path");
+ EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1");
+ EXPECT_EQ(ret->encoded_path(), "/path");
+ EXPECT_EQ(ret->scheme(), "tftp");
}
{
// Protocol, no schema on url
- std::optional<TftpUrl> ret = parseTftpUrl("1.1.1.1/path", "TFTP", res);
+ std::optional<boost::urls::url> ret =
+ parseSimpleUpdateUrl("1.1.1.1/path", "TFTP", res);
ASSERT_TRUE(ret);
if (!ret)
{
return;
}
- EXPECT_EQ(ret->tftpServer, "1.1.1.1");
- EXPECT_EQ(ret->fwFile, "path");
+ EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1");
+ EXPECT_EQ(ret->encoded_path(), "/path");
+ EXPECT_EQ(ret->scheme(), "tftp");
}
{
// Both protocl and schema on url
- std::optional<TftpUrl> ret = parseTftpUrl("tftp://1.1.1.1/path", "TFTP",
- res);
+ std::optional<boost::urls::url> ret =
+ parseSimpleUpdateUrl("tftp://1.1.1.1/path", "TFTP", res);
ASSERT_TRUE(ret);
if (!ret)
{
return;
}
- EXPECT_EQ(ret->tftpServer, "1.1.1.1");
- EXPECT_EQ(ret->fwFile, "path");
+ EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1");
+ EXPECT_EQ(ret->encoded_path(), "/path");
+ EXPECT_EQ(ret->scheme(), "tftp");
}
}
@@ -55,16 +59,19 @@
{
crow::Response res;
// No protocol, no schema
- ASSERT_EQ(parseTftpUrl("1.1.1.1/path", std::nullopt, res), std::nullopt);
+ ASSERT_EQ(parseSimpleUpdateUrl("1.1.1.1/path", std::nullopt, res),
+ std::nullopt);
// No host
- ASSERT_EQ(parseTftpUrl("/path", "TFTP", res), std::nullopt);
+ ASSERT_EQ(parseSimpleUpdateUrl("/path", "TFTP", res), std::nullopt);
// No host
- ASSERT_EQ(parseTftpUrl("path", "TFTP", res), std::nullopt);
+ ASSERT_EQ(parseSimpleUpdateUrl("path", "TFTP", res), std::nullopt);
// No path
- ASSERT_EQ(parseTftpUrl("tftp://1.1.1.1", "TFTP", res), std::nullopt);
- ASSERT_EQ(parseTftpUrl("tftp://1.1.1.1/", "TFTP", res), std::nullopt);
+ ASSERT_EQ(parseSimpleUpdateUrl("tftp://1.1.1.1", "TFTP", res),
+ std::nullopt);
+ ASSERT_EQ(parseSimpleUpdateUrl("tftp://1.1.1.1/", "TFTP", res),
+ std::nullopt);
}
} // namespace
} // namespace redfish