Merge pull request #37134 from thaJeztah/fix-env-substitution

builder: fix processing of invalid substitusion syntax

builder/dockerfile/shell/envVarTest

@@ -18,7 +18,6 @@ A|'hello\there'            |     hello\there
 A|'hello\\there'           |     hello\\there
 A|"''"                     |     ''
 A|$.                       |     $.
-A|$1                       |
 A|he$1x                    |     hex
 A|he$.x                    |     he$.x
 # Next one is different on Windows as $pwd==$PWD
@@ -29,10 +28,14 @@ A|he\$PWD                  |     he$PWD
 A|he\\$PWD                 |     he\/home
 A|"he\$PWD"                |     he$PWD
 A|"he\\$PWD"               |     he\/home
+A|\${}                     |     ${}
+A|\${}aaa                  |     ${}aaa
 A|he\${}                   |     he${}
 A|he\${}xx                 |     he${}xx
-A|he${}                    |     he
-A|he${}xx                  |     hexx
+A|${}                      |     error
+A|${}aaa                   |     error
+A|he${}                    |     error
+A|he${}xx                  |     error
 A|he${hi}                  |     he
 A|he${hi}xx                |     hexx
 A|he${PWD}                 |     he/home
@@ -88,8 +91,8 @@ A|안녕\$PWD                  |     안녕$PWD
 A|안녕\\$PWD                 |     안녕\/home
 A|안녕\${}                   |     안녕${}
 A|안녕\${}xx                 |     안녕${}xx
-A|안녕${}                    |     안녕
-A|안녕${}xx                  |     안녕xx
+A|안녕${}                    |     error
+A|안녕${}xx                  |     error
 A|안녕${hi}                  |     안녕
 A|안녕${hi}xx                |     안녕xx
 A|안녕${PWD}                 |     안녕/home
@@ -119,3 +122,111 @@ 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
+A|${:}                      |     error
+A|${:-bbb}                  |     error
+A|${:+bbb}                  |     error
+
+# Positional parameters won't be set:
+# http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_05_01
+A|$1                        |
+A|${1}                      |
+A|${1:+bbb}                 |
+A|${1:-bbb}                 |     bbb
+A|$2                        |
+A|${2}                      |
+A|${2:+bbb}                 |
+A|${2:-bbb}                 |     bbb
+A|$3                        |
+A|${3}                      |
+A|${3:+bbb}                 |
+A|${3:-bbb}                 |     bbb
+A|$4                        |
+A|${4}                      |
+A|${4:+bbb}                 |
+A|${4:-bbb}                 |     bbb
+A|$5                        |
+A|${5}                      |
+A|${5:+bbb}                 |
+A|${5:-bbb}                 |     bbb
+A|$6                        |
+A|${6}                      |
+A|${6:+bbb}                 |
+A|${6:-bbb}                 |     bbb
+A|$7                        |
+A|${7}                      |
+A|${7:+bbb}                 |
+A|${7:-bbb}                 |     bbb
+A|$8                        |
+A|${8}                      |
+A|${8:+bbb}                 |
+A|${8:-bbb}                 |     bbb
+A|$9                        |
+A|${9}                      |
+A|${9:+bbb}                 |
+A|${9:-bbb}                 |     bbb
+A|$999                      |
+A|${999}                    |
+A|${999:+bbb}               |
+A|${999:-bbb}               |     bbb
+A|$999aaa                   |     aaa
+A|${999}aaa                 |     aaa
+A|${999:+bbb}aaa            |     aaa
+A|${999:-bbb}aaa            |     bbbaaa
+A|$001                      |
+A|${001}                    |
+A|${001:+bbb}               |
+A|${001:-bbb}               |     bbb
+A|$001aaa                   |     aaa
+A|${001}aaa                 |     aaa
+A|${001:+bbb}aaa            |     aaa
+A|${001:-bbb}aaa            |     bbbaaa
+
+# Special parameters won't be set in the Dockerfile:
+# http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_05_02
+A|$@                        |
+A|${@}                      |
+A|${@:+bbb}                 |
+A|${@:-bbb}                 |     bbb
+A|$@@@                      |     @@
+A|$@aaa                     |     aaa
+A|${@}aaa                   |     aaa
+A|${@:+bbb}aaa              |     aaa
+A|${@:-bbb}aaa              |     bbbaaa
+A|$*                        |
+A|${*}                      |
+A|${*:+bbb}                 |
+A|${*:-bbb}                 |     bbb
+A|$#                        |
+A|${#}                      |
+A|${#:+bbb}                 |
+A|${#:-bbb}                 |     bbb
+A|$?                        |
+A|${?}                      |
+A|${?:+bbb}                 |
+A|${?:-bbb}                 |     bbb
+A|$-                        |
+A|${-}                      |
+A|${-:+bbb}                 |
+A|${-:-bbb}                 |     bbb
+A|$$                        |
+A|${$}                      |
+A|${$:+bbb}                 |
+A|${$:-bbb}                 |     bbb
+A|$!                        |
+A|${!}                      |
+A|${!:+bbb}                 |
+A|${!:-bbb}                 |     bbb
+A|$0                        |
+A|${0}                      |
+A|${0:+bbb}                 |
+A|${0:-bbb}                 |     bbb

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
 }
 
@@ -259,22 +261,29 @@ func (sw *shellWord) processDollar() (string, error) {
 	}
 
 	sw.scanner.Next()
+	switch sw.scanner.Peek() {
+	case scanner.EOF:
+		return "", errors.New("syntax error: missing '}'")
+	case '{', '}', ':':
+		// Invalid ${{xx}, ${:xx}, ${:}. ${} case
+		return "", errors.New("syntax error: bad substitution")
+	}
 	name := sw.processName()
-	ch := sw.scanner.Peek()
-	if ch == '}' {
+	ch := sw.scanner.Next()
+	switch ch {
+	case '}':
 		// Normal ${xx} case
-		sw.scanner.Next()
 		return sw.getEnv(name), nil
-	}
-	if ch == ':' {
+	case ':':
 		// Special ${xx:...} format processing
 		// Yes it allows for recursive $'s in the ... spot
-
-		sw.scanner.Next() // skip over :
 		modifier := sw.scanner.Next()
 
 		word, _, err := sw.processStopOn('}')
 		if err != nil {
+			if sw.scanner.Peek() == scanner.EOF {
+				return "", errors.New("syntax error: missing '}'")
+			}
 			return "", err
 		}
 
@@ -310,6 +319,14 @@ func (sw *shellWord) processName() string {
 	for sw.scanner.Peek() != scanner.EOF {
 		ch := sw.scanner.Peek()
 		if name.Len() == 0 && unicode.IsDigit(ch) {
+			for sw.scanner.Peek() != scanner.EOF && unicode.IsDigit(sw.scanner.Peek()) {
+				// Keep reading until the first non-digit character, or EOF
+				ch = sw.scanner.Next()
+				name.WriteRune(ch)
+			}
+			return name.String()
+		}
+		if name.Len() == 0 && isSpecialParam(ch) {
 			ch = sw.scanner.Next()
 			return string(ch)
 		}
@@ -323,6 +340,18 @@ func (sw *shellWord) processName() string {
 	return name.String()
 }
 
+// isSpecialParam checks if the provided character is a special parameters,
+// as defined in http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_05_02
+func isSpecialParam(char rune) bool {
+	switch char {
+	case '@', '*', '#', '?', '-', '$', '!', '0':
+		// Special parameters
+		// http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_05_02
+		return true
+	}
+	return false
+}
+
 func (sw *shellWord) getEnv(name string) string {
 	for _, env := range sw.envs {
 		i := strings.Index(env, "=")

builder/dockerfile/shell/lex_test.go

@@ -26,13 +26,11 @@ func TestShellParser4EnvVars(t *testing.T) {
 		line := scanner.Text()
 		lineCount++
 
-		// Trim comments and blank lines
-		i := strings.Index(line, "#")
-		if i >= 0 {
-			line = line[:i]
+		// Skip comments and blank lines
+		if strings.HasPrefix(line, "#") {
+			continue
 		}
 		line = strings.TrimSpace(line)
-
 		if line == "" {
 			continue
 		}
@@ -53,10 +51,10 @@ 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))
+				assert.Check(t, err, "at line %d of %s", lineCount, fn)
+				assert.Check(t, is.Equal(newWord, expected), "at line %d of %s", lineCount, fn)
 			}
 		}
 	}