Browse code

Fix --label being env var expanded.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

Daniel Nephin authored on 2017/03/14 06:25:37
Showing 5 changed files
... ...
@@ -129,47 +129,18 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error {
129 129
 
130 130
 	}
131 131
 
132
-	// count the number of nodes that we are going to traverse first
133
-	// so we can pre-create the argument and message array. This speeds up the
134
-	// allocation of those list a lot when they have a lot of arguments
135
-	cursor := ast
136
-	var n int
137
-	for cursor.Next != nil {
138
-		cursor = cursor.Next
139
-		n++
140
-	}
141
-	msgList := make([]string, n)
142
-
143
-	var i int
132
+	msgList := initMsgList(ast)
144 133
 	// Append build args to runConfig environment variables
145 134
 	envs := append(b.runConfig.Env, b.buildArgsWithoutConfigEnv()...)
146 135
 
147
-	for ast.Next != nil {
136
+	for i := 0; ast.Next != nil; i++ {
148 137
 		ast = ast.Next
149
-		var str string
150
-		str = ast.Value
151
-		if replaceEnvAllowed[cmd] {
152
-			var err error
153
-			var words []string
154
-
155
-			if allowWordExpansion[cmd] {
156
-				words, err = ProcessWords(str, envs, b.directive.EscapeToken)
157
-				if err != nil {
158
-					return err
159
-				}
160
-				strList = append(strList, words...)
161
-			} else {
162
-				str, err = ProcessWord(str, envs, b.directive.EscapeToken)
163
-				if err != nil {
164
-					return err
165
-				}
166
-				strList = append(strList, str)
167
-			}
168
-		} else {
169
-			strList = append(strList, str)
138
+		words, err := b.evaluateEnv(cmd, ast.Value, envs)
139
+		if err != nil {
140
+			return err
170 141
 		}
142
+		strList = append(strList, words...)
171 143
 		msgList[i] = ast.Value
172
-		i++
173 144
 	}
174 145
 
175 146
 	msg += " " + strings.Join(msgList, " ")
... ...
@@ -186,6 +157,29 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error {
186 186
 	return fmt.Errorf("Unknown instruction: %s", upperCasedCmd)
187 187
 }
188 188
 
189
+// count the number of nodes that we are going to traverse first
190
+// allocation of those list a lot when they have a lot of arguments
191
+func initMsgList(cursor *parser.Node) []string {
192
+	var n int
193
+	for ; cursor.Next != nil; n++ {
194
+		cursor = cursor.Next
195
+	}
196
+	return make([]string, n)
197
+}
198
+
199
+func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string, error) {
200
+	if !replaceEnvAllowed[cmd] {
201
+		return []string{str}, nil
202
+	}
203
+	var processFunc func(string, []string, rune) ([]string, error)
204
+	if allowWordExpansion[cmd] {
205
+		processFunc = ProcessWords
206
+	} else {
207
+		processFunc = ProcessWord
208
+	}
209
+	return processFunc(str, envs, b.directive.EscapeToken)
210
+}
211
+
189 212
 // buildArgsWithoutConfigEnv returns a list of key=value pairs for all the build
190 213
 // args that are not overriden by runConfig environment variables.
191 214
 func (b *Builder) buildArgsWithoutConfigEnv() []string {
... ...
@@ -220,7 +220,9 @@ func NodeFromLabels(labels map[string]string) *Node {
220 220
 	for _, key := range keys {
221 221
 		value := labels[key]
222 222
 		labelPairs = append(labelPairs, fmt.Sprintf("%q='%s'", key, value))
223
-		node := newKeyValueNode(key, value)
223
+		// Value must be single quoted to prevent env variable expansion
224
+		// See https://github.com/docker/docker/issues/26027
225
+		node := newKeyValueNode(key, "'"+value+"'")
224 226
 		rootNode, prevNode = appendKeyValueNode(node, rootNode, prevNode)
225 227
 	}
226 228
 
... ...
@@ -40,19 +40,19 @@ func TestParseNameValNewFormat(t *testing.T) {
40 40
 func TestNodeFromLabels(t *testing.T) {
41 41
 	labels := map[string]string{
42 42
 		"foo":   "bar",
43
-		"weird": "'first second'",
43
+		"weird": "first' second",
44 44
 	}
45 45
 	expected := &Node{
46 46
 		Value:    "label",
47
-		Original: `LABEL "foo"='bar' "weird"=''first second''`,
47
+		Original: `LABEL "foo"='bar' "weird"='first' second'`,
48 48
 		Next: &Node{
49 49
 			Value: "foo",
50 50
 			Next: &Node{
51
-				Value: "bar",
51
+				Value: "'bar'",
52 52
 				Next: &Node{
53 53
 					Value: "weird",
54 54
 					Next: &Node{
55
-						Value: "'first second'",
55
+						Value: "'first' second'",
56 56
 					},
57 57
 				},
58 58
 			},
... ...
@@ -24,16 +24,9 @@ type shellWord struct {
24 24
 
25 25
 // ProcessWord will use the 'env' list of environment variables,
26 26
 // and replace any env var references in 'word'.
27
-func ProcessWord(word string, env []string, escapeToken rune) (string, error) {
28
-	sw := &shellWord{
29
-		word:        word,
30
-		envs:        env,
31
-		pos:         0,
32
-		escapeToken: escapeToken,
33
-	}
34
-	sw.scanner.Init(strings.NewReader(word))
35
-	word, _, err := sw.process()
36
-	return word, err
27
+func ProcessWord(word string, env []string, escapeToken rune) ([]string, error) {
28
+	word, _, err := process(word, env, escapeToken)
29
+	return []string{word}, err
37 30
 }
38 31
 
39 32
 // ProcessWords will use the 'env' list of environment variables,
... ...
@@ -44,6 +37,11 @@ func ProcessWord(word string, env []string, escapeToken rune) (string, error) {
44 44
 // Note, each one is trimmed to remove leading and trailing spaces (unless
45 45
 // they are quoted", but ProcessWord retains spaces between words.
46 46
 func ProcessWords(word string, env []string, escapeToken rune) ([]string, error) {
47
+	_, words, err := process(word, env, escapeToken)
48
+	return words, err
49
+}
50
+
51
+func process(word string, env []string, escapeToken rune) (string, []string, error) {
47 52
 	sw := &shellWord{
48 53
 		word:        word,
49 54
 		envs:        env,
... ...
@@ -51,8 +49,7 @@ func ProcessWords(word string, env []string, escapeToken rune) ([]string, error)
51 51
 		escapeToken: escapeToken,
52 52
 	}
53 53
 	sw.scanner.Init(strings.NewReader(word))
54
-	_, words, err := sw.process()
55
-	return words, err
54
+	return sw.process()
56 55
 }
57 56
 
58 57
 func (sw *shellWord) process() (string, []string, error) {
... ...
@@ -6,6 +6,8 @@ import (
6 6
 	"runtime"
7 7
 	"strings"
8 8
 	"testing"
9
+
10
+	"github.com/docker/docker/pkg/testutil/assert"
9 11
 )
10 12
 
11 13
 func TestShellParser4EnvVars(t *testing.T) {
... ...
@@ -13,9 +15,7 @@ func TestShellParser4EnvVars(t *testing.T) {
13 13
 	lineCount := 0
14 14
 
15 15
 	file, err := os.Open(fn)
16
-	if err != nil {
17
-		t.Fatalf("Can't open '%s': %s", err, fn)
18
-	}
16
+	assert.NilError(t, err)
19 17
 	defer file.Close()
20 18
 
21 19
 	scanner := bufio.NewScanner(file)
... ...
@@ -36,29 +36,25 @@ func TestShellParser4EnvVars(t *testing.T) {
36 36
 		}
37 37
 
38 38
 		words := strings.Split(line, "|")
39
-		if len(words) != 3 {
40
-			t.Fatalf("Error in '%s' - should be exactly one | in:%q", fn, line)
41
-		}
39
+		assert.Equal(t, len(words), 3)
42 40
 
43
-		words[0] = strings.TrimSpace(words[0])
44
-		words[1] = strings.TrimSpace(words[1])
45
-		words[2] = strings.TrimSpace(words[2])
41
+		platform := strings.TrimSpace(words[0])
42
+		source := strings.TrimSpace(words[1])
43
+		expected := strings.TrimSpace(words[2])
46 44
 
47 45
 		// Key W=Windows; A=All; U=Unix
48
-		if (words[0] != "W") && (words[0] != "A") && (words[0] != "U") {
49
-			t.Fatalf("Invalid tag %s at line %d of %s. Must be W, A or U", words[0], lineCount, fn)
46
+		if platform != "W" && platform != "A" && platform != "U" {
47
+			t.Fatalf("Invalid tag %s at line %d of %s. Must be W, A or U", platform, lineCount, fn)
50 48
 		}
51 49
 
52
-		if ((words[0] == "W" || words[0] == "A") && runtime.GOOS == "windows") ||
53
-			((words[0] == "U" || words[0] == "A") && runtime.GOOS != "windows") {
54
-			newWord, err := ProcessWord(words[1], envs, '\\')
55
-
56
-			if err != nil {
57
-				newWord = "error"
58
-			}
59
-
60
-			if newWord != words[2] {
61
-				t.Fatalf("Error. Src: %s  Calc: %s  Expected: %s at line %d", words[1], newWord, words[2], lineCount)
50
+		if ((platform == "W" || platform == "A") && runtime.GOOS == "windows") ||
51
+			((platform == "U" || platform == "A") && runtime.GOOS != "windows") {
52
+			newWord, err := ProcessWord(source, envs, '\\')
53
+			if expected == "error" {
54
+				assert.Error(t, err, "")
55
+			} else {
56
+				assert.NilError(t, err)
57
+				assert.DeepEqual(t, newWord, []string{expected})
62 58
 			}
63 59
 		}
64 60
 	}