removed duplication of shellQuote function and added test cases. (#3927)

Azure · Oct 1, 2018 · e89eb97 · e89eb97
1 parent e4cb44c
commit e89eb97
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 12 deletions.
diff --git a/pkg/acsengine/template_generator.go b/pkg/acsengine/template_generator.go
@@ -847,10 +847,8 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
 			masterShAsset := getOpenshiftMasterShAsset(cs.Properties.OrchestratorProfile.OrchestratorVersion)
 			tb := MustAsset(masterShAsset)
 			t, err := template.New("master").Funcs(template.FuncMap{
-				"quote": strconv.Quote,
-				"shellQuote": func(s string) string {
-					return `'` + strings.Replace(s, `'`, `'\''`, -1) + `'`
-				},
+				"quote":      strconv.Quote,
+				"shellQuote": helpers.ShellQuote,
 			}).Parse(string(tb))
 			if err != nil {
 				return "", err
@@ -875,10 +873,8 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
 			nodeShAsset := getOpenshiftNodeShAsset(cs.Properties.OrchestratorProfile.OrchestratorVersion)
 			tb := MustAsset(nodeShAsset)
 			t, err := template.New("node").Funcs(template.FuncMap{
-				"quote": strconv.Quote,
-				"shellQuote": func(s string) string {
-					return `'` + strings.Replace(s, `'`, `'\''`, -1) + `'`
-				},
+				"quote":      strconv.Quote,
+				"shellQuote": helpers.ShellQuote,
 			}).Parse(string(tb))
 			if err != nil {
 				return "", err
@@ -921,9 +917,7 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
 		"IsCustomVNET": func() bool {
 			return cs.Properties.AreAgentProfilesCustomVNET()
 		},
-		"quote": strconv.Quote,
-		"shellQuote": func(s string) string {
-			return `'` + strings.Replace(s, `'`, `'\''`, -1) + `'`
-		},
+		"quote":      strconv.Quote,
+		"shellQuote": helpers.ShellQuote,
 	}
 }
diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go
@@ -156,3 +156,8 @@ func GetHomeDir() string {
 	}
 	return os.Getenv("HOME")
 }
+
+// ShellQuote returns a string that is enclosed within single quotes. If the string already has single quotes, they will be escaped.
+func ShellQuote(s string) string {
+	return `'` + strings.Replace(s, `'`, `'\''`, -1) + `'`
+}
diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go
@@ -5,6 +5,8 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"math/rand"
+	"os/exec"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -231,3 +233,84 @@ func TestEqualError(t *testing.T) {
 		}
 	}
 }
+
+func TestShellQuote(t *testing.T) {
+	testcases := []struct {
+		input    string
+		expected string
+	}{
+		{
+			"Hel'lo'p\"la`ygr'ound'",
+			`'Hel'\''lo'\''p"la` + "`" + `ygr'\''ound'\'''`,
+		},
+		{
+			`"PwEV@QG7/PYt"re9`,
+			`'"PwEV@QG7/PYt"re9'`,
+		},
+		{
+			"",
+			"''",
+		},
+		{
+			"plaintext1234",
+			`'plaintext1234'`,
+		},
+		{
+			"Hel'lo'p\"la`ygr'ound",
+			`'Hel'\''lo'\''p"la` + "`" + `ygr'\''ound'`,
+		},
+		{
+			`conse''cutive`,
+			`'conse'\'''\''cutive'`,
+		},
+		{
+			"conse\\\\cutive",
+			`'conse\\cutive'`,
+		},
+		{
+			"consec\"\"utive",
+			`'consec""utive'`,
+		},
+		{
+			`PwEV@QG7/PYt"re9`,
+			`'PwEV@QG7/PYt"re9'`,
+		},
+		{
+			"Lnsr@191",
+			"'Lnsr@191'",
+		},
+		{
+			"Jach#321",
+			"'Jach#321'",
+		},
+		{
+			"Bgmo%219",
+			"'Bgmo%219'",
+		},
+		{
+			"@#$%^&*-_!+=[]{}|\\:,.?/~\"();" + "`",
+			`'@#$%^&*-_!+=[]{}|\:,.?/~"();` + "`'",
+		},
+	}
+
+	for _, test := range testcases {
+		actual := ShellQuote(test.input)
+
+		if actual != test.expected {
+			t.Errorf("expected shellQuote to return %s, but got %s", test.expected, actual)
+		}
+
+		if runtime.GOOS != "windows" {
+			out, err := exec.Command("/bin/bash", "-c", "testvar="+actual+"; echo $testvar").Output()
+			if err != nil {
+				t.Errorf("unexpected error : %s", err.Error())
+			}
+
+			o := string(out[:len(out)-1]) //dropping last character as echo prints out a new line as the last character
+
+			if o != test.input {
+				t.Errorf("failed in Bash output test. Expected %s but got %s", test, o)
+			}
+		}
+	}
+}