Merge #3117

3117: Fix newlines and empty strings which go through SSH r=townsend2010 a=luis4a0 This fixes #3116, the execution of strings containing quotes and newlines, like `multipass exec instance -- bash -c 'blah\nblah'`. This also fixes #3119, the execution of strings containing empty substrings delimited with single or double quotes. Co-authored-by: Luis Peñaranda <[email protected]>
canonical · Jun 15, 2023 · 596f104 · 596f104
2 parents 5522948 + 7e31a61
commit 596f104
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 5 deletions.
diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp
@@ -248,18 +248,35 @@ std::string mp::utils::escape_char(const std::string& in, char c)
 // Escape all characters which need to be escaped in the shell.
 std::string mp::utils::escape_for_shell(const std::string& in)
 {
+    // If the input string is empty, it means that the shell received an empty string enclosed in quotes and removed
+    // them. It must be quoted again for the shell to recognize it.
+    if (in.empty())
+    {
+        return "\'\'";
+    }
+
     std::string ret;
+
     std::back_insert_iterator<std::string> ret_insert = std::back_inserter(ret);
 
     for (char c : in)
     {
-        // If the character is in one of these code ranges, then it must be escaped.
-        if (c < 0x25 || c > 0x7a || (c > 0x25 && c < 0x2b) || (c > 0x5a && c < 0x5f) || 0x2c == c || 0x3b == c ||
-            0x3c == c || 0x3e == c || 0x3f == c || 0x60 == c)
+        if (0xa == c) // newline
+        {
+            *ret_insert++ = 0x5c; // backslash
+            *ret_insert++ = 0x20; // space
+        }
+        else
         {
-            *ret_insert++ = '\\';
+            // If the character is in one of these code ranges, then it must be escaped.
+            if (c < 0x25 || c > 0x7a || (c > 0x25 && c < 0x2b) || (c > 0x5a && c < 0x5f) || 0x2c == c || 0x3b == c ||
+                0x3c == c || 0x3e == c || 0x3f == c || 0x60 == c)
+            {
+                *ret_insert++ = 0x5c; // backslash
+            }
+
+            *ret_insert++ = c;
         }
-        *ret_insert++ = c;
     }
 
     return ret;

diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp
@@ -426,6 +426,20 @@ TEST(Utils, escape_for_shell_actually_escapes)
     EXPECT_THAT(res, ::testing::StrEq("I\\'ve\\ got\\ \\\"quotes\\\""));
 }
 
+TEST(Utils, escape_for_shell_replaces_newlines_with_spaces)
+{
+    std::string s{"I've got\nnewlines"};
+    auto res = mp::utils::escape_for_shell(s);
+    EXPECT_THAT(res, ::testing::StrEq("I\\'ve\\ got\\ newlines"));
+}
+
+TEST(Utils, escape_for_shell_quotes_empty_string)
+{
+    std::string s{""};
+    auto res = mp::utils::escape_for_shell(s);
+    EXPECT_THAT(res, ::testing::StrEq("''"));
+}
+
 TEST(Utils, try_action_actually_times_out)
 {
     bool on_timeout_called{false};