Merge pull request #296 from compose-spec/fix-interpolation-interference

Fix Interpolation interference from outer string
compose-spec · Aug 1, 2022 · ff627a8 · ff627a8
2 parents 56e6c33 + 5062118
commit ff627a8
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 20 deletions.
diff --git a/template/template.go b/template/template.go
@@ -61,13 +61,14 @@ type SubstituteFunc func(string, Mapping) (string, bool, error)
 // SubstituteWith substitute variables in the string with their values.
 // It accepts additional substitute function.
 func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, subsFuncs ...SubstituteFunc) (string, error) {
-	var err error
+	var outerErr error
 
-	if len(subsFuncs) == 0 {
-		_, subsFunc := getSubstitutionFunctionForTemplate(template)
-		subsFuncs = []SubstituteFunc{subsFunc}
-	}
 	result := pattern.ReplaceAllStringFunc(template, func(substring string) string {
+		_, subsFunc := getSubstitutionFunctionForTemplate(substring)
+		if len(subsFuncs) > 0 {
+			subsFunc = subsFuncs[0]
+		}
+
 		closingBraceIndex := getFirstBraceClosingIndex(substring)
 		rest := ""
 		if closingBraceIndex > -1 {
@@ -89,24 +90,21 @@ func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, su
 		}
 
 		if substitution == "" {
-			err = &InvalidTemplateError{Template: template}
+			outerErr = &InvalidTemplateError{Template: template}
 			return ""
 		}
 
 		if braced {
-			for _, f := range subsFuncs {
-				var (
-					value   string
-					applied bool
-				)
-				value, applied, err = f(substitution, mapping)
-				if err != nil {
-					return ""
-				}
-				if !applied {
-					continue
-				}
-				interpolatedNested, err := SubstituteWith(rest, mapping, pattern, subsFuncs...)
+			var (
+				value   string
+				applied bool
+			)
+			value, applied, outerErr = subsFunc(substitution, mapping)
+			if outerErr != nil {
+				return ""
+			}
+			if applied {
+				interpolatedNested, err := SubstituteWith(rest, mapping, pattern)
 				if err != nil {
 					return ""
 				}
@@ -121,7 +119,7 @@ func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, su
 		return value
 	})
 
-	return result, err
+	return result, outerErr
 }
 
 func getSubstitutionFunctionForTemplate(template string) (string, SubstituteFunc) {

diff --git a/template/template_test.go b/template/template_test.go
@@ -155,6 +155,63 @@ func TestNonAlphanumericDefault(t *testing.T) {
 	assert.Check(t, is.Equal("ok /non:-alphanumeric", result))
 }
 
+func TestInterpolationExternalInterference(t *testing.T) {
+	testCases := []struct {
+		name     string
+		template string
+		expected string
+	}{
+		{
+			template: "-ok ${BAR:-defaultValue}",
+			expected: "-ok defaultValue",
+		},
+		{
+			template: "+ok ${UNSET:-${BAR-defaultValue}}",
+			expected: "+ok ",
+		},
+		{
+			template: "-ok ${FOO:-defaultValue}",
+			expected: "-ok first",
+		},
+		{
+			template: ":-ok ${UNSET-defaultValue}",
+			expected: ":-ok defaultValue",
+		},
+		{
+			template: ":-ok ${BAR-defaultValue}",
+			expected: ":-ok ",
+		},
+		{
+			template: ":?ok ${BAR-defaultValue}",
+			expected: ":?ok ",
+		},
+		{
+			template: ":?ok ${BAR:-defaultValue}",
+			expected: ":?ok defaultValue",
+		},
+		{
+			template: ":+ok ${BAR:-defaultValue}",
+			expected: ":+ok defaultValue",
+		},
+		{
+			template: "+ok ${BAR-defaultValue}",
+			expected: "+ok ",
+		},
+		{
+			template: "?ok ${BAR:-defaultValue}",
+			expected: "?ok defaultValue",
+		},
+	}
+	for i, tc := range testCases {
+		tc := tc
+		t.Run(fmt.Sprintf("Interpolation Should not be impacted by outer text: %d", i), func(t *testing.T) {
+			result, err := Substitute(tc.template, defaultMapping)
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(tc.expected, result))
+		})
+	}
+}
+
 func TestDefaultsWithNestedExpansion(t *testing.T) {
 	testCases := []struct {
 		template string