From 6d66e3e7a5ecb021a9e89c4f85fadecf23e2000c Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Wed, 28 Jan 2015 18:28:48 -0800 Subject: [PATCH] Fix some escaping around env var processing Clarify in the docs that ENV is not recursive Closes #10391 Signed-off-by: Doug Davis --- builder/evaluator.go | 6 +- builder/parser/line_parsers.go | 15 +- builder/parser/testfiles/env/Dockerfile | 8 + builder/parser/testfiles/env/result | 18 +- builder/shell_parser.go | 209 +++++++++++++++++++++++ builder/shell_parser_test.go | 51 ++++++ builder/support.go | 41 ----- builder/words | 43 +++++ docs/sources/reference/builder.md | 11 ++ integration-cli/docker_cli_build_test.go | 66 +++++-- 10 files changed, 402 insertions(+), 66 deletions(-) create mode 100644 builder/shell_parser.go create mode 100644 builder/shell_parser_test.go create mode 100644 builder/words diff --git a/builder/evaluator.go b/builder/evaluator.go index 985656f16a..ba2726084d 100644 --- a/builder/evaluator.go +++ b/builder/evaluator.go @@ -302,7 +302,11 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { var str string str = ast.Value if _, ok := replaceEnvAllowed[cmd]; ok { - str = b.replaceEnv(ast.Value) + var err error + str, err = ProcessWord(ast.Value, b.Config.Env) + if err != nil { + return err + } } strList[i+l] = str msgList[i] = ast.Value diff --git a/builder/parser/line_parsers.go b/builder/parser/line_parsers.go index 45c929ee69..3026a0b06d 100644 --- a/builder/parser/line_parsers.go +++ b/builder/parser/line_parsers.go @@ -90,7 +90,7 @@ func parseNameVal(rest string, key string) (*Node, map[string]bool, error) { if blankOK || len(word) > 0 { words = append(words, word) - // Look for = and if no there assume + // Look for = and if not there assume // we're doing the old stuff and // just read the rest of the line if !strings.Contains(word, "=") { @@ -107,12 +107,15 @@ func parseNameVal(rest string, key string) (*Node, map[string]bool, error) { quote = ch blankOK = true phase = inQuote - continue } if ch == '\\' { if pos+1 == len(rest) { continue // just skip \ at end } + // If we're not quoted and we see a \, then always just + // add \ plus the char to the word, even if the char + // is a quote. + word += string(ch) pos++ ch = rune(rest[pos]) } @@ -122,15 +125,17 @@ func parseNameVal(rest string, key string) (*Node, map[string]bool, error) { if phase == inQuote { if ch == quote { phase = inWord - continue } - if ch == '\\' { + // \ is special except for ' quotes - can't escape anything for ' + if ch == '\\' && quote != '\'' { if pos+1 == len(rest) { phase = inWord continue // just skip \ at end } pos++ - ch = rune(rest[pos]) + nextCh := rune(rest[pos]) + word += string(ch) + ch = nextCh } word += string(ch) } diff --git a/builder/parser/testfiles/env/Dockerfile b/builder/parser/testfiles/env/Dockerfile index bb78503cce..08fa18acec 100644 --- a/builder/parser/testfiles/env/Dockerfile +++ b/builder/parser/testfiles/env/Dockerfile @@ -7,6 +7,14 @@ ENV name=value\ value2 ENV name="value'quote space'value2" ENV name='value"double quote"value2' ENV name=value\ value2 name2=value2\ value3 +ENV name="a\"b" +ENV name="a\'b" +ENV name='a\'b' +ENV name='a\'b'' +ENV name='a\"b' +ENV name="''" +# don't put anything after the next line - it must be the last line of the +# Dockerfile and it must end with \ ENV name=value \ name1=value1 \ name2="value2a \ diff --git a/builder/parser/testfiles/env/result b/builder/parser/testfiles/env/result index a473d0fa39..ba0a6dd7cb 100644 --- a/builder/parser/testfiles/env/result +++ b/builder/parser/testfiles/env/result @@ -2,9 +2,15 @@ (env "name" "value") (env "name" "value") (env "name" "value" "name2" "value2") -(env "name" "value value1") -(env "name" "value value2") -(env "name" "value'quote space'value2") -(env "name" "value\"double quote\"value2") -(env "name" "value value2" "name2" "value2 value3") -(env "name" "value" "name1" "value1" "name2" "value2a value2b" "name3" "value3an\"value3b\"" "name4" "value4a\\nvalue4b") +(env "name" "\"value value1\"") +(env "name" "value\\ value2") +(env "name" "\"value'quote space'value2\"") +(env "name" "'value\"double quote\"value2'") +(env "name" "value\\ value2" "name2" "value2\\ value3") +(env "name" "\"a\\\"b\"") +(env "name" "\"a\\'b\"") +(env "name" "'a\\'b'") +(env "name" "'a\\'b''") +(env "name" "'a\\\"b'") +(env "name" "\"''\"") +(env "name" "value" "name1" "value1" "name2" "\"value2a value2b\"" "name3" "\"value3a\\n\\\"value3b\\\"\"" "name4" "\"value4a\\\\nvalue4b\"") diff --git a/builder/shell_parser.go b/builder/shell_parser.go new file mode 100644 index 0000000000..b8c746773c --- /dev/null +++ b/builder/shell_parser.go @@ -0,0 +1,209 @@ +package builder + +// This will take a single word and an array of env variables and +// process all quotes (" and ') as well as $xxx and ${xxx} env variable +// tokens. Tries to mimic bash shell process. +// It doesn't support all flavors of ${xx:...} formats but new ones can +// be added by adding code to the "special ${} format processing" section + +import ( + "fmt" + "strings" + "unicode" +) + +type shellWord struct { + word string + envs []string + pos int +} + +func ProcessWord(word string, env []string) (string, error) { + sw := &shellWord{ + word: word, + envs: env, + pos: 0, + } + return sw.process() +} + +func (sw *shellWord) process() (string, error) { + return sw.processStopOn('\000') +} + +// 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) { + var result string + var charFuncMapping = map[rune]func() (string, error){ + '\'': sw.processSingleQuote, + '"': sw.processDoubleQuote, + '$': sw.processDollar, + } + + for sw.pos < len(sw.word) { + ch := sw.peek() + if stopChar != '\000' && ch == stopChar { + sw.next() + break + } + if fn, ok := charFuncMapping[ch]; ok { + // Call special processing func for certain chars + tmp, err := fn() + if err != nil { + return "", err + } + result += tmp + } else { + // Not special, just add it to the result + ch = sw.next() + if ch == '\\' { + // '\' escapes, except end of line + ch = sw.next() + if ch == '\000' { + continue + } + } + result += string(ch) + } + } + + return result, nil +} + +func (sw *shellWord) peek() rune { + if sw.pos == len(sw.word) { + return '\000' + } + return rune(sw.word[sw.pos]) +} + +func (sw *shellWord) next() rune { + if sw.pos == len(sw.word) { + return '\000' + } + ch := rune(sw.word[sw.pos]) + sw.pos++ + return ch +} + +func (sw *shellWord) processSingleQuote() (string, error) { + // All chars between single quotes are taken as-is + // Note, you can't escape ' + var result string + + sw.next() + + for { + ch := sw.next() + if ch == '\000' || ch == '\'' { + break + } + result += string(ch) + } + return result, nil +} + +func (sw *shellWord) processDoubleQuote() (string, error) { + // All chars up to the next " are taken as-is, even ', except any $ chars + // But you can escape " with a \ + var result string + + sw.next() + + for sw.pos < len(sw.word) { + ch := sw.peek() + if ch == '"' { + sw.next() + break + } + if ch == '$' { + tmp, err := sw.processDollar() + if err != nil { + return "", err + } + result += tmp + } else { + ch = sw.next() + if ch == '\\' { + chNext := sw.peek() + + if chNext == '\000' { + // Ignore \ at end of word + continue + } + + if chNext == '"' || chNext == '$' { + // \" and \$ can be escaped, all other \'s are left as-is + ch = sw.next() + } + } + result += string(ch) + } + } + + return result, nil +} + +func (sw *shellWord) processDollar() (string, error) { + sw.next() + ch := sw.peek() + if ch == '{' { + sw.next() + name := sw.processName() + ch = sw.peek() + if ch == '}' { + // Normal ${xx} case + sw.next() + return sw.getEnv(name), nil + } + return "", fmt.Errorf("Unsupported ${} substitution: %s", sw.word) + } else { + // $xxx case + name := sw.processName() + if name == "" { + return "$", nil + } + return sw.getEnv(name), nil + } +} + +func (sw *shellWord) processName() string { + // Read in a name (alphanumeric or _) + // If it starts with a numeric then just return $# + var name string + + for sw.pos < len(sw.word) { + ch := sw.peek() + if len(name) == 0 && unicode.IsDigit(ch) { + ch = sw.next() + return string(ch) + } + if !unicode.IsLetter(ch) && !unicode.IsDigit(ch) && ch != '_' { + break + } + ch = sw.next() + name += string(ch) + } + + return name +} + +func (sw *shellWord) getEnv(name string) string { + for _, env := range sw.envs { + i := strings.Index(env, "=") + if i < 0 { + if name == env { + // Should probably never get here, but just in case treat + // it like "var" and "var=" are the same + return "" + } + continue + } + if name != env[:i] { + continue + } + return env[i+1:] + } + return "" +} diff --git a/builder/shell_parser_test.go b/builder/shell_parser_test.go new file mode 100644 index 0000000000..79260492f3 --- /dev/null +++ b/builder/shell_parser_test.go @@ -0,0 +1,51 @@ +package builder + +import ( + "bufio" + "os" + "strings" + "testing" +) + +func TestShellParser(t *testing.T) { + file, err := os.Open("words") + if err != nil { + t.Fatalf("Can't open 'words': %s", err) + } + defer file.Close() + + scanner := bufio.NewScanner(file) + envs := []string{"PWD=/home", "SHELL=bash"} + for scanner.Scan() { + line := scanner.Text() + + // Trim comments and blank lines + i := strings.Index(line, "#") + if i >= 0 { + line = line[:i] + } + line = strings.TrimSpace(line) + + if line == "" { + continue + } + + words := strings.Split(line, "|") + if len(words) != 2 { + t.Fatalf("Error in 'words' - should be 2 words:%q", words) + } + + words[0] = strings.TrimSpace(words[0]) + words[1] = strings.TrimSpace(words[1]) + + newWord, err := ProcessWord(words[0], envs) + + if err != nil { + newWord = "error" + } + + if newWord != words[1] { + t.Fatalf("Error. Src: %s Calc: %s Expected: %s", words[0], newWord, words[1]) + } + } +} diff --git a/builder/support.go b/builder/support.go index 6833457f3a..787ff10ccb 100644 --- a/builder/support.go +++ b/builder/support.go @@ -1,50 +1,9 @@ package builder import ( - "regexp" "strings" ) -var ( - // `\\\\+|[^\\]|\b|\A` - match any number of "\\" (ie, properly-escaped backslashes), or a single non-backslash character, or a word boundary, or beginning-of-line - // `\$` - match literal $ - // `[[:alnum:]_]+` - match things like `$SOME_VAR` - // `{[[:alnum:]_]+}` - match things like `${SOME_VAR}` - tokenEnvInterpolation = regexp.MustCompile(`(\\|\\\\+|[^\\]|\b|\A)\$([[:alnum:]_]+|{[[:alnum:]_]+})`) - // this intentionally punts on more exotic interpolations like ${SOME_VAR%suffix} and lets the shell handle those directly -) - -// handle environment replacement. Used in dispatcher. -func (b *Builder) replaceEnv(str string) string { - for _, match := range tokenEnvInterpolation.FindAllString(str, -1) { - idx := strings.Index(match, "\\$") - if idx != -1 { - if idx+2 >= len(match) { - str = strings.Replace(str, match, "\\$", -1) - continue - } - - prefix := match[:idx] - stripped := match[idx+2:] - str = strings.Replace(str, match, prefix+"$"+stripped, -1) - continue - } - - match = match[strings.Index(match, "$"):] - matchKey := strings.Trim(match, "${}") - - for _, keyval := range b.Config.Env { - tmp := strings.SplitN(keyval, "=", 2) - if tmp[0] == matchKey { - str = strings.Replace(str, match, tmp[1], -1) - break - } - } - } - - return str -} - func handleJsonArgs(args []string, attributes map[string]bool) []string { if len(args) == 0 { return []string{} diff --git a/builder/words b/builder/words new file mode 100644 index 0000000000..2148f72537 --- /dev/null +++ b/builder/words @@ -0,0 +1,43 @@ +hello | hello +he'll'o | hello +he'llo | hello +he\'llo | he'llo +he\\'llo | he\llo +abc\tdef | abctdef +"abc\tdef" | abc\tdef +'abc\tdef' | abc\tdef +hello\ | hello +hello\\ | hello\ +"hello | hello +"hello\" | hello" +"hel'lo" | hel'lo +'hello | hello +'hello\' | hello\ +"''" | '' +$. | $. +$1 | +he$1x | hex +he$.x | he$.x +he$pwd. | he. +he$PWD | he/home +he\$PWD | he$PWD +he\\$PWD | he\/home +he\${} | he${} +he\${}xx | he${}xx +he${} | he +he${}xx | hexx +he${hi} | he +he${hi}xx | hexx +he${PWD} | he/home +he${.} | error +'he${XX}' | he${XX} +"he${PWD}" | he/home +"he'$PWD'" | he'/home' +"$PWD" | /home +'$PWD' | $PWD +'\$PWD' | \$PWD +'"hello"' | "hello" +he\$PWD | he$PWD +"he\$PWD" | he$PWD +'he\$PWD' | he\$PWD +he${PWD | error diff --git a/docs/sources/reference/builder.md b/docs/sources/reference/builder.md index 6955d31e0e..9e56abf639 100644 --- a/docs/sources/reference/builder.md +++ b/docs/sources/reference/builder.md @@ -146,6 +146,17 @@ The instructions that handle environment variables in the `Dockerfile` are: `ONBUILD` instructions are **NOT** supported for environment replacement, even the instructions above. +Environment variable subtitution will use the same value for each variable +throughout the entire command. In other words, in this example: + + ENV abc=hello + ENV abc=bye def=$abc + ENV ghi=$abc + +will result in `def` having a value of `hello`, not `bye`. However, +`ghi` will have a value of `bye` because it is not part of the same command +that set `abc` to `bye`. + ## The `.dockerignore` file If a file named `.dockerignore` exists in the source repository, then it diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index c83759d75a..6e78c59adb 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -239,9 +239,18 @@ func TestBuildEnvironmentReplacementEnv(t *testing.T) { _, err := buildImage(name, ` - FROM scratch - ENV foo foo + FROM busybox + ENV foo zzz ENV bar ${foo} + ENV abc1='$foo' + ENV env1=$foo env2=${foo} env3="$foo" env4="${foo}" + RUN [ "$abc1" = '$foo' ] && (echo "$abc1" | grep -q foo) + ENV abc2="\$foo" + RUN [ "$abc2" = '$foo' ] && (echo "$abc2" | grep -q foo) + ENV abc3 '$foo' + RUN [ "$abc3" = '$foo' ] && (echo "$abc3" | grep -q foo) + ENV abc4 "\$foo" + RUN [ "$abc4" = '$foo' ] && (echo "$abc4" | grep -q foo) `, true) if err != nil { @@ -260,13 +269,19 @@ func TestBuildEnvironmentReplacementEnv(t *testing.T) { } found := false + envCount := 0 for _, env := range envResult { parts := strings.SplitN(env, "=", 2) if parts[0] == "bar" { found = true - if parts[1] != "foo" { - t.Fatalf("Could not find replaced var for env `bar`: got %q instead of `foo`", parts[1]) + if parts[1] != "zzz" { + t.Fatalf("Could not find replaced var for env `bar`: got %q instead of `zzz`", parts[1]) + } + } else if strings.HasPrefix(parts[0], "env") { + envCount++ + if parts[1] != "zzz" { + t.Fatalf("%s should be 'foo' but instead its %q", parts[0], parts[1]) } } } @@ -275,6 +290,10 @@ func TestBuildEnvironmentReplacementEnv(t *testing.T) { t.Fatal("Never found the `bar` env variable") } + if envCount != 4 { + t.Fatalf("Didn't find all env vars - only saw %d\n%s", envCount, envResult) + } + logDone("build - env environment replacement") } @@ -361,8 +380,8 @@ func TestBuildHandleEscapes(t *testing.T) { t.Fatal(err) } - if _, ok := result[`\\\\\\${FOO}`]; !ok { - t.Fatal(`Could not find volume \\\\\\${FOO} set from env foo in volumes table`) + if _, ok := result[`\\\${FOO}`]; !ok { + t.Fatal(`Could not find volume \\\${FOO} set from env foo in volumes table`, result) } logDone("build - handle escapes") @@ -2128,7 +2147,7 @@ func TestBuildRelativeWorkdir(t *testing.T) { func TestBuildWorkdirWithEnvVariables(t *testing.T) { name := "testbuildworkdirwithenvvariables" - expected := "/test1/test2/$MISSING_VAR" + expected := "/test1/test2" defer deleteImages(name) _, err := buildImage(name, `FROM busybox @@ -3897,9 +3916,9 @@ ENV abc=zzz TO=/docker/world/hello ADD $FROM $TO RUN [ "$(cat $TO)" = "hello" ] ENV abc "zzz" -RUN [ $abc = \"zzz\" ] +RUN [ $abc = "zzz" ] ENV abc 'yyy' -RUN [ $abc = \'yyy\' ] +RUN [ $abc = 'yyy' ] ENV abc= RUN [ "$abc" = "" ] @@ -3915,13 +3934,34 @@ RUN [ "$abc" = "'foo'" ] ENV abc=\"foo\" RUN [ "$abc" = "\"foo\"" ] ENV abc "foo" -RUN [ "$abc" = "\"foo\"" ] +RUN [ "$abc" = "foo" ] ENV abc 'foo' -RUN [ "$abc" = "'foo'" ] +RUN [ "$abc" = 'foo' ] ENV abc \'foo\' -RUN [ "$abc" = "\\'foo\\'" ] +RUN [ "$abc" = "'foo'" ] ENV abc \"foo\" -RUN [ "$abc" = "\\\"foo\\\"" ] +RUN [ "$abc" = '"foo"' ] + +ENV e1=bar +ENV e2=$e1 +ENV e3=$e11 +ENV e4=\$e1 +ENV e5=\$e11 +RUN [ "$e0,$e1,$e2,$e3,$e4,$e5" = ',bar,bar,,$e1,$e11' ] + +ENV ee1 bar +ENV ee2 $ee1 +ENV ee3 $ee11 +ENV ee4 \$ee1 +ENV ee5 \$ee11 +RUN [ "$ee1,$ee2,$ee3,$ee4,$ee5" = 'bar,bar,,$ee1,$ee11' ] + +ENV eee1="foo" +ENV eee2='foo' +ENV eee3 "foo" +ENV eee4 'foo' +RUN [ "$eee1,$eee2,$eee3,$eee4" = 'foo,foo,foo,foo' ] + ` ctx, err := fakeContext(dockerfile, map[string]string{ "hello/docker/world": "hello",