From 434d77bc0edf21b970e0414ee54112768a490aaa Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 13 Mar 2017 17:25:37 -0400 Subject: [PATCH] Fix --label being env var expanded. Signed-off-by: Daniel Nephin --- builder/dockerfile/evaluator.go | 64 +++++++++---------- builder/dockerfile/parser/line_parsers.go | 4 +- .../dockerfile/parser/line_parsers_test.go | 8 +-- builder/dockerfile/shell_parser.go | 21 +++--- builder/dockerfile/shell_parser_test.go | 38 +++++------ 5 files changed, 62 insertions(+), 73 deletions(-) diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index d0f9a7ca55..08c05daf8f 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -129,47 +129,18 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error { } - // count the number of nodes that we are going to traverse first - // so we can pre-create the argument and message array. This speeds up the - // allocation of those list a lot when they have a lot of arguments - cursor := ast - var n int - for cursor.Next != nil { - cursor = cursor.Next - n++ - } - msgList := make([]string, n) - - var i int + msgList := initMsgList(ast) // Append build args to runConfig environment variables envs := append(b.runConfig.Env, b.buildArgsWithoutConfigEnv()...) - for ast.Next != nil { + for i := 0; ast.Next != nil; i++ { ast = ast.Next - var str string - str = ast.Value - if replaceEnvAllowed[cmd] { - var err error - var words []string - - if allowWordExpansion[cmd] { - words, err = ProcessWords(str, envs, b.directive.EscapeToken) - if err != nil { - return err - } - strList = append(strList, words...) - } else { - str, err = ProcessWord(str, envs, b.directive.EscapeToken) - if err != nil { - return err - } - strList = append(strList, str) - } - } else { - strList = append(strList, str) + words, err := b.evaluateEnv(cmd, ast.Value, envs) + if err != nil { + return err } + strList = append(strList, words...) msgList[i] = ast.Value - i++ } msg += " " + strings.Join(msgList, " ") @@ -186,6 +157,29 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error { return fmt.Errorf("Unknown instruction: %s", upperCasedCmd) } +// count the number of nodes that we are going to traverse first +// allocation of those list a lot when they have a lot of arguments +func initMsgList(cursor *parser.Node) []string { + var n int + for ; cursor.Next != nil; n++ { + cursor = cursor.Next + } + return make([]string, n) +} + +func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string, error) { + if !replaceEnvAllowed[cmd] { + return []string{str}, nil + } + var processFunc func(string, []string, rune) ([]string, error) + if allowWordExpansion[cmd] { + processFunc = ProcessWords + } else { + processFunc = ProcessWord + } + return processFunc(str, envs, b.directive.EscapeToken) +} + // buildArgsWithoutConfigEnv returns a list of key=value pairs for all the build // args that are not overriden by runConfig environment variables. func (b *Builder) buildArgsWithoutConfigEnv() []string { diff --git a/builder/dockerfile/parser/line_parsers.go b/builder/dockerfile/parser/line_parsers.go index 4efae66823..0334834e62 100644 --- a/builder/dockerfile/parser/line_parsers.go +++ b/builder/dockerfile/parser/line_parsers.go @@ -220,7 +220,9 @@ func NodeFromLabels(labels map[string]string) *Node { for _, key := range keys { value := labels[key] labelPairs = append(labelPairs, fmt.Sprintf("%q='%s'", key, value)) - node := newKeyValueNode(key, value) + // Value must be single quoted to prevent env variable expansion + // See https://github.com/docker/docker/issues/26027 + node := newKeyValueNode(key, "'"+value+"'") rootNode, prevNode = appendKeyValueNode(node, rootNode, prevNode) } diff --git a/builder/dockerfile/parser/line_parsers_test.go b/builder/dockerfile/parser/line_parsers_test.go index 5f762de422..30b6bdd824 100644 --- a/builder/dockerfile/parser/line_parsers_test.go +++ b/builder/dockerfile/parser/line_parsers_test.go @@ -40,19 +40,19 @@ func TestParseNameValNewFormat(t *testing.T) { func TestNodeFromLabels(t *testing.T) { labels := map[string]string{ "foo": "bar", - "weird": "'first second'", + "weird": "first' second", } expected := &Node{ Value: "label", - Original: `LABEL "foo"='bar' "weird"=''first second''`, + Original: `LABEL "foo"='bar' "weird"='first' second'`, Next: &Node{ Value: "foo", Next: &Node{ - Value: "bar", + Value: "'bar'", Next: &Node{ Value: "weird", Next: &Node{ - Value: "'first second'", + Value: "'first' second'", }, }, }, diff --git a/builder/dockerfile/shell_parser.go b/builder/dockerfile/shell_parser.go index 189afd1fdb..2a8b441134 100644 --- a/builder/dockerfile/shell_parser.go +++ b/builder/dockerfile/shell_parser.go @@ -24,16 +24,9 @@ type shellWord struct { // ProcessWord will use the 'env' list of environment variables, // and replace any env var references in 'word'. -func ProcessWord(word string, env []string, escapeToken rune) (string, error) { - sw := &shellWord{ - word: word, - envs: env, - pos: 0, - escapeToken: escapeToken, - } - sw.scanner.Init(strings.NewReader(word)) - word, _, err := sw.process() - return word, err +func ProcessWord(word string, env []string, escapeToken rune) ([]string, error) { + word, _, err := process(word, env, escapeToken) + return []string{word}, err } // ProcessWords will use the 'env' list of environment variables, @@ -44,6 +37,11 @@ func ProcessWord(word string, env []string, escapeToken rune) (string, error) { // Note, each one is trimmed to remove leading and trailing spaces (unless // they are quoted", but ProcessWord retains spaces between words. func ProcessWords(word string, env []string, escapeToken rune) ([]string, error) { + _, words, err := process(word, env, escapeToken) + return words, err +} + +func process(word string, env []string, escapeToken rune) (string, []string, error) { sw := &shellWord{ word: word, envs: env, @@ -51,8 +49,7 @@ func ProcessWords(word string, env []string, escapeToken rune) ([]string, error) escapeToken: escapeToken, } sw.scanner.Init(strings.NewReader(word)) - _, words, err := sw.process() - return words, err + return sw.process() } func (sw *shellWord) process() (string, []string, error) { diff --git a/builder/dockerfile/shell_parser_test.go b/builder/dockerfile/shell_parser_test.go index ca4728d798..0712fde675 100644 --- a/builder/dockerfile/shell_parser_test.go +++ b/builder/dockerfile/shell_parser_test.go @@ -6,6 +6,8 @@ import ( "runtime" "strings" "testing" + + "github.com/docker/docker/pkg/testutil/assert" ) func TestShellParser4EnvVars(t *testing.T) { @@ -13,9 +15,7 @@ func TestShellParser4EnvVars(t *testing.T) { lineCount := 0 file, err := os.Open(fn) - if err != nil { - t.Fatalf("Can't open '%s': %s", err, fn) - } + assert.NilError(t, err) defer file.Close() scanner := bufio.NewScanner(file) @@ -36,29 +36,25 @@ func TestShellParser4EnvVars(t *testing.T) { } words := strings.Split(line, "|") - if len(words) != 3 { - t.Fatalf("Error in '%s' - should be exactly one | in:%q", fn, line) - } + assert.Equal(t, len(words), 3) - words[0] = strings.TrimSpace(words[0]) - words[1] = strings.TrimSpace(words[1]) - words[2] = strings.TrimSpace(words[2]) + platform := strings.TrimSpace(words[0]) + source := strings.TrimSpace(words[1]) + expected := strings.TrimSpace(words[2]) // Key W=Windows; A=All; U=Unix - if (words[0] != "W") && (words[0] != "A") && (words[0] != "U") { - t.Fatalf("Invalid tag %s at line %d of %s. Must be W, A or U", words[0], lineCount, fn) + if platform != "W" && platform != "A" && platform != "U" { + t.Fatalf("Invalid tag %s at line %d of %s. Must be W, A or U", platform, lineCount, fn) } - if ((words[0] == "W" || words[0] == "A") && runtime.GOOS == "windows") || - ((words[0] == "U" || words[0] == "A") && runtime.GOOS != "windows") { - newWord, err := ProcessWord(words[1], envs, '\\') - - if err != nil { - newWord = "error" - } - - if newWord != words[2] { - t.Fatalf("Error. Src: %s Calc: %s Expected: %s at line %d", words[1], newWord, words[2], lineCount) + if ((platform == "W" || platform == "A") && runtime.GOOS == "windows") || + ((platform == "U" || platform == "A") && runtime.GOOS != "windows") { + newWord, err := ProcessWord(source, envs, '\\') + if expected == "error" { + assert.Error(t, err, "") + } else { + assert.NilError(t, err) + assert.DeepEqual(t, newWord, []string{expected}) } } }