Browse code

Fix builder from being over-aggressive on ${}

`${SOME_VAR%pattern}` was turning into `SOME_VAL%pattern}` which the shell would then balk at.

I've updated the `TOKEN_ENV_INTERPOLATION` regex to account for this (ie, if `${` is used, it _must_ also match the closing `}`), and renamed the variable to not be exported (since it's not used outside the function following it).

I also added comments for the bits of `tokenEnvInterpolation` so they're easier to follow. :smile:

Signed-off-by: Andrew Page <admwiggin@gmail.com>

Tianon Gravi authored on 2014/10/14 15:58:55
Showing 2 changed files
... ...
@@ -6,12 +6,17 @@ import (
6 6
 )
7 7
 
8 8
 var (
9
-	TOKEN_ENV_INTERPOLATION = regexp.MustCompile(`(\\\\+|[^\\]|\b|\A)\$({?)([[:alnum:]_]+)(}?)`)
9
+	// `\\\\+|[^\\]|\b|\A` - match any number of "\\" (ie, properly-escaped backslashes), or a single non-backslash character, or a word boundary, or beginning-of-line
10
+	// `\$` - match literal $
11
+	// `[[:alnum:]_]+` - match things like `$SOME_VAR`
12
+	// `{[[:alnum:]_]+}` - match things like `${SOME_VAR}`
13
+	tokenEnvInterpolation = regexp.MustCompile(`(\\\\+|[^\\]|\b|\A)\$([[:alnum:]_]+|{[[:alnum:]_]+})`)
14
+	// this intentionally punts on more exotic interpolations like ${SOME_VAR%suffix} and lets the shell handle those directly
10 15
 )
11 16
 
12 17
 // handle environment replacement. Used in dispatcher.
13 18
 func (b *Builder) replaceEnv(str string) string {
14
-	for _, match := range TOKEN_ENV_INTERPOLATION.FindAllString(str, -1) {
19
+	for _, match := range tokenEnvInterpolation.FindAllString(str, -1) {
15 20
 		match = match[strings.Index(match, "$"):]
16 21
 		matchKey := strings.Trim(match, "${}")
17 22
 
... ...
@@ -2710,3 +2710,33 @@ func TestBuildRunShEntrypoint(t *testing.T) {
2710 2710
 
2711 2711
 	logDone("build - entrypoint with /bin/echo running successfully")
2712 2712
 }
2713
+
2714
+func TestBuildExoticShellInterpolation(t *testing.T) {
2715
+	name := "testbuildexoticshellinterpolation"
2716
+	defer deleteImages(name)
2717
+
2718
+	_, err := buildImage(name, `
2719
+		FROM busybox
2720
+
2721
+		ENV SOME_VAR a.b.c
2722
+
2723
+		RUN [ "$SOME_VAR"       = 'a.b.c' ]
2724
+		RUN [ "${SOME_VAR}"     = 'a.b.c' ]
2725
+		RUN [ "${SOME_VAR%.*}"  = 'a.b'   ]
2726
+		RUN [ "${SOME_VAR%%.*}" = 'a'     ]
2727
+		RUN [ "${SOME_VAR#*.}"  = 'b.c'   ]
2728
+		RUN [ "${SOME_VAR##*.}" = 'c'     ]
2729
+		RUN [ "${SOME_VAR/c/d}" = 'a.b.d' ]
2730
+		RUN [ "${#SOME_VAR}"    = '5'     ]
2731
+
2732
+		RUN [ "${SOME_UNSET_VAR:-$SOME_VAR}" = 'a.b.c' ]
2733
+		RUN [ "${SOME_VAR:+Version: ${SOME_VAR}}" = 'Version: a.b.c' ]
2734
+		RUN [ "${SOME_UNSET_VAR:+${SOME_VAR}}" = '' ]
2735
+		RUN [ "${SOME_UNSET_VAR:-${SOME_VAR:-d.e.f}}" = 'a.b.c' ]
2736
+	`, false)
2737
+	if err != nil {
2738
+		t.Fatal(err)
2739
+	}
2740
+
2741
+	logDone("build - exotic shell interpolation")
2742
+}