Skip to content

builder: fix processing of invalid substitution syntax #37134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tonistiigi merged 6 commits into from
May 30, 2018
Merged

builder: fix processing of invalid substitution syntax #37134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
955a6ad
builder: fix processing of invalid substitusion syntax
thaJeztah May 23, 2018
334bf3e
Fix detection for missing parameter in substitution
thaJeztah May 25, 2018
b80e030
Add line-numbers to asserts
thaJeztah May 25, 2018
8687a3f
Add more test-cases for positional parameters
thaJeztah May 25, 2018
9654e9b
Add detection of "special parameters" for substitution
thaJeztah May 25, 2018
2628896
Handle multi-digit positional parameters
thaJeztah May 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 116 additions & 5 deletions builder/dockerfile/shell/envVarTest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
47 changes: 38 additions & 9 deletions builder/dockerfile/shell/lex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -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, "=")
Expand Down
14 changes: 6 additions & 8 deletions builder/dockerfile/shell/lex_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}
}
}
Expand Down