From c8db46ad053daa297bf3b3f1a4e8b6e840d1e81f Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Wed, 23 May 2018 16:31:49 +0200
Subject: [PATCH] builder: fix processing of invalid substitusion syntax

The builder did not detect syntax errors in substitusions in the
Dockerfile, causing those values to be processed incorrectly instead of
producing an error.

Example 1: missing `}`

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${aaa:-bbb
    RUN echo $var
    EOF

Before:

    Step 3/3 : RUN echo $var
     ---> Running in f06571e77146
    bbb

After:

    Step 2/3 : ARG var=${aaa:-bbb
    failed to process "${aaa:-bbb": syntax error: missing '}'

Example 2: missing closing `}`, no default value

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${aaa
    RUN echo $var
    EOF

Before:

    Step 2/3 : ARG var=${aaa
    failed to process "${aaa": missing ':' in substitution

After:

    Step 2/3 : ARG var=${aaa
    failed to process "${aaa": syntax error: missing '}'

Example 3: double opening bracket (`{`)

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${{aaa:-bbb}
    RUN echo $var
    EOF

Before:

    Step 2/3 : ARG var=${{aaa:-bbb}
    failed to process "${{aaa:-bbb}": missing ':' in substitution

After:

    Step 2/3 : ARG var=${{aaa:-bbb}
    failed to process "${{aaa:-bbb}": syntax error: bad substitution

Example 4: double opening bracket (`{`), no default value

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${{aaa}
    RUN echo $var
    EOF

Before:

    Step 2/3 : ARG var=${{aaa}
    failed to process "${{aaa}": missing ':' in substitution

After:

    Step 2/3 : ARG var=${{aaa}
    failed to process "${{aaa}": syntax error: bad substitution

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 955a6ad95f7891a45692d975793abf1eeb07cdd5
Component: engine
---
 .../builder/dockerfile/shell/envVarTest       | 11 ++++++++++
 .../engine/builder/dockerfile/shell/lex.go    | 20 ++++++++++++++-----
 .../builder/dockerfile/shell/lex_test.go      |  2 +-
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/components/engine/builder/dockerfile/shell/envVarTest b/components/engine/builder/dockerfile/shell/envVarTest
index 946b278592..677b33d9a7 100644
--- a/components/engine/builder/dockerfile/shell/envVarTest
+++ b/components/engine/builder/dockerfile/shell/envVarTest
@@ -119,3 +119,14 @@ A|안녕${XXX:-\$PWD:}xx       |     안녕$PWD:xx
 A|안녕${XXX:-\${PWD}z}xx     |     안녕${PWDz}xx
 A|$KOREAN                    |     한국어
 A|안녕$KOREAN                |     안녕한국어
+A|${{aaa}                   |     error
+A|${aaa}}                   |     }
+A|${aaa                     |     error
+A|${{aaa:-bbb}              |     error
+A|${aaa:-bbb}}              |     bbb}
+A|${aaa:-bbb                |     error
+A|${aaa:-bbb}               |     bbb
+A|${aaa:-${bbb:-ccc}}       |     ccc
+A|${aaa:-bbb ${foo}         |     error
+A|${aaa:-bbb {foo}          |     bbb {foo
+
diff --git a/components/engine/builder/dockerfile/shell/lex.go b/components/engine/builder/dockerfile/shell/lex.go
index bd3fac525a..d0d6f53e66 100644
--- a/components/engine/builder/dockerfile/shell/lex.go
+++ b/components/engine/builder/dockerfile/shell/lex.go
@@ -131,7 +131,7 @@ func (sw *shellWord) processStopOn(stopChar rune) (string, []string, error) {
 
 		if stopChar != scanner.EOF && ch == stopChar {
 			sw.scanner.Next()
-			break
+			return result.String(), words.getWords(), nil
 		}
 		if fn, ok := charFuncMapping[ch]; ok {
 			// Call special processing func for certain chars
@@ -166,7 +166,9 @@ func (sw *shellWord) processStopOn(stopChar rune) (string, []string, error) {
 			result.WriteRune(ch)
 		}
 	}
-
+	if stopChar != scanner.EOF {
+		return "", []string{}, errors.Errorf("unexpected end of statement while looking for matching %s", string(stopChar))
+	}
 	return result.String(), words.getWords(), nil
 }
 
@@ -261,12 +263,17 @@ func (sw *shellWord) processDollar() (string, error) {
 	sw.scanner.Next()
 	name := sw.processName()
 	ch := sw.scanner.Peek()
-	if ch == '}' {
+	switch ch {
+	case '}':
 		// Normal ${xx} case
 		sw.scanner.Next()
 		return sw.getEnv(name), nil
-	}
-	if ch == ':' {
+	case '{':
+		// Invalid ${{xx} case
+		return "", errors.New("syntax error: bad substitution")
+	case scanner.EOF:
+		return "", errors.New("syntax error: missing '}'")
+	case ':':
 		// Special ${xx:...} format processing
 		// Yes it allows for recursive $'s in the ... spot
 
@@ -275,6 +282,9 @@ func (sw *shellWord) processDollar() (string, error) {
 
 		word, _, err := sw.processStopOn('}')
 		if err != nil {
+			if sw.scanner.Peek() == scanner.EOF {
+				return "", errors.New("syntax error: missing '}'")
+			}
 			return "", err
 		}
 
diff --git a/components/engine/builder/dockerfile/shell/lex_test.go b/components/engine/builder/dockerfile/shell/lex_test.go
index f38da2026f..765ae14b25 100644
--- a/components/engine/builder/dockerfile/shell/lex_test.go
+++ b/components/engine/builder/dockerfile/shell/lex_test.go
@@ -53,7 +53,7 @@ func TestShellParser4EnvVars(t *testing.T) {
 			((platform == "U" || platform == "A") && runtime.GOOS != "windows") {
 			newWord, err := shlex.ProcessWord(source, envs)
 			if expected == "error" {
-				assert.Check(t, is.ErrorContains(err, ""))
+				assert.Check(t, is.ErrorContains(err, ""), "input: %q, result: %q", source, newWord)
 			} else {
 				assert.Check(t, err)
 				assert.Check(t, is.Equal(newWord, expected))