commit 482c45a52e221882be5142a873cd828c380d7e54
author Josh Lehan <krellan@google.com> Tue Mar 29 17:10:44 2022 -0700
committer Ed Tanous <ed@tanous.net> Thu May 12 23:23:34 2022 +0000
tree a6451f283092a705f0cc201e6ed620a1807c5314
parent 1476687deb1697d865b20458a0097c9ab5fd44e2

Separate validFilename into a separate function

This will generalize it and make it callable from other places

Tested: Added test cases, they pass

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: I8df30d6fe6753a2454d7051cc2d8813ddbf14bad

include/openbmc_dbus_rest.hpp

diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index 75e31f1..272776c 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -51,6 +51,13 @@
 const constexpr char* forbiddenResDesc =
     "The specified resource cannot be created";
 
+inline bool validateFilename(const std::string& filename)
+{
+    std::regex validFilename(R"(^[\w\- ]+(\.?[\w\- ]*)$)");
+
+    return std::regex_match(filename, validFilename);
+}
+
 inline void setErrorResponse(crow::Response& res,
                              boost::beast::http::status result,
                              const std::string& desc,
@@ -2538,8 +2545,7 @@
             [](const crow::Request&,
                const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
                const std::string& dumpId) {
-                std::regex validFilename(R"(^[\w\- ]+(\.?[\w\- ]*)$)");
-                if (!std::regex_match(dumpId, validFilename))
+                if (!validateFilename(dumpId))
                 {
                     asyncResp->res.result(
                         boost::beast::http::status::bad_request);

include/ut/openbmc_dbus_rest_test.cpp

diff --git a/include/ut/openbmc_dbus_rest_test.cpp b/include/ut/openbmc_dbus_rest_test.cpp
new file mode 100644
index 0000000..e671e47
--- /dev/null
+++ b/include/ut/openbmc_dbus_rest_test.cpp
@@ -0,0 +1,35 @@
+#include "include/openbmc_dbus_rest.hpp"
+
+#include "gtest/gtest.h"
+
+// Also see redfish-core/ut/configfile_test.cpp
+TEST(OpenbmcDbusRestTest, ValidFilenameGood)
+{
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("GoodConfigFile"));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("_Underlines_"));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("8675309"));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("-Dashes-"));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("With Spaces"));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("One.Dot"));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("trailingdot."));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename("-_ o _-"));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename(" "));
+    EXPECT_TRUE(crow::openbmc_mapper::validateFilename(" ."));
+}
+
+// There is no length test yet because validateFilename() does not care yet
+TEST(OpenbmcDbusRestTest, ValidFilenameBad)
+{
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename(""));
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename("Bad@file"));
+    EXPECT_FALSE(
+        crow::openbmc_mapper::validateFilename("/../../../../../etc/badpath"));
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename("/../../etc/badpath"));
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename("/mydir/configFile"));
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename("/"));
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename(".leadingdot"));
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename("Two..Dots"));
+    EXPECT_FALSE(
+        crow::openbmc_mapper::validateFilename("../../../../../../etc/shadow"));
+    EXPECT_FALSE(crow::openbmc_mapper::validateFilename("."));
+}

meson.build

diff --git a/meson.build b/meson.build
index e8897b3..0ab48ff 100644
--- a/meson.build
+++ b/meson.build
@@ -383,6 +383,7 @@
   'include/ut/http_utility_test.cpp',
   'include/ut/human_sort_test.cpp',
   'include/ut/multipart_test.cpp',
+  'include/ut/openbmc_dbus_rest_test.cpp',
   'redfish-core/include/utils/query_param_test.cpp',
   'redfish-core/lib/ut/service_root_test.cpp',
   'redfish-core/ut/configfile_test.cpp',