Merge pull request #32858 from dnephin/builder-shell-words-interface

[Builder] Remove b.escapeToken, create ShellLex

builder/dockerfile/builder.go

@@ -61,7 +61,6 @@ type Builder struct {
 	imageCache    builder.ImageCache
 
 	// TODO: these move to DispatchState
-	escapeToken rune
 	maintainer  string
 	cmdSet      bool
 	noBaseImage bool   // A flag to track the use of `scratch` as the base image
@@ -125,7 +124,6 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back
 		runConfig:     new(container.Config),
 		tmpContainers: map[string]struct{}{},
 		buildArgs:     newBuildArgs(config.BuildArgs),
-		escapeToken:   parser.DefaultEscapeToken,
 	}
 	b.imageContexts = &imageContexts{b: b}
 	return b, nil
@@ -219,8 +217,7 @@ func (b *Builder) build(dockerfile *parser.Result, stdout io.Writer, stderr io.W
 }
 
 func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result) (string, error) {
-	// TODO: pass this to dispatchRequest instead
+	shlex := NewShellLex(dockerfile.EscapeToken)
-	b.escapeToken = dockerfile.EscapeToken
 
 	total := len(dockerfile.AST.Children)
 	var imageID string
@@ -238,7 +235,7 @@ func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result)
 			break
 		}
 
-		if err := b.dispatch(i, total, n); err != nil {
+		if err := b.dispatch(i, total, n, shlex); err != nil {
 			if b.options.ForceRemove {
 				b.clearTmp()
 			}
@@ -361,13 +358,12 @@ func checkDispatchDockerfile(dockerfile *parser.Node) error {
 }
 
 func dispatchFromDockerfile(b *Builder, result *parser.Result) error {
-	// TODO: pass this to dispatchRequest instead
+	shlex := NewShellLex(result.EscapeToken)
-	b.escapeToken = result.EscapeToken
 	ast := result.AST
 	total := len(ast.Children)
 
 	for i, n := range ast.Children {
-		if err := b.dispatch(i, total, n); err != nil {
+		if err := b.dispatch(i, total, n, shlex); err != nil {
 			return err
 		}
 	}

builder/dockerfile/dispatchers.go

@@ -194,7 +194,7 @@ func from(req dispatchRequest) error {
 		return err
 	}
 
-	image, err := req.builder.getFromImage(req.args[0])
+	image, err := req.builder.getFromImage(req.shlex, req.args[0])
 	if err != nil {
 		return err
 	}
@@ -222,13 +222,13 @@ func parseBuildStageName(args []string) (string, error) {
 	return stageName, nil
 }
 
-func (b *Builder) getFromImage(name string) (builder.Image, error) {
+func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, error) {
 	substitutionArgs := []string{}
 	for key, value := range b.buildArgs.GetAllMeta() {
 		substitutionArgs = append(substitutionArgs, key+"="+value)
 	}
 
-	name, err := ProcessWord(name, substitutionArgs, b.escapeToken)
+	name, err := shlex.ProcessWord(name, substitutionArgs)
 	if err != nil {
 		return nil, err
 	}

builder/dockerfile/dispatchers_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/builder"
+	"github.com/docker/docker/builder/dockerfile/parser"
 	"github.com/docker/docker/pkg/testutil"
 	"github.com/docker/go-connections/nat"
 	"github.com/stretchr/testify/assert"
@@ -38,6 +39,7 @@ func defaultDispatchReq(builder *Builder, args ...string) dispatchRequest {
 		args:      args,
 		flags:     NewBFlags(),
 		runConfig: &container.Config{},
+		shlex:     NewShellLex(parser.DefaultEscapeToken),
 	}
 }
 

builder/dockerfile/evaluator.go

@@ -65,9 +65,10 @@ type dispatchRequest struct {
 	flags      *BFlags
 	original   string
 	runConfig  *container.Config
+	shlex      *ShellLex
 }
 
-func newDispatchRequestFromNode(node *parser.Node, builder *Builder, args []string) dispatchRequest {
+func newDispatchRequestFromNode(node *parser.Node, builder *Builder, args []string, shlex *ShellLex) dispatchRequest {
 	return dispatchRequest{
 		builder:    builder,
 		args:       args,
@@ -75,6 +76,7 @@ func newDispatchRequestFromNode(node *parser.Node, builder *Builder, args []stri
 		original:   node.Original,
 		flags:      NewBFlagsWithArgs(node.Flags),
 		runConfig:  builder.runConfig,
+		shlex:      shlex,
 	}
 }
 
@@ -119,7 +121,7 @@ func init() {
 // such as `RUN` in ONBUILD RUN foo. There is special case logic in here to
 // deal with that, at least until it becomes more of a general concern with new
 // features.
-func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node) error {
+func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node, shlex *ShellLex) error {
 	cmd := node.Value
 	upperCasedCmd := strings.ToUpper(cmd)
 
@@ -154,9 +156,10 @@ func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node) error {
 	// Append build args to runConfig environment variables
 	envs := append(b.runConfig.Env, b.buildArgsWithoutConfigEnv()...)
 
+	processFunc := getProcessFunc(shlex, cmd)
 	for i := 0; ast.Next != nil; i++ {
 		ast = ast.Next
-		words, err := b.evaluateEnv(cmd, ast.Value, envs)
+		words, err := processFunc(ast.Value, envs)
 		if err != nil {
 			return err
 		}
@@ -170,7 +173,7 @@ func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node) error {
 	// XXX yes, we skip any cmds that are not valid; the parser should have
 	// picked these out already.
 	if f, ok := evaluateTable[cmd]; ok {
-		return f(newDispatchRequestFromNode(node, b, strList))
+		return f(newDispatchRequestFromNode(node, b, strList, shlex))
 	}
 
 	return fmt.Errorf("Unknown instruction: %s", upperCasedCmd)
@@ -186,20 +189,22 @@ func initMsgList(cursor *parser.Node) []string {
 	return make([]string, n)
 }
 
-func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string, error) {
+type processFunc func(string, []string) ([]string, error)
-	if !replaceEnvAllowed[cmd] {
+
-		return []string{str}, nil
+func getProcessFunc(shlex *ShellLex, cmd string) processFunc {
+	switch {
+	case !replaceEnvAllowed[cmd]:
+		return func(word string, _ []string) ([]string, error) {
+			return []string{word}, nil
 		}
-	var processFunc func(string, []string, rune) ([]string, error)
+	case allowWordExpansion[cmd]:
-	if allowWordExpansion[cmd] {
+		return shlex.ProcessWords
-		processFunc = ProcessWords
+	default:
-	} else {
+		return func(word string, envs []string) ([]string, error) {
-		processFunc = func(word string, envs []string, escape rune) ([]string, error) {
+			word, err := shlex.ProcessWord(word, envs)
-			word, err := ProcessWord(word, envs, escape)
 			return []string{word}, err
 		}
 	}
-	return processFunc(str, envs, b.escapeToken)
 }
 
 // buildArgsWithoutConfigEnv returns a list of key=value pairs for all the build

builder/dockerfile/evaluator_test.go

@@ -190,8 +190,9 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
 		buildArgs: newBuildArgs(options.BuildArgs),
 	}
 
+	shlex := NewShellLex(parser.DefaultEscapeToken)
 	n := result.AST
-	err = b.dispatch(0, len(n.Children), n.Children[0])
+	err = b.dispatch(0, len(n.Children), n.Children[0], shlex)
 
 	if err == nil {
 		t.Fatalf("No error when executing test %s", testCase.name)

builder/dockerfile/shell_parser.go

@@ -1,11 +1,5 @@
 package dockerfile
 
-// 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 (
 	"bytes"
 	"strings"
@@ -15,18 +9,26 @@ import (
 	"github.com/pkg/errors"
 )
 
-type shellWord struct {
+// ShellLex performs shell word splitting and variable expansion.
-	word        string
+//
-	scanner     scanner.Scanner
+// ShellLex takes a string and an array of env variables and
-	envs        []string
+// process all quotes (" and ') as well as $xxx and ${xxx} env variable
-	pos         int
+// 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
+type ShellLex struct {
 	escapeToken rune
 }
 
+// NewShellLex creates a new ShellLex which uses escapeToken to escape quotes.
+func NewShellLex(escapeToken rune) *ShellLex {
+	return &ShellLex{escapeToken: escapeToken}
+}
+
 // 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) {
+func (s *ShellLex) ProcessWord(word string, env []string) (string, error) {
-	word, _, err := process(word, env, escapeToken)
+	word, _, err := s.process(word, env)
 	return word, err
 }
 
@@ -37,24 +39,32 @@ func ProcessWord(word string, env []string, escapeToken rune) (string, error) {
 // 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, escapeToken rune) ([]string, error) {
+func (s *ShellLex) ProcessWords(word string, env []string) ([]string, error) {
-	_, words, err := process(word, env, escapeToken)
+	_, words, err := s.process(word, env)
 	return words, err
 }
 
-func process(word string, env []string, escapeToken rune) (string, []string, error) {
+func (s *ShellLex) process(word string, env []string) (string, []string, error) {
 	sw := &shellWord{
-		word:        word,
 		envs:        env,
-		pos:         0,
+		escapeToken: s.escapeToken,
-		escapeToken: escapeToken,
 	}
 	sw.scanner.Init(strings.NewReader(word))
-	return sw.process()
+	return sw.process(word)
 }
 
-func (sw *shellWord) process() (string, []string, error) {
+type shellWord struct {
-	return sw.processStopOn(scanner.EOF)
+	scanner     scanner.Scanner
+	envs        []string
+	escapeToken rune
+}
+
+func (sw *shellWord) process(source string) (string, []string, error) {
+	word, words, err := sw.processStopOn(scanner.EOF)
+	if err != nil {
+		err = errors.Wrapf(err, "failed to process %q", source)
+	}
+	return word, words, err
 }
 
 type wordsStruct struct {
@@ -286,10 +296,10 @@ func (sw *shellWord) processDollar() (string, error) {
 			return newValue, nil
 
 		default:
-			return "", errors.Errorf("unsupported modifier (%c) in substitution: %s", modifier, sw.word)
+			return "", errors.Errorf("unsupported modifier (%c) in substitution", modifier)
 		}
 	}
-	return "", errors.Errorf("missing ':' in substitution: %s", sw.word)
+	return "", errors.Errorf("missing ':' in substitution")
 }
 
 func (sw *shellWord) processName() string {

builder/dockerfile/shell_parser_test.go

@@ -18,6 +18,7 @@ func TestShellParser4EnvVars(t *testing.T) {
 	assert.NoError(t, err)
 	defer file.Close()
 
+	shlex := NewShellLex('\\')
 	scanner := bufio.NewScanner(file)
 	envs := []string{"PWD=/home", "SHELL=bash", "KOREAN=한국어"}
 	for scanner.Scan() {
@@ -49,7 +50,7 @@ func TestShellParser4EnvVars(t *testing.T) {
 
 		if ((platform == "W" || platform == "A") && runtime.GOOS == "windows") ||
 			((platform == "U" || platform == "A") && runtime.GOOS != "windows") {
-			newWord, err := ProcessWord(source, envs, '\\')
+			newWord, err := shlex.ProcessWord(source, envs)
 			if expected == "error" {
 				assert.Error(t, err)
 			} else {
@@ -69,6 +70,7 @@ func TestShellParser4Words(t *testing.T) {
 	}
 	defer file.Close()
 
+	shlex := NewShellLex('\\')
 	envs := []string{}
 	scanner := bufio.NewScanner(file)
 	lineNum := 0
@@ -93,7 +95,7 @@ func TestShellParser4Words(t *testing.T) {
 		test := strings.TrimSpace(words[0])
 		expected := strings.Split(strings.TrimLeft(words[1], " "), ",")
 
-		result, err := ProcessWords(test, envs, '\\')
+		result, err := shlex.ProcessWords(test, envs)
 
 		if err != nil {
 			result = []string{"error"}
@@ -111,11 +113,7 @@ func TestShellParser4Words(t *testing.T) {
 }
 
 func TestGetEnv(t *testing.T) {
-	sw := &shellWord{
+	sw := &shellWord{envs: nil}
-		word: "",
-		envs: nil,
-		pos:  0,
-	}
 
 	sw.envs = []string{}
 	if sw.getEnv("foo") != "" {