From ddb5ed7e1f4ad3c47e951a17fe24f98be0c949c7 Mon Sep 17 00:00:00 2001
From: Jooyung Han <jooyung@google.com>
Date: Mon, 8 Mar 2021 20:42:32 +0900
Subject: [PATCH] Add ShellEscapeIncludingSpaces(string)

ShellEscape(string) doesn't escape a string with spaces like "arg1
arg2". However, when we want to escape a string to use it as an
argument, then strings with spaces should be escaped.

For example,

  "command " + ShellEscapeIncludingSpaces("a b")

becomes "command 'a b'" so that "command" will get "a b" as a single
argument.

Bug: 182092664
Test: Added tests to escape_test.go
Change-Id: I8f88c18bc4f9f7aacfe9e701b8f0876dd8b9a8c3
---
 proptools/escape.go      | 50 +++++++++++++++++++++++++---------------
 proptools/escape_test.go | 43 ++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 18 deletions(-)

diff --git a/proptools/escape.go b/proptools/escape.go
index e7f0456..b8790b5 100644
--- a/proptools/escape.go
+++ b/proptools/escape.go
@@ -56,30 +56,44 @@ func ShellEscapeList(slice []string) []string {
 
 }
 
-// ShellEscapeList takes string that may contain characters that are meaningful to bash and
+func shellUnsafeChar(r rune) bool {
+	switch {
+	case 'A' <= r && r <= 'Z',
+		'a' <= r && r <= 'z',
+		'0' <= r && r <= '9',
+		r == '_',
+		r == '+',
+		r == '-',
+		r == '=',
+		r == '.',
+		r == ',',
+		r == '/':
+		return false
+	default:
+		return true
+	}
+}
+
+// ShellEscape takes string that may contain characters that are meaningful to bash and
 // escapes it if necessary by wrapping it in single quotes, and replacing internal single quotes with
 // '\'' (one single quote to end the quoting, a shell-escaped single quote to insert a real single
 // quote, and then a single quote to restarting quoting.
 func ShellEscape(s string) string {
-	shellUnsafeChar := func(r rune) bool {
-		switch {
-		case 'A' <= r && r <= 'Z',
-			'a' <= r && r <= 'z',
-			'0' <= r && r <= '9',
-			r == '_',
-			r == '+',
-			r == '-',
-			r == '=',
-			r == '.',
-			r == ',',
-			r == '/',
-			r == ' ':
-			return false
-		default:
-			return true
-		}
+	shellUnsafeCharNotSpace := func(r rune) bool {
+		return r != ' ' && shellUnsafeChar(r)
 	}
 
+	if strings.IndexFunc(s, shellUnsafeCharNotSpace) == -1 {
+		// No escaping necessary
+		return s
+	}
+
+	return `'` + singleQuoteReplacer.Replace(s) + `'`
+}
+
+// ShellEscapeIncludingSpaces escapes the input `s` in a similar way to ShellEscape except that
+// this treats spaces as meaningful characters.
+func ShellEscapeIncludingSpaces(s string) string {
 	if strings.IndexFunc(s, shellUnsafeChar) == -1 {
 		// No escaping necessary
 		return s
diff --git a/proptools/escape_test.go b/proptools/escape_test.go
index 633d711..5823a05 100644
--- a/proptools/escape_test.go
+++ b/proptools/escape_test.go
@@ -91,6 +91,24 @@ var shellEscapeTestCase = []escapeTestCase{
 	},
 }
 
+var shellEscapeIncludingSpacesTestCase = []escapeTestCase{
+	{
+		name: "no escaping",
+		in:   `test`,
+		out:  `test`,
+	},
+	{
+		name: "spacing",
+		in:   `arg1 arg2`,
+		out:  `'arg1 arg2'`,
+	},
+	{
+		name: "single quote",
+		in:   `'arg'`,
+		out:  `''\''arg'\'''`,
+	},
+}
+
 func TestNinjaEscaping(t *testing.T) {
 	for _, testCase := range ninjaEscapeTestCase {
 		got := NinjaEscape(testCase.in)
@@ -109,6 +127,15 @@ func TestShellEscaping(t *testing.T) {
 	}
 }
 
+func TestShellEscapeIncludingSpaces(t *testing.T) {
+	for _, testCase := range shellEscapeIncludingSpacesTestCase {
+		got := ShellEscapeIncludingSpaces(testCase.in)
+		if got != testCase.out {
+			t.Errorf("%s: expected `%s` got `%s`", testCase.name, testCase.out, got)
+		}
+	}
+}
+
 func TestExternalShellEscaping(t *testing.T) {
 	if testing.Short() {
 		return
@@ -124,3 +151,19 @@ func TestExternalShellEscaping(t *testing.T) {
 		}
 	}
 }
+
+func TestExternalShellEscapeIncludingSpaces(t *testing.T) {
+	if testing.Short() {
+		return
+	}
+	for _, testCase := range shellEscapeIncludingSpacesTestCase {
+		cmd := "echo -n " + ShellEscapeIncludingSpaces(testCase.in)
+		got, err := exec.Command("/bin/sh", "-c", cmd).Output()
+		if err != nil {
+			t.Error(err)
+		}
+		if string(got) != testCase.in {
+			t.Errorf("%s: expected `%s` got `%s`", testCase.name, testCase.in, got)
+		}
+	}
+}