Browse code

Fix some escaping around env var processing Clarify in the docs that ENV is not recursive

Closes #10391

Signed-off-by: Doug Davis <dug@us.ibm.com>

Doug Davis authored on 2015/01/29 11:28:48
Showing 10 changed files
... ...
@@ -302,7 +302,11 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
302 302
 		var str string
303 303
 		str = ast.Value
304 304
 		if _, ok := replaceEnvAllowed[cmd]; ok {
305
-			str = b.replaceEnv(ast.Value)
305
+			var err error
306
+			str, err = ProcessWord(ast.Value, b.Config.Env)
307
+			if err != nil {
308
+				return err
309
+			}
306 310
 		}
307 311
 		strList[i+l] = str
308 312
 		msgList[i] = ast.Value
... ...
@@ -90,7 +90,7 @@ func parseNameVal(rest string, key string) (*Node, map[string]bool, error) {
90 90
 				if blankOK || len(word) > 0 {
91 91
 					words = append(words, word)
92 92
 
93
-					// Look for = and if no there assume
93
+					// Look for = and if not there assume
94 94
 					// we're doing the old stuff and
95 95
 					// just read the rest of the line
96 96
 					if !strings.Contains(word, "=") {
... ...
@@ -107,12 +107,15 @@ func parseNameVal(rest string, key string) (*Node, map[string]bool, error) {
107 107
 				quote = ch
108 108
 				blankOK = true
109 109
 				phase = inQuote
110
-				continue
111 110
 			}
112 111
 			if ch == '\\' {
113 112
 				if pos+1 == len(rest) {
114 113
 					continue // just skip \ at end
115 114
 				}
115
+				// If we're not quoted and we see a \, then always just
116
+				// add \ plus the char to the word, even if the char
117
+				// is a quote.
118
+				word += string(ch)
116 119
 				pos++
117 120
 				ch = rune(rest[pos])
118 121
 			}
... ...
@@ -122,15 +125,17 @@ func parseNameVal(rest string, key string) (*Node, map[string]bool, error) {
122 122
 		if phase == inQuote {
123 123
 			if ch == quote {
124 124
 				phase = inWord
125
-				continue
126 125
 			}
127
-			if ch == '\\' {
126
+			// \ is special except for ' quotes - can't escape anything for '
127
+			if ch == '\\' && quote != '\'' {
128 128
 				if pos+1 == len(rest) {
129 129
 					phase = inWord
130 130
 					continue // just skip \ at end
131 131
 				}
132 132
 				pos++
133
-				ch = rune(rest[pos])
133
+				nextCh := rune(rest[pos])
134
+				word += string(ch)
135
+				ch = nextCh
134 136
 			}
135 137
 			word += string(ch)
136 138
 		}
... ...
@@ -7,6 +7,14 @@ ENV name=value\ value2
7 7
 ENV name="value'quote space'value2"
8 8
 ENV name='value"double quote"value2'
9 9
 ENV name=value\ value2 name2=value2\ value3
10
+ENV name="a\"b"
11
+ENV name="a\'b"
12
+ENV name='a\'b'
13
+ENV name='a\'b''
14
+ENV name='a\"b'
15
+ENV name="''"
16
+# don't put anything after the next line - it must be the last line of the
17
+# Dockerfile and it must end with \
10 18
 ENV name=value \
11 19
     name1=value1 \
12 20
     name2="value2a \
... ...
@@ -2,9 +2,15 @@
2 2
 (env "name" "value")
3 3
 (env "name" "value")
4 4
 (env "name" "value" "name2" "value2")
5
-(env "name" "value value1")
6
-(env "name" "value value2")
7
-(env "name" "value'quote space'value2")
8
-(env "name" "value\"double quote\"value2")
9
-(env "name" "value value2" "name2" "value2 value3")
10
-(env "name" "value" "name1" "value1" "name2" "value2a            value2b" "name3" "value3an\"value3b\"" "name4" "value4a\\nvalue4b")
5
+(env "name" "\"value value1\"")
6
+(env "name" "value\\ value2")
7
+(env "name" "\"value'quote space'value2\"")
8
+(env "name" "'value\"double quote\"value2'")
9
+(env "name" "value\\ value2" "name2" "value2\\ value3")
10
+(env "name" "\"a\\\"b\"")
11
+(env "name" "\"a\\'b\"")
12
+(env "name" "'a\\'b'")
13
+(env "name" "'a\\'b''")
14
+(env "name" "'a\\\"b'")
15
+(env "name" "\"''\"")
16
+(env "name" "value" "name1" "value1" "name2" "\"value2a            value2b\"" "name3" "\"value3a\\n\\\"value3b\\\"\"" "name4" "\"value4a\\\\nvalue4b\"")
11 17
new file mode 100644
... ...
@@ -0,0 +1,209 @@
0
+package builder
1
+
2
+// This will take a single word and an array of env variables and
3
+// process all quotes (" and ') as well as $xxx and ${xxx} env variable
4
+// tokens.  Tries to mimic bash shell process.
5
+// It doesn't support all flavors of ${xx:...} formats but new ones can
6
+// be added by adding code to the "special ${} format processing" section
7
+
8
+import (
9
+	"fmt"
10
+	"strings"
11
+	"unicode"
12
+)
13
+
14
+type shellWord struct {
15
+	word string
16
+	envs []string
17
+	pos  int
18
+}
19
+
20
+func ProcessWord(word string, env []string) (string, error) {
21
+	sw := &shellWord{
22
+		word: word,
23
+		envs: env,
24
+		pos:  0,
25
+	}
26
+	return sw.process()
27
+}
28
+
29
+func (sw *shellWord) process() (string, error) {
30
+	return sw.processStopOn('\000')
31
+}
32
+
33
+// Process the word, starting at 'pos', and stop when we get to the
34
+// end of the word or the 'stopChar' character
35
+func (sw *shellWord) processStopOn(stopChar rune) (string, error) {
36
+	var result string
37
+	var charFuncMapping = map[rune]func() (string, error){
38
+		'\'': sw.processSingleQuote,
39
+		'"':  sw.processDoubleQuote,
40
+		'$':  sw.processDollar,
41
+	}
42
+
43
+	for sw.pos < len(sw.word) {
44
+		ch := sw.peek()
45
+		if stopChar != '\000' && ch == stopChar {
46
+			sw.next()
47
+			break
48
+		}
49
+		if fn, ok := charFuncMapping[ch]; ok {
50
+			// Call special processing func for certain chars
51
+			tmp, err := fn()
52
+			if err != nil {
53
+				return "", err
54
+			}
55
+			result += tmp
56
+		} else {
57
+			// Not special, just add it to the result
58
+			ch = sw.next()
59
+			if ch == '\\' {
60
+				// '\' escapes, except end of line
61
+				ch = sw.next()
62
+				if ch == '\000' {
63
+					continue
64
+				}
65
+			}
66
+			result += string(ch)
67
+		}
68
+	}
69
+
70
+	return result, nil
71
+}
72
+
73
+func (sw *shellWord) peek() rune {
74
+	if sw.pos == len(sw.word) {
75
+		return '\000'
76
+	}
77
+	return rune(sw.word[sw.pos])
78
+}
79
+
80
+func (sw *shellWord) next() rune {
81
+	if sw.pos == len(sw.word) {
82
+		return '\000'
83
+	}
84
+	ch := rune(sw.word[sw.pos])
85
+	sw.pos++
86
+	return ch
87
+}
88
+
89
+func (sw *shellWord) processSingleQuote() (string, error) {
90
+	// All chars between single quotes are taken as-is
91
+	// Note, you can't escape '
92
+	var result string
93
+
94
+	sw.next()
95
+
96
+	for {
97
+		ch := sw.next()
98
+		if ch == '\000' || ch == '\'' {
99
+			break
100
+		}
101
+		result += string(ch)
102
+	}
103
+	return result, nil
104
+}
105
+
106
+func (sw *shellWord) processDoubleQuote() (string, error) {
107
+	// All chars up to the next " are taken as-is, even ', except any $ chars
108
+	// But you can escape " with a \
109
+	var result string
110
+
111
+	sw.next()
112
+
113
+	for sw.pos < len(sw.word) {
114
+		ch := sw.peek()
115
+		if ch == '"' {
116
+			sw.next()
117
+			break
118
+		}
119
+		if ch == '$' {
120
+			tmp, err := sw.processDollar()
121
+			if err != nil {
122
+				return "", err
123
+			}
124
+			result += tmp
125
+		} else {
126
+			ch = sw.next()
127
+			if ch == '\\' {
128
+				chNext := sw.peek()
129
+
130
+				if chNext == '\000' {
131
+					// Ignore \ at end of word
132
+					continue
133
+				}
134
+
135
+				if chNext == '"' || chNext == '$' {
136
+					// \" and \$ can be escaped, all other \'s are left as-is
137
+					ch = sw.next()
138
+				}
139
+			}
140
+			result += string(ch)
141
+		}
142
+	}
143
+
144
+	return result, nil
145
+}
146
+
147
+func (sw *shellWord) processDollar() (string, error) {
148
+	sw.next()
149
+	ch := sw.peek()
150
+	if ch == '{' {
151
+		sw.next()
152
+		name := sw.processName()
153
+		ch = sw.peek()
154
+		if ch == '}' {
155
+			// Normal ${xx} case
156
+			sw.next()
157
+			return sw.getEnv(name), nil
158
+		}
159
+		return "", fmt.Errorf("Unsupported ${} substitution: %s", sw.word)
160
+	} else {
161
+		// $xxx case
162
+		name := sw.processName()
163
+		if name == "" {
164
+			return "$", nil
165
+		}
166
+		return sw.getEnv(name), nil
167
+	}
168
+}
169
+
170
+func (sw *shellWord) processName() string {
171
+	// Read in a name (alphanumeric or _)
172
+	// If it starts with a numeric then just return $#
173
+	var name string
174
+
175
+	for sw.pos < len(sw.word) {
176
+		ch := sw.peek()
177
+		if len(name) == 0 && unicode.IsDigit(ch) {
178
+			ch = sw.next()
179
+			return string(ch)
180
+		}
181
+		if !unicode.IsLetter(ch) && !unicode.IsDigit(ch) && ch != '_' {
182
+			break
183
+		}
184
+		ch = sw.next()
185
+		name += string(ch)
186
+	}
187
+
188
+	return name
189
+}
190
+
191
+func (sw *shellWord) getEnv(name string) string {
192
+	for _, env := range sw.envs {
193
+		i := strings.Index(env, "=")
194
+		if i < 0 {
195
+			if name == env {
196
+				// Should probably never get here, but just in case treat
197
+				// it like "var" and "var=" are the same
198
+				return ""
199
+			}
200
+			continue
201
+		}
202
+		if name != env[:i] {
203
+			continue
204
+		}
205
+		return env[i+1:]
206
+	}
207
+	return ""
208
+}
0 209
new file mode 100644
... ...
@@ -0,0 +1,51 @@
0
+package builder
1
+
2
+import (
3
+	"bufio"
4
+	"os"
5
+	"strings"
6
+	"testing"
7
+)
8
+
9
+func TestShellParser(t *testing.T) {
10
+	file, err := os.Open("words")
11
+	if err != nil {
12
+		t.Fatalf("Can't open 'words': %s", err)
13
+	}
14
+	defer file.Close()
15
+
16
+	scanner := bufio.NewScanner(file)
17
+	envs := []string{"PWD=/home", "SHELL=bash"}
18
+	for scanner.Scan() {
19
+		line := scanner.Text()
20
+
21
+		// Trim comments and blank lines
22
+		i := strings.Index(line, "#")
23
+		if i >= 0 {
24
+			line = line[:i]
25
+		}
26
+		line = strings.TrimSpace(line)
27
+
28
+		if line == "" {
29
+			continue
30
+		}
31
+
32
+		words := strings.Split(line, "|")
33
+		if len(words) != 2 {
34
+			t.Fatalf("Error in 'words' - should be 2 words:%q", words)
35
+		}
36
+
37
+		words[0] = strings.TrimSpace(words[0])
38
+		words[1] = strings.TrimSpace(words[1])
39
+
40
+		newWord, err := ProcessWord(words[0], envs)
41
+
42
+		if err != nil {
43
+			newWord = "error"
44
+		}
45
+
46
+		if newWord != words[1] {
47
+			t.Fatalf("Error. Src: %s  Calc: %s  Expected: %s", words[0], newWord, words[1])
48
+		}
49
+	}
50
+}
... ...
@@ -1,50 +1,9 @@
1 1
 package builder
2 2
 
3 3
 import (
4
-	"regexp"
5 4
 	"strings"
6 5
 )
7 6
 
8
-var (
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
15
-)
16
-
17
-// handle environment replacement. Used in dispatcher.
18
-func (b *Builder) replaceEnv(str string) string {
19
-	for _, match := range tokenEnvInterpolation.FindAllString(str, -1) {
20
-		idx := strings.Index(match, "\\$")
21
-		if idx != -1 {
22
-			if idx+2 >= len(match) {
23
-				str = strings.Replace(str, match, "\\$", -1)
24
-				continue
25
-			}
26
-
27
-			prefix := match[:idx]
28
-			stripped := match[idx+2:]
29
-			str = strings.Replace(str, match, prefix+"$"+stripped, -1)
30
-			continue
31
-		}
32
-
33
-		match = match[strings.Index(match, "$"):]
34
-		matchKey := strings.Trim(match, "${}")
35
-
36
-		for _, keyval := range b.Config.Env {
37
-			tmp := strings.SplitN(keyval, "=", 2)
38
-			if tmp[0] == matchKey {
39
-				str = strings.Replace(str, match, tmp[1], -1)
40
-				break
41
-			}
42
-		}
43
-	}
44
-
45
-	return str
46
-}
47
-
48 7
 func handleJsonArgs(args []string, attributes map[string]bool) []string {
49 8
 	if len(args) == 0 {
50 9
 		return []string{}
51 10
new file mode 100644
... ...
@@ -0,0 +1,43 @@
0
+hello                    |     hello
1
+he'll'o                  |     hello
2
+he'llo                   |     hello
3
+he\'llo                  |     he'llo
4
+he\\'llo                 |     he\llo
5
+abc\tdef                 |     abctdef
6
+"abc\tdef"               |     abc\tdef
7
+'abc\tdef'               |     abc\tdef
8
+hello\                   |     hello
9
+hello\\                  |     hello\
10
+"hello                   |     hello
11
+"hello\"                 |     hello"
12
+"hel'lo"                 |     hel'lo
13
+'hello                   |     hello
14
+'hello\'                 |     hello\
15
+"''"                     |     ''
16
+$.                       |     $.
17
+$1                       |     
18
+he$1x                    |     hex
19
+he$.x                    |     he$.x
20
+he$pwd.                  |     he.
21
+he$PWD                   |     he/home
22
+he\$PWD                  |     he$PWD
23
+he\\$PWD                 |     he\/home
24
+he\${}                   |     he${}
25
+he\${}xx                 |     he${}xx
26
+he${}                    |     he
27
+he${}xx                  |     hexx
28
+he${hi}                  |     he
29
+he${hi}xx                |     hexx
30
+he${PWD}                 |     he/home
31
+he${.}                   |     error
32
+'he${XX}'                |     he${XX}
33
+"he${PWD}"               |     he/home
34
+"he'$PWD'"               |     he'/home'
35
+"$PWD"                   |     /home
36
+'$PWD'                   |     $PWD
37
+'\$PWD'                  |     \$PWD
38
+'"hello"'                |     "hello"
39
+he\$PWD                  |     he$PWD
40
+"he\$PWD"                |     he$PWD
41
+'he\$PWD'                |     he\$PWD
42
+he${PWD                  |     error
... ...
@@ -146,6 +146,17 @@ The instructions that handle environment variables in the `Dockerfile` are:
146 146
 `ONBUILD` instructions are **NOT** supported for environment replacement, even
147 147
 the instructions above.
148 148
 
149
+Environment variable subtitution will use the same value for each variable
150
+throughout the entire command.  In other words, in this example:
151
+
152
+    ENV abc=hello
153
+    ENV abc=bye def=$abc
154
+    ENV ghi=$abc
155
+
156
+will result in `def` having a value of `hello`, not `bye`.  However, 
157
+`ghi` will have a value of `bye` because it is not part of the same command
158
+that set `abc` to `bye`.
159
+
149 160
 ## The `.dockerignore` file
150 161
 
151 162
 If a file named `.dockerignore` exists in the source repository, then it
... ...
@@ -239,9 +239,18 @@ func TestBuildEnvironmentReplacementEnv(t *testing.T) {
239 239
 
240 240
 	_, err := buildImage(name,
241 241
 		`
242
-  FROM scratch
243
-  ENV foo foo
242
+  FROM busybox
243
+  ENV foo zzz
244 244
   ENV bar ${foo}
245
+  ENV abc1='$foo'
246
+  ENV env1=$foo env2=${foo} env3="$foo" env4="${foo}"
247
+  RUN [ "$abc1" = '$foo' ] && (echo "$abc1" | grep -q foo)
248
+  ENV abc2="\$foo"
249
+  RUN [ "$abc2" = '$foo' ] && (echo "$abc2" | grep -q foo)
250
+  ENV abc3 '$foo'
251
+  RUN [ "$abc3" = '$foo' ] && (echo "$abc3" | grep -q foo)
252
+  ENV abc4 "\$foo"
253
+  RUN [ "$abc4" = '$foo' ] && (echo "$abc4" | grep -q foo)
245 254
   `, true)
246 255
 
247 256
 	if err != nil {
... ...
@@ -260,13 +269,19 @@ func TestBuildEnvironmentReplacementEnv(t *testing.T) {
260 260
 	}
261 261
 
262 262
 	found := false
263
+	envCount := 0
263 264
 
264 265
 	for _, env := range envResult {
265 266
 		parts := strings.SplitN(env, "=", 2)
266 267
 		if parts[0] == "bar" {
267 268
 			found = true
268
-			if parts[1] != "foo" {
269
-				t.Fatalf("Could not find replaced var for env `bar`: got %q instead of `foo`", parts[1])
269
+			if parts[1] != "zzz" {
270
+				t.Fatalf("Could not find replaced var for env `bar`: got %q instead of `zzz`", parts[1])
271
+			}
272
+		} else if strings.HasPrefix(parts[0], "env") {
273
+			envCount++
274
+			if parts[1] != "zzz" {
275
+				t.Fatalf("%s should be 'foo' but instead its %q", parts[0], parts[1])
270 276
 			}
271 277
 		}
272 278
 	}
... ...
@@ -275,6 +290,10 @@ func TestBuildEnvironmentReplacementEnv(t *testing.T) {
275 275
 		t.Fatal("Never found the `bar` env variable")
276 276
 	}
277 277
 
278
+	if envCount != 4 {
279
+		t.Fatalf("Didn't find all env vars - only saw %d\n%s", envCount, envResult)
280
+	}
281
+
278 282
 	logDone("build - env environment replacement")
279 283
 }
280 284
 
... ...
@@ -361,8 +380,8 @@ func TestBuildHandleEscapes(t *testing.T) {
361 361
 		t.Fatal(err)
362 362
 	}
363 363
 
364
-	if _, ok := result[`\\\\\\${FOO}`]; !ok {
365
-		t.Fatal(`Could not find volume \\\\\\${FOO} set from env foo in volumes table`)
364
+	if _, ok := result[`\\\${FOO}`]; !ok {
365
+		t.Fatal(`Could not find volume \\\${FOO} set from env foo in volumes table`, result)
366 366
 	}
367 367
 
368 368
 	logDone("build - handle escapes")
... ...
@@ -2128,7 +2147,7 @@ func TestBuildRelativeWorkdir(t *testing.T) {
2128 2128
 
2129 2129
 func TestBuildWorkdirWithEnvVariables(t *testing.T) {
2130 2130
 	name := "testbuildworkdirwithenvvariables"
2131
-	expected := "/test1/test2/$MISSING_VAR"
2131
+	expected := "/test1/test2"
2132 2132
 	defer deleteImages(name)
2133 2133
 	_, err := buildImage(name,
2134 2134
 		`FROM busybox
... ...
@@ -3897,9 +3916,9 @@ ENV    abc=zzz TO=/docker/world/hello
3897 3897
 ADD    $FROM $TO
3898 3898
 RUN    [ "$(cat $TO)" = "hello" ]
3899 3899
 ENV    abc "zzz"
3900
-RUN    [ $abc = \"zzz\" ]
3900
+RUN    [ $abc = "zzz" ]
3901 3901
 ENV    abc 'yyy'
3902
-RUN    [ $abc = \'yyy\' ]
3902
+RUN    [ $abc = 'yyy' ]
3903 3903
 ENV    abc=
3904 3904
 RUN    [ "$abc" = "" ]
3905 3905
 
... ...
@@ -3915,13 +3934,34 @@ RUN    [ "$abc" = "'foo'" ]
3915 3915
 ENV    abc=\"foo\"
3916 3916
 RUN    [ "$abc" = "\"foo\"" ]
3917 3917
 ENV    abc "foo"
3918
-RUN    [ "$abc" = "\"foo\"" ]
3918
+RUN    [ "$abc" = "foo" ]
3919 3919
 ENV    abc 'foo'
3920
-RUN    [ "$abc" = "'foo'" ]
3920
+RUN    [ "$abc" = 'foo' ]
3921 3921
 ENV    abc \'foo\'
3922
-RUN    [ "$abc" = "\\'foo\\'" ]
3922
+RUN    [ "$abc" = "'foo'" ]
3923 3923
 ENV    abc \"foo\"
3924
-RUN    [ "$abc" = "\\\"foo\\\"" ]
3924
+RUN    [ "$abc" = '"foo"' ]
3925
+
3926
+ENV    e1=bar
3927
+ENV    e2=$e1
3928
+ENV    e3=$e11
3929
+ENV    e4=\$e1
3930
+ENV    e5=\$e11
3931
+RUN    [ "$e0,$e1,$e2,$e3,$e4,$e5" = ',bar,bar,,$e1,$e11' ]
3932
+
3933
+ENV    ee1 bar
3934
+ENV    ee2 $ee1
3935
+ENV    ee3 $ee11
3936
+ENV    ee4 \$ee1
3937
+ENV    ee5 \$ee11
3938
+RUN    [ "$ee1,$ee2,$ee3,$ee4,$ee5" = 'bar,bar,,$ee1,$ee11' ]
3939
+
3940
+ENV    eee1="foo"
3941
+ENV    eee2='foo'
3942
+ENV    eee3 "foo"
3943
+ENV    eee4 'foo'
3944
+RUN    [ "$eee1,$eee2,$eee3,$eee4" = 'foo,foo,foo,foo' ]
3945
+
3925 3946
 `
3926 3947
 	ctx, err := fakeContext(dockerfile, map[string]string{
3927 3948
 		"hello/docker/world": "hello",