diff --git a/builder/dockerfile/words b/builder/dockerfile/envVarTest similarity index 100% rename from builder/dockerfile/words rename to builder/dockerfile/envVarTest diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index a27018915f..d6e628b8d6 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -42,6 +42,19 @@ var replaceEnvAllowed = map[string]struct{}{ command.Arg: {}, } +// Certain commands are allowed to have their args split into more +// words after env var replacements. Meaning: +// ENV foo="123 456" +// EXPOSE $foo +// should result in the same thing as: +// EXPOSE 123 456 +// and not treat "123 456" as a single word. +// Note that: EXPOSE "$foo" and EXPOSE $foo are not the same thing. +// Quotes will cause it to still be treated as single word. +var allowWordExpansion = map[string]bool{ + command.Expose: true, +} + var evaluateTable map[string]func(*Builder, []string, map[string]bool, string) error func init() { @@ -92,7 +105,7 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { attrs := ast.Attributes original := ast.Original flags := ast.Flags - strs := []string{} + strList := []string{} msg := fmt.Sprintf("Step %d : %s", stepN+1, upperCasedCmd) if len(ast.Flags) > 0 { @@ -104,7 +117,7 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { return fmt.Errorf("ONBUILD requires at least one argument") } ast = ast.Next.Children[0] - strs = append(strs, ast.Value) + strList = append(strList, ast.Value) msg += " " + ast.Value if len(ast.Flags) > 0 { @@ -122,9 +135,6 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { cursor = cursor.Next n++ } - l := len(strs) - strList := make([]string, n+l) - copy(strList, strs) msgList := make([]string, n) var i int @@ -155,12 +165,24 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { str = ast.Value if _, ok := replaceEnvAllowed[cmd]; ok { var err error - str, err = ProcessWord(ast.Value, envs) - if err != nil { - return err + var words []string + + if _, ok := allowWordExpansion[cmd]; ok { + words, err = ProcessWords(str, envs) + if err != nil { + return err + } + strList = append(strList, words...) + } else { + str, err = ProcessWord(str, envs) + if err != nil { + return err + } + strList = append(strList, str) } + } else { + strList = append(strList, str) } - strList[i+l] = str msgList[i] = ast.Value i++ } diff --git a/builder/dockerfile/shell_parser.go b/builder/dockerfile/shell_parser.go index 7425ba4e86..c714266778 100644 --- a/builder/dockerfile/shell_parser.go +++ b/builder/dockerfile/shell_parser.go @@ -29,17 +29,85 @@ func ProcessWord(word string, env []string) (string, error) { pos: 0, } sw.scanner.Init(strings.NewReader(word)) - return sw.process() + word, _, err := sw.process() + return word, err } -func (sw *shellWord) process() (string, error) { +// ProcessWords will use the 'env' list of environment variables, +// and replace any env var references in 'word' then it will also +// return a slice of strings which represents the 'word' +// split up based on spaces - taking into account quotes. Note that +// this splitting is done **after** the env var substitutions are done. +// 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) ([]string, error) { + sw := &shellWord{ + word: word, + envs: env, + pos: 0, + } + sw.scanner.Init(strings.NewReader(word)) + _, words, err := sw.process() + return words, err +} + +func (sw *shellWord) process() (string, []string, error) { return sw.processStopOn(scanner.EOF) } +type wordsStruct struct { + word string + words []string + inWord bool +} + +func (w *wordsStruct) addChar(ch rune) { + if unicode.IsSpace(ch) && w.inWord { + if len(w.word) != 0 { + w.words = append(w.words, w.word) + w.word = "" + w.inWord = false + } + } else if !unicode.IsSpace(ch) { + w.addRawChar(ch) + } +} + +func (w *wordsStruct) addRawChar(ch rune) { + w.word += string(ch) + w.inWord = true +} + +func (w *wordsStruct) addString(str string) { + var scan scanner.Scanner + scan.Init(strings.NewReader(str)) + for scan.Peek() != scanner.EOF { + w.addChar(scan.Next()) + } +} + +func (w *wordsStruct) addRawString(str string) { + w.word += str + w.inWord = true +} + +func (w *wordsStruct) getWords() []string { + if len(w.word) > 0 { + w.words = append(w.words, w.word) + + // Just in case we're called again by mistake + w.word = "" + w.inWord = false + } + return w.words +} + // Process the word, starting at 'pos', and stop when we get to the // end of the word or the 'stopChar' character -func (sw *shellWord) processStopOn(stopChar rune) (string, error) { +func (sw *shellWord) processStopOn(stopChar rune) (string, []string, error) { var result string + var words wordsStruct + var charFuncMapping = map[rune]func() (string, error){ '\'': sw.processSingleQuote, '"': sw.processDoubleQuote, @@ -57,9 +125,15 @@ func (sw *shellWord) processStopOn(stopChar rune) (string, error) { // Call special processing func for certain chars tmp, err := fn() if err != nil { - return "", err + return "", []string{}, err } result += tmp + + if ch == rune('$') { + words.addString(tmp) + } else { + words.addRawString(tmp) + } } else { // Not special, just add it to the result ch = sw.scanner.Next() @@ -73,13 +147,16 @@ func (sw *shellWord) processStopOn(stopChar rune) (string, error) { break } + words.addRawChar(ch) + } else { + words.addChar(ch) } result += string(ch) } } - return result, nil + return result, words.getWords(), nil } func (sw *shellWord) processSingleQuote() (string, error) { @@ -160,7 +237,7 @@ func (sw *shellWord) processDollar() (string, error) { sw.scanner.Next() // skip over : modifier := sw.scanner.Next() - word, err := sw.processStopOn('}') + word, _, err := sw.processStopOn('}') if err != nil { return "", err } diff --git a/builder/dockerfile/shell_parser_test.go b/builder/dockerfile/shell_parser_test.go index ed6a1d109c..81ac591e99 100644 --- a/builder/dockerfile/shell_parser_test.go +++ b/builder/dockerfile/shell_parser_test.go @@ -7,10 +7,12 @@ import ( "testing" ) -func TestShellParser(t *testing.T) { - file, err := os.Open("words") +func TestShellParser4EnvVars(t *testing.T) { + fn := "envVarTest" + + file, err := os.Open(fn) if err != nil { - t.Fatalf("Can't open 'words': %s", err) + t.Fatalf("Can't open '%s': %s", err, fn) } defer file.Close() @@ -32,7 +34,7 @@ func TestShellParser(t *testing.T) { words := strings.Split(line, "|") if len(words) != 2 { - t.Fatalf("Error in 'words' - should be 2 words:%q", words) + t.Fatalf("Error in '%s' - should be exactly one | in:%q", fn, line) } words[0] = strings.TrimSpace(words[0]) @@ -50,6 +52,54 @@ func TestShellParser(t *testing.T) { } } +func TestShellParser4Words(t *testing.T) { + fn := "wordsTest" + + file, err := os.Open(fn) + if err != nil { + t.Fatalf("Can't open '%s': %s", err, fn) + } + defer file.Close() + + envs := []string{} + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := scanner.Text() + + if strings.HasPrefix(line, "#") { + continue + } + + if strings.HasPrefix(line, "ENV ") { + line = strings.TrimLeft(line[3:], " ") + envs = append(envs, line) + continue + } + + words := strings.Split(line, "|") + if len(words) != 2 { + t.Fatalf("Error in '%s' - should be exactly one | in: %q", fn, line) + } + test := strings.TrimSpace(words[0]) + expected := strings.Split(strings.TrimLeft(words[1], " "), ",") + + result, err := ProcessWords(test, envs) + + if err != nil { + result = []string{"error"} + } + + if len(result) != len(expected) { + t.Fatalf("Error. %q was suppose to result in %q, but got %q instead", test, expected, result) + } + for i, w := range expected { + if w != result[i] { + t.Fatalf("Error. %q was suppose to result in %q, but got %q instead", test, expected, result) + } + } + } +} + func TestGetEnv(t *testing.T) { sw := &shellWord{ word: "", diff --git a/builder/dockerfile/wordsTest b/builder/dockerfile/wordsTest new file mode 100644 index 0000000000..fa916c67f9 --- /dev/null +++ b/builder/dockerfile/wordsTest @@ -0,0 +1,25 @@ +hello | hello +hello${hi}bye | hellobye +ENV hi=hi +hello${hi}bye | hellohibye +ENV space=abc def +hello${space}bye | helloabc,defbye +hello"${space}"bye | helloabc defbye +hello "${space}"bye | hello,abc defbye +ENV leading= ab c +hello${leading}def | hello,ab,cdef +hello"${leading}" def | hello ab c,def +hello"${leading}" | hello ab c +hello${leading} | hello,ab,c +# next line MUST have 3 trailing spaces, don't erase them! +ENV trailing=ab c +hello${trailing} | helloab,c +hello${trailing}d | helloab,c,d +hello"${trailing}"d | helloab c d +# next line MUST have 3 trailing spaces, don't erase them! +hel"lo${trailing}" | helloab c +hello" there " | hello there +hello there | hello,there +hello\ there | hello there +hello" there | hello there +hello\" there | hello",there diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 7053fb4fd7..49fa09aad5 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -150,6 +150,8 @@ func (s *DockerSuite) TestBuildEnvironmentReplacementExpose(c *check.C) { FROM scratch ENV port 80 EXPOSE ${port} + ENV ports " 99 100 " + EXPOSE ${ports} `, true) if err != nil { c.Fatal(err) @@ -166,8 +168,13 @@ func (s *DockerSuite) TestBuildEnvironmentReplacementExpose(c *check.C) { c.Fatal(err) } - if _, ok := exposedPorts["80/tcp"]; !ok { - c.Fatal("Exposed port 80 from environment not in Config.ExposedPorts on image") + exp := []int{80, 99, 100} + + for _, p := range exp { + tmp := fmt.Sprintf("%d/tcp", p) + if _, ok := exposedPorts[tmp]; !ok { + c.Fatalf("Exposed port %d from environment not in Config.ExposedPorts on image", p) + } } }