Merge pull request #17789 from duglin/Issue17781

Allow for env vars to have spaces in some cases

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++
 	}

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
 			}

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: "",

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

integration-cli/docker_cli_build_test.go

@@ -151,6 +151,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)
@@ -167,8 +169,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)
+		}
 	}
 
 }