From e4fded2c484022738c2b4a37e762b24ae1d2f902 Mon Sep 17 00:00:00 2001
From: Ryan Prichard <rprichard@google.com>
Date: Mon, 9 Jul 2018 23:51:58 -0700
Subject: [PATCH] adb: fix escape_arg for multiple quotes

escape_arg reuses the same index for the source (s) and the destination
(result), so it breaks on strings containing more than one quote, e.g:

 * a'b'c    ==> 'a'\''b'c'
 * a'bcde'f ==> 'a'\''b'\'cde'f'

Also make the function more efficient by doing fewer string copies. This
code is based on the android::base::Split code.

Use EXPECT_EQ because the tests can keep going if one fails.

Bug: none
Test: adb_test --gtest_filter=adb_utils.escape_arg
Change-Id: I6ca6e905fa53cc61b9a87276cb7116a5df7e8017
---
 adb/adb_utils.cpp      | 20 ++++++++++--------
 adb/adb_utils_test.cpp | 48 ++++++++++++++++++++++++------------------
 2 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp
index b236fb39f..0c3327fc3 100644
--- a/adb/adb_utils.cpp
+++ b/adb/adb_utils.cpp
@@ -79,22 +79,24 @@ bool directory_exists(const std::string& path) {
 }
 
 std::string escape_arg(const std::string& s) {
-  std::string result = s;
-
   // Escape any ' in the string (before we single-quote the whole thing).
   // The correct way to do this for the shell is to replace ' with '\'' --- that is,
   // close the existing single-quoted string, escape a single single-quote, and start
   // a new single-quoted string. Like the C preprocessor, the shell will concatenate
   // these pieces into one string.
-  for (size_t i = 0; i < s.size(); ++i) {
-    if (s[i] == '\'') {
-      result.insert(i, "'\\'");
-      i += 2;
-    }
+
+  std::string result;
+  result.push_back('\'');
+
+  size_t base = 0;
+  while (true) {
+    size_t found = s.find('\'', base);
+    result.append(s, base, found - base);
+    if (found == s.npos) break;
+    result.append("'\\''");
+    base = found + 1;
   }
 
-  // Prefix and suffix the whole string with '.
-  result.insert(result.begin(), '\'');
   result.push_back('\'');
   return result;
 }
diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp
index e1b628707..341323fa9 100644
--- a/adb/adb_utils_test.cpp
+++ b/adb/adb_utils_test.cpp
@@ -82,30 +82,38 @@ TEST(adb_utils, directory_exists_win32_symlink_junction) {
 #endif
 
 TEST(adb_utils, escape_arg) {
-  ASSERT_EQ(R"('')", escape_arg(""));
+  EXPECT_EQ(R"('')", escape_arg(""));
 
-  ASSERT_EQ(R"('abc')", escape_arg("abc"));
+  EXPECT_EQ(R"('abc')", escape_arg("abc"));
 
-  ASSERT_EQ(R"(' abc')", escape_arg(" abc"));
-  ASSERT_EQ(R"(''\''abc')", escape_arg("'abc"));
-  ASSERT_EQ(R"('"abc')", escape_arg("\"abc"));
-  ASSERT_EQ(R"('\abc')", escape_arg("\\abc"));
-  ASSERT_EQ(R"('(abc')", escape_arg("(abc"));
-  ASSERT_EQ(R"(')abc')", escape_arg(")abc"));
+  auto wrap = [](const std::string& x) { return '\'' + x + '\''; };
+  const std::string q = R"('\'')";
+  EXPECT_EQ(wrap(q), escape_arg("'"));
+  EXPECT_EQ(wrap(q + q), escape_arg("''"));
+  EXPECT_EQ(wrap(q + "abc" + q), escape_arg("'abc'"));
+  EXPECT_EQ(wrap(q + "abc"), escape_arg("'abc"));
+  EXPECT_EQ(wrap("abc" + q), escape_arg("abc'"));
+  EXPECT_EQ(wrap("abc" + q + "def"), escape_arg("abc'def"));
+  EXPECT_EQ(wrap("a" + q + "b" + q + "c"), escape_arg("a'b'c"));
+  EXPECT_EQ(wrap("a" + q + "bcde" + q + "f"), escape_arg("a'bcde'f"));
 
-  ASSERT_EQ(R"('abc abc')", escape_arg("abc abc"));
-  ASSERT_EQ(R"('abc'\''abc')", escape_arg("abc'abc"));
-  ASSERT_EQ(R"('abc"abc')", escape_arg("abc\"abc"));
-  ASSERT_EQ(R"('abc\abc')", escape_arg("abc\\abc"));
-  ASSERT_EQ(R"('abc(abc')", escape_arg("abc(abc"));
-  ASSERT_EQ(R"('abc)abc')", escape_arg("abc)abc"));
+  EXPECT_EQ(R"(' abc')", escape_arg(" abc"));
+  EXPECT_EQ(R"('"abc')", escape_arg("\"abc"));
+  EXPECT_EQ(R"('\abc')", escape_arg("\\abc"));
+  EXPECT_EQ(R"('(abc')", escape_arg("(abc"));
+  EXPECT_EQ(R"(')abc')", escape_arg(")abc"));
 
-  ASSERT_EQ(R"('abc ')", escape_arg("abc "));
-  ASSERT_EQ(R"('abc'\''')", escape_arg("abc'"));
-  ASSERT_EQ(R"('abc"')", escape_arg("abc\""));
-  ASSERT_EQ(R"('abc\')", escape_arg("abc\\"));
-  ASSERT_EQ(R"('abc(')", escape_arg("abc("));
-  ASSERT_EQ(R"('abc)')", escape_arg("abc)"));
+  EXPECT_EQ(R"('abc abc')", escape_arg("abc abc"));
+  EXPECT_EQ(R"('abc"abc')", escape_arg("abc\"abc"));
+  EXPECT_EQ(R"('abc\abc')", escape_arg("abc\\abc"));
+  EXPECT_EQ(R"('abc(abc')", escape_arg("abc(abc"));
+  EXPECT_EQ(R"('abc)abc')", escape_arg("abc)abc"));
+
+  EXPECT_EQ(R"('abc ')", escape_arg("abc "));
+  EXPECT_EQ(R"('abc"')", escape_arg("abc\""));
+  EXPECT_EQ(R"('abc\')", escape_arg("abc\\"));
+  EXPECT_EQ(R"('abc(')", escape_arg("abc("));
+  EXPECT_EQ(R"('abc)')", escape_arg("abc)"));
 }
 
 void test_mkdirs(const std::string& basepath) {