From bb1d702e25f5f23e8d5a755295f2921caaea2abb Mon Sep 17 00:00:00 2001
From: Raphael Isemann <teemperor@gmail.com>
Date: Thu, 22 Oct 2020 13:50:22 +0200
Subject: [PATCH] [lldb][NFC] Make GetShellSafeArgument return std::string and
 unittest it.

---
 lldb/include/lldb/Utility/Args.h              |  5 ++---
 lldb/source/Host/common/ProcessLaunchInfo.cpp |  8 ++++----
 lldb/source/Utility/Args.cpp                  |  9 ++++-----
 lldb/unittests/Utility/ArgsTest.cpp           | 23 +++++++++++++++++++++++
 4 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Utility/Args.h b/lldb/include/lldb/Utility/Args.h
index 82e6d14..b93677b 100644
--- a/lldb/include/lldb/Utility/Args.h
+++ b/lldb/include/lldb/Utility/Args.h
@@ -263,9 +263,8 @@ public:
 
   static uint32_t StringToGenericRegister(llvm::StringRef s);
 
-  static const char *GetShellSafeArgument(const FileSpec &shell,
-                                          const char *unsafe_arg,
-                                          std::string &safe_arg);
+  static std::string GetShellSafeArgument(const FileSpec &shell,
+                                          llvm::StringRef unsafe_arg);
 
   // EncodeEscapeSequences will change the textual representation of common
   // escape sequences like "\n" (two characters) into a single '\n'. It does
diff --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp
index 851069d..01cf3d7 100644
--- a/lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -253,7 +253,6 @@ bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell(
       if (argv == nullptr || argv[0] == nullptr)
         return false;
       Args shell_arguments;
-      std::string safe_arg;
       shell_arguments.AppendArgument(shell_executable);
       const llvm::Triple &triple = GetArchitecture().GetTriple();
       if (triple.getOS() == llvm::Triple::Win32 &&
@@ -330,9 +329,10 @@ bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell(
           return false;
       } else {
         for (size_t i = 0; argv[i] != nullptr; ++i) {
-          const char *arg =
-              Args::GetShellSafeArgument(m_shell, argv[i], safe_arg);
-          shell_command.Printf(" %s", arg);
+          std::string safe_arg = Args::GetShellSafeArgument(m_shell, argv[i]);
+          // Add a space to separate this arg from the previous one.
+          shell_command.PutCString(" ");
+          shell_command.PutCString(safe_arg);
         }
       }
       shell_arguments.AppendArgument(shell_command.GetString());
diff --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp
index 2cbe727..152e73db 100644
--- a/lldb/source/Utility/Args.cpp
+++ b/lldb/source/Utility/Args.cpp
@@ -379,9 +379,8 @@ void Args::Clear() {
   m_argv.push_back(nullptr);
 }
 
-const char *Args::GetShellSafeArgument(const FileSpec &shell,
-                                       const char *unsafe_arg,
-                                       std::string &safe_arg) {
+std::string Args::GetShellSafeArgument(const FileSpec &shell,
+                                       llvm::StringRef unsafe_arg) {
   struct ShellDescriptor {
     ConstString m_basename;
     const char *m_escapables;
@@ -403,7 +402,7 @@ const char *Args::GetShellSafeArgument(const FileSpec &shell,
     }
   }
 
-  safe_arg.assign(unsafe_arg);
+  std::string safe_arg(unsafe_arg);
   size_t prev_pos = 0;
   while (prev_pos < safe_arg.size()) {
     // Escape spaces and quotes
@@ -414,7 +413,7 @@ const char *Args::GetShellSafeArgument(const FileSpec &shell,
     } else
       break;
   }
-  return safe_arg.c_str();
+  return safe_arg;
 }
 
 lldb::Encoding Args::StringToEncoding(llvm::StringRef s,
diff --git a/lldb/unittests/Utility/ArgsTest.cpp b/lldb/unittests/Utility/ArgsTest.cpp
index ed1235f..cd62c08 100644
--- a/lldb/unittests/Utility/ArgsTest.cpp
+++ b/lldb/unittests/Utility/ArgsTest.cpp
@@ -9,6 +9,7 @@
 #include "gtest/gtest.h"
 
 #include "lldb/Utility/Args.h"
+#include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StringList.h"
 
 #include <limits>
@@ -313,3 +314,25 @@ TEST(ArgsTest, Yaml) {
   EXPECT_EQ(entries[2].GetQuoteChar(), '"');
   EXPECT_EQ(entries[3].GetQuoteChar(), '\0');
 }
+
+TEST(ArgsTest, GetShellSafeArgument) {
+  // Try escaping with bash at start/middle/end of the argument.
+  FileSpec bash("/bin/bash", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "\"b"), "\\\"b");
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a\""), "a\\\"");
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a\"b"), "a\\\"b");
+
+  // String that doesn't need to be escaped
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a"), "a");
+
+  // Try escaping with tcsh and the tcsh-specific "$" escape.
+  FileSpec tcsh("/bin/tcsh", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(tcsh, "a$b"), "a\\$b");
+  // Bash however doesn't need escaping for "$".
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a$b"), "a$b");
+
+  // Try escaping with an unknown shell.
+  FileSpec unknown_shell("/bin/unknown_shell", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(unknown_shell, "a'b"), "a\\'b");
+  EXPECT_EQ(Args::GetShellSafeArgument(unknown_shell, "a\"b"), "a\\\"b");
+}
-- 
2.7.4