From 006b4996cbceda2cdebb7a14ab49e80cde143bbb Mon Sep 17 00:00:00 2001
From: Cole Wippern <cgwippern@google.com>
Date: Fri, 8 Nov 2019 14:10:36 -0800
Subject: [PATCH] Fix quote strip behavior for ARG values

* fixes issue 847
* previous implementation did not properly parse blank
values which were enclosed in quotes
---
 .../Dockerfile_test_arg_multi_empty_val       |  3 +
 pkg/dockerfile/dockerfile.go                  | 65 +++++++++++++++----
 pkg/dockerfile/dockerfile_test.go             | 10 ++-
 3 files changed, 64 insertions(+), 14 deletions(-)
 create mode 100644 integration/dockerfiles/Dockerfile_test_arg_multi_empty_val

diff --git a/integration/dockerfiles/Dockerfile_test_arg_multi_empty_val b/integration/dockerfiles/Dockerfile_test_arg_multi_empty_val
new file mode 100644
index 0000000000..4bc755d03e
--- /dev/null
+++ b/integration/dockerfiles/Dockerfile_test_arg_multi_empty_val
@@ -0,0 +1,3 @@
+ARG VARIANT=""
+ARG VERSION=1
+FROM busybox${VARIANT}:1.3${VERSION}
diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go
index ea3f5a392b..9929f7f4d9 100644
--- a/pkg/dockerfile/dockerfile.go
+++ b/pkg/dockerfile/dockerfile.go
@@ -123,23 +123,13 @@ func Parse(b []byte) ([]instructions.Stage, []instructions.ArgCommand, error) {
 // stripEnclosingQuotes removes quotes enclosing the value of each instructions.ArgCommand in a slice
 // if the quotes are escaped it leaves them
 func stripEnclosingQuotes(metaArgs []instructions.ArgCommand) ([]instructions.ArgCommand, error) {
-	dbl := byte('"')
-	sgl := byte('\'')
-
 	for i := range metaArgs {
 		arg := metaArgs[i]
 		v := arg.Value
 		if v != nil {
-			val := *v
-			fmt.Printf("val %s\n", val)
-			if (val[0] == dbl && val[len(val)-1] == dbl) || (val[0] == sgl && val[len(val)-1] == sgl) {
-				val = val[1 : len(val)-1]
-			} else if val[:2] == `\"` && val[len(val)-2:] == `\"` {
-				continue
-			} else if val[:2] == `\'` && val[len(val)-2:] == `\'` {
-				continue
-			} else if val[0] == dbl || val[0] == sgl || val[len(val)-1] == dbl || val[len(val)-1] == sgl {
-				return nil, errors.New("quotes wrapping arg values must be matched")
+			val, err := extractValFromQuotes(*v)
+			if err != nil {
+				return nil, err
 			}
 
 			arg.Value = &val
@@ -149,6 +139,55 @@ func stripEnclosingQuotes(metaArgs []instructions.ArgCommand) ([]instructions.Ar
 	return metaArgs, nil
 }
 
+func extractValFromQuotes(val string) (string, error) {
+	backSlash := byte('\\')
+	if len(val) < 2 {
+		return val, nil
+	}
+
+	var leader string
+	var tail string
+
+	switch char := val[0]; char {
+	case '\'', '"':
+		leader = string([]byte{char})
+	case backSlash:
+		switch char := val[1]; char {
+		case '\'', '"':
+			leader = string([]byte{backSlash, char})
+		}
+	}
+
+	// If the length of leader is greater than one then it must be an escaped
+	// character.
+	if len(leader) < 2 {
+		switch char := val[len(val)-1]; char {
+		case '\'', '"':
+			tail = string([]byte{char})
+		}
+	} else {
+		switch char := val[len(val)-2:]; char {
+		case `\'`, `\"`:
+			tail = char
+		}
+	}
+
+	if leader != tail {
+		logrus.Infof("leader %s tail %s", leader, tail)
+		return "", errors.New("quotes wrapping arg values must be matched")
+	}
+
+	if leader == "" {
+		return val, nil
+	}
+
+	if len(leader) == 2 {
+		return val, nil
+	}
+
+	return val[1 : len(val)-1], nil
+}
+
 // targetStage returns the index of the target stage kaniko is trying to build
 func targetStage(stages []instructions.Stage, target string) (int, error) {
 	if target == "" {
diff --git a/pkg/dockerfile/dockerfile_test.go b/pkg/dockerfile/dockerfile_test.go
index 52974bd27a..a5cdfc7745 100644
--- a/pkg/dockerfile/dockerfile_test.go
+++ b/pkg/dockerfile/dockerfile_test.go
@@ -118,7 +118,7 @@ func Test_stripEnclosingQuotes(t *testing.T) {
 		success:  true,
 	}, {
 		name:     "blank value with enclosing double quotes",
-		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", "\"\"")},
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", `""`)},
 		expected: []string{""},
 		success:  true,
 	}, {
@@ -144,6 +144,14 @@ func Test_stripEnclosingQuotes(t *testing.T) {
 		},
 		expected: []string{"Purr", "Mrow"},
 		success:  true,
+	}, {
+		name: "multiple values, one blank, one a single int",
+		inArgs: []instructions.ArgCommand{
+			newArgCommand("MEOW", `""`),
+			newArgCommand("MEW", `1`),
+		},
+		expected: []string{"", "1"},
+		success:  true,
 	}, {
 		name:    "no values",
 		success: true,