From 895feba5f2ddaf2a0117d3b68c67e97c9f5e4473 Mon Sep 17 00:00:00 2001
From: Joel Rosdahl <joel@rosdahl.net>
Date: Sun, 16 Jul 2023 17:36:58 +0200
Subject: [PATCH] chore: Improve util::format_argv_for_logging

---
 src/util/string.cpp           |  3 ++-
 src/util/string.hpp           |  3 ++-
 unittest/test_util_string.cpp | 32 ++++++++++++++++++++++++++++----
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/util/string.cpp b/src/util/string.cpp
index fb3cdea4a..a9d61d1c6 100644
--- a/src/util/string.cpp
+++ b/src/util/string.cpp
@@ -55,7 +55,8 @@ format_argv_for_logging(const char* const* argv)
     if (i != 0) {
       result += ' ';
     }
-    std::string arg(argv[i]);
+    std::string arg = replace_all(argv[i], "\\", "\\\\");
+    arg = replace_all(arg, "\"", "\\\"");
     if (arg.empty() || arg.find(' ') != std::string::npos) {
       arg = FMT("\"{}\"", arg);
     }
diff --git a/src/util/string.hpp b/src/util/string.hpp
index f5a9ebc03..684f059e7 100644
--- a/src/util/string.hpp
+++ b/src/util/string.hpp
@@ -45,7 +45,8 @@ enum class SizeUnitPrefixType { binary, decimal };
 bool ends_with(std::string_view string, std::string_view suffix);
 
 // Format `argv` as a simple string for logging purposes. That is, the result is
-// not intended to be machine parsable. `argv` must be terminated by a nullptr.
+// not intended to be easily machine parsable. `argv` must be terminated by a
+// nullptr.
 std::string format_argv_for_logging(const char* const* argv);
 
 // Format a hexadecimal string representing `data`. The returned string will be
diff --git a/unittest/test_util_string.cpp b/unittest/test_util_string.cpp
index 13b040dda..4b1a2cc4e 100644
--- a/unittest/test_util_string.cpp
+++ b/unittest/test_util_string.cpp
@@ -41,11 +41,35 @@ TEST_SUITE_BEGIN("util");
 
 TEST_CASE("util::format_argv_for_logging")
 {
-  const char* argv_0[] = {nullptr};
-  CHECK(util::format_argv_for_logging(argv_0) == "");
+  SUBCASE("nullptr")
+  {
+    const char* argv[] = {nullptr};
+    CHECK(util::format_argv_for_logging(argv) == "");
+  }
 
-  const char* argv_2[] = {"foo", "bar", nullptr};
-  CHECK(util::format_argv_for_logging(argv_2) == "foo bar");
+  SUBCASE("plain arguments")
+  {
+    const char* argv[] = {"foo", "bar", nullptr};
+    CHECK(util::format_argv_for_logging(argv) == "foo bar");
+  }
+
+  SUBCASE("argument with space")
+  {
+    const char* argv[] = {"foo bar", "fum", nullptr};
+    CHECK(util::format_argv_for_logging(argv) == "\"foo bar\" fum");
+  }
+
+  SUBCASE("argument with double quote")
+  {
+    const char* argv[] = {"foo\"bar", "fum", nullptr};
+    CHECK(util::format_argv_for_logging(argv) == "foo\\\"bar fum");
+  }
+
+  SUBCASE("argument with backslash")
+  {
+    const char* argv[] = {"foo\\bar", "fum", nullptr};
+    CHECK(util::format_argv_for_logging(argv) == "foo\\\\bar fum");
+  }
 }
 
 TEST_CASE("util::format_base16")
-- 
2.47.2