Issue #439 add additional tests for quotes

Add additional tests to ensure that ARG values with quotes are handled properly
GoogleContainerTools · Oct 25, 2019 · 026bffd · 026bffd
1 parent 79649a1
commit 026bffd
Show file tree

Hide file tree

Showing 9 changed files with 217 additions and 10 deletions.
diff --git a/integration/dockerfiles/Dockerfile_test_arg_blank_with_quotes b/integration/dockerfiles/Dockerfile_test_arg_blank_with_quotes
@@ -0,0 +1,12 @@
+ARG FILE_NAME=""
+
+FROM busybox:latest AS builder
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+
+FROM busybox:latest
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+COPY --from=builder /$FILE_NAME.txt /
diff --git a/integration/dockerfiles/Dockerfile_test_arg_from_missing_first_quote b/integration/dockerfiles/Dockerfile_test_arg_from_missing_first_quote
@@ -0,0 +1,13 @@
+ARG FILE_NAME="myFile"
+ARG IMAGE_NAME=busybox:latest"
+
+FROM $IMAGE_NAME AS builder
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+
+FROM busybox:latest
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+COPY --from=builder /$FILE_NAME.txt /
diff --git a/integration/dockerfiles/Dockerfile_test_arg_from_missing_last_quote b/integration/dockerfiles/Dockerfile_test_arg_from_missing_last_quote
@@ -0,0 +1,13 @@
+ARG FILE_NAME="myFile"
+ARG IMAGE_NAME="busybox:latest
+
+FROM $IMAGE_NAME AS builder
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+
+FROM busybox:latest
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+COPY --from=builder /$FILE_NAME.txt /
diff --git a/integration/dockerfiles/Dockerfile_test_arg_from_quotes b/integration/dockerfiles/Dockerfile_test_arg_from_quotes
@@ -0,0 +1,13 @@
+ARG FILE_NAME="myFile"
+ARG IMAGE_NAME="busybox:latest"
+
+FROM $IMAGE_NAME AS builder
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+
+FROM busybox:latest
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+COPY --from=builder /$FILE_NAME.txt /
diff --git a/integration/dockerfiles/Dockerfile_test_arg_from_quotes_escaped b/integration/dockerfiles/Dockerfile_test_arg_from_quotes_escaped
@@ -0,0 +1,13 @@
+ARG FILE_NAME="myFile"
+ARG IMAGE_NAME=\"busybox:latest\"
+
+FROM $IMAGE_NAME AS builder
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+
+FROM busybox:latest
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+COPY --from=builder /$FILE_NAME.txt /
diff --git a/integration/dockerfiles/Dockerfile_test_arg_from_single_quotes b/integration/dockerfiles/Dockerfile_test_arg_from_single_quotes
@@ -0,0 +1,13 @@
+ARG FILE_NAME="myFile"
+ARG IMAGE_NAME='busybox:latest'
+
+FROM $IMAGE_NAME AS builder
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+
+FROM busybox:latest
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+COPY --from=builder /$FILE_NAME.txt /
diff --git a/integration/dockerfiles/Dockerfile_test_arg_from_single_quotes_escaped b/integration/dockerfiles/Dockerfile_test_arg_from_single_quotes_escaped
@@ -0,0 +1,13 @@
+ARG FILE_NAME="myFile"
+ARG IMAGE_NAME=\'busybox:latest\'
+
+FROM $IMAGE_NAME AS builder
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+
+FROM busybox:latest
+ARG FILE_NAME
+
+RUN echo $FILE_NAME && touch /$FILE_NAME.txt && stat /$FILE_NAME.txt;
+COPY --from=builder /$FILE_NAME.txt /
diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go
@@ -112,30 +112,41 @@ func Parse(b []byte) ([]instructions.Stage, []instructions.ArgCommand, error) {
 		return nil, nil, err
 	}
 
-	metaArgs = stripEnclosingDoubleQuotes(metaArgs)
+	metaArgs, err = stripEnclosingQuotes(metaArgs)
+	if err != nil {
+		return nil, nil, err
+	}
 
 	return stages, metaArgs, nil
 }
 
-// stripEnclosingDoubleQuotes removes double quotes enclosing the value of each instructions.ArgCommand in a slice
-func stripEnclosingDoubleQuotes(metaArgs []instructions.ArgCommand) []instructions.ArgCommand {
+// 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
-			if val[0] == '"' {
-				val = val[1:]
+			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[0:2] == `\"` && val[len(val)-2:len(val)] == `\"` {
+				continue
+			} else if val[0:2] == `\'` && val[len(val)-2:len(val)] == `\'` {
+				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")
 			}
 
-			if val[len(val)-1] == '"' {
-				val = val[:len(val)-1]
-			}
 			arg.Value = &val
 			metaArgs[i] = arg
 		}
 	}
-	return metaArgs
+	return metaArgs, nil
 }
 
 // targetStage returns the index of the target stage kaniko is trying to build

diff --git a/pkg/dockerfile/dockerfile_test.go b/pkg/dockerfile/dockerfile_test.go
@@ -27,7 +27,7 @@ import (
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
 )
 
-func TestStagesArgValueWithQuotes(t *testing.T) {
+func Test_Stages_ArgValueWithQuotes(t *testing.T) {
 	dockerfile := `
 	ARG IMAGE="ubuntu:16.04"
 	FROM ${IMAGE}
@@ -75,6 +75,112 @@ func TestStagesArgValueWithQuotes(t *testing.T) {
 	}
 }
 
+func Test_stripEnclosingQuotes(t *testing.T) {
+	type testCase struct {
+		name     string
+		inArgs   []instructions.ArgCommand
+		expected []string
+		success  bool
+	}
+
+	newArgCommand := func(key, val string) instructions.ArgCommand {
+		return instructions.ArgCommand{
+			KeyValuePairOptional: instructions.KeyValuePairOptional{Key: key, Value: &val},
+		}
+	}
+
+	cases := []testCase{{
+		name:     "value with no enclosing quotes",
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", "Purr")},
+		expected: []string{"Purr"},
+		success:  true,
+	}, {
+		name:   "value with unmatched leading double quote",
+		inArgs: []instructions.ArgCommand{newArgCommand("MEOW", "\"Purr")},
+	}, {
+		name:   "value with unmatched trailing double quote",
+		inArgs: []instructions.ArgCommand{newArgCommand("MEOW", "Purr\"")},
+	}, {
+		name:     "value with enclosing double quotes",
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", "\"mrow\"")},
+		expected: []string{"mrow"},
+		success:  true,
+	}, {
+		name:   "value with unmatched leading single quote",
+		inArgs: []instructions.ArgCommand{newArgCommand("MEOW", "'Purr")},
+	}, {
+		name:   "value with unmatched trailing single quote",
+		inArgs: []instructions.ArgCommand{newArgCommand("MEOW", "Purr'")},
+	}, {
+		name:     "value with enclosing single quotes",
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", "'mrow'")},
+		expected: []string{"mrow"},
+		success:  true,
+	}, {
+		name:     "blank value with enclosing double quotes",
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", "\"\"")},
+		expected: []string{""},
+		success:  true,
+	}, {
+		name:     "blank value with enclosing single quotes",
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", "''")},
+		expected: []string{""},
+		success:  true,
+	}, {
+		name:     "value with escaped, enclosing double quotes",
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", `\"Purr\"`)},
+		expected: []string{`\"Purr\"`},
+		success:  true,
+	}, {
+		name:     "value with escaped, enclosing single quotes",
+		inArgs:   []instructions.ArgCommand{newArgCommand("MEOW", `\'Purr\'`)},
+		expected: []string{`\'Purr\'`},
+		success:  true,
+	}, {
+		name: "multiple values enclosed with single quotes",
+		inArgs: []instructions.ArgCommand{
+			newArgCommand("MEOW", `'Purr'`),
+			newArgCommand("MEW", `'Mrow'`),
+		},
+		expected: []string{"Purr", "Mrow"},
+		success:  true,
+	}, {
+		name:    "no values",
+		success: true,
+	}}
+
+	for _, test := range cases {
+		t.Run(test.name, func(t *testing.T) {
+			inArgs := test.inArgs
+			expected := test.expected
+			success := test.success
+
+			out, err := stripEnclosingQuotes(inArgs)
+			if success && err != nil {
+				t.Fatal(err)
+			}
+
+			if !success && err == nil {
+				t.Fatal("expected an error but none received")
+			}
+
+			if len(expected) != len(out) {
+				t.Fatalf("Expected %d args but got %d", len(expected), len(out))
+			}
+
+			for i := range out {
+				if expected[i] != out[i].ValueString() {
+					t.Errorf(
+						"Expected arg at index %d to equal %v but instead equaled %v",
+						i,
+						expected[i],
+						out[i].ValueString())
+				}
+			}
+		})
+	}
+}
+
 func Test_resolveStages(t *testing.T) {
 	dockerfile := `
 	FROM scratch