commit 8db46a0fb0984f807f7e3b5c611af1ed31b17557
author William A. Kennington III <wak@google.com> Wed May 05 17:53:35 2021 -0700
committer Patrick Williams <patrick@stwcx.xyz> Thu May 13 15:05:14 2021 +0000
tree 8525d2ebf775afe52fa3127824aafbe8edaa77d8
parent 285f78b142c83b849aff792c00eea5c5650ba17f

native_types: string_path_wrapper: Don't escape all strings

Not all strings require escaping when appending them to a path. This
allows us to now append `openbmc_project` to `/xyz` and have
expected behavior.

Change-Id: I49a2ee100455cff4067dcf3df61f3e145231eeba
Signed-off-by: William A. Kennington III <wak@google.com>

src/message/native_types.cpp

diff --git a/src/message/native_types.cpp b/src/message/native_types.cpp
index cd0f010..0a81a14 100644
--- a/src/message/native_types.cpp
+++ b/src/message/native_types.cpp
@@ -1,5 +1,6 @@
 #include <sdbusplus/message/native_types.hpp>
 
+#include <algorithm>
 #include <array>
 #include <cctype>
 
@@ -36,6 +37,11 @@
     return !std::isalnum(c);
 }
 
+inline bool pathRequiresEscape(char c)
+{
+    return pathShouldEscape(c) && c != '_';
+}
+
 inline void pathAppendEscape(std::string& s, char c)
 {
     s.append(1, '_');
@@ -117,8 +123,11 @@
     {
         str.append(1, '/');
     }
-    if (extId.empty())
+    if (extId.empty() ||
+        (!pathShouldEscape(extId[0]) &&
+         std::none_of(extId.begin() + 1, extId.end(), pathRequiresEscape)))
     {
+        str.append(extId);
         return *this;
     }
     pathAppendEscape(str, extId[0]);

test/message/types.cpp

diff --git a/test/message/types.cpp b/test/message/types.cpp
index 5e4e982..d666008 100644
--- a/test/message/types.cpp
+++ b/test/message/types.cpp
@@ -76,11 +76,11 @@
 TEST(MessageTypes, ObjectPathOperatorSlash)
 {
     EXPECT_EQ(sdbusplus::message::object_path("/") / "abc",
-              sdbusplus::message::object_path("/_61bc"));
+              sdbusplus::message::object_path("/abc"));
     EXPECT_EQ(sdbusplus::message::object_path("/") / "abc",
-              sdbusplus::message::object_path("/_61bc"));
+              sdbusplus::message::object_path("/abc"));
     EXPECT_EQ(sdbusplus::message::object_path("/abc") / "def",
-              sdbusplus::message::object_path("/abc/_64ef"));
+              sdbusplus::message::object_path("/abc/def"));
     EXPECT_EQ(sdbusplus::message::object_path("/abc") / "-",
               sdbusplus::message::object_path("/abc/_2d"));
     EXPECT_EQ(sdbusplus::message::object_path("/abc") / " ",
@@ -88,7 +88,7 @@
     EXPECT_EQ(sdbusplus::message::object_path("/abc") / "/",
               sdbusplus::message::object_path("/abc/_2f"));
     EXPECT_EQ(sdbusplus::message::object_path("/abc") / "ab_cd",
-              sdbusplus::message::object_path("/abc/_61b_5fcd"));
+              sdbusplus::message::object_path("/abc/ab_cd"));
     EXPECT_EQ(sdbusplus::message::object_path("/abc") / "_ab_cd",
               sdbusplus::message::object_path("/abc/_5fab_5fcd"));
     EXPECT_EQ(sdbusplus::message::object_path("/abc") / "ab-c_d",
@@ -96,14 +96,14 @@
 
     // Test the std::string overload.  This is largely just for coverage
     EXPECT_EQ(sdbusplus::message::object_path("/") / std::string("abc"),
-              sdbusplus::message::object_path("/_61bc"));
+              sdbusplus::message::object_path("/abc"));
 }
 
 TEST(MessageTypes, ObjectPathOperatorSlashEqual)
 {
     sdbusplus::message::object_path path("/");
     path /= "abc";
-    EXPECT_EQ(path, sdbusplus::message::object_path("/_61bc"));
+    EXPECT_EQ(path, sdbusplus::message::object_path("/abc"));
 
     sdbusplus::message::object_path path2("/");
     path2 /= std::string("d-ef");