Skip to content

Commit

Permalink
Merge pull request #661 from tiborvass/dockerfile-shell-only-envvar
Browse files Browse the repository at this point in the history 
dockerfile: RUN's CustomName no longer consume quotes and only replaces environment variables if set
  • Loading branch information
@tonistiigi
tonistiigi committed Oct 29, 2018
2 parents 388c0f9 + b9859b2 commit 85935a3
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 70 deletions.
7 changes: 6 additions & 1 deletion frontend/dockerfile/dockerfile2llb/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,12 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
return err
}
opt = append(opt, runMounts...)
opt = append(opt, llb.WithCustomName(prefixCommand(d, uppercaseCmd(processCmdEnv(dopt.shlex, c.String(), env)), d.prefixPlatform, d.state.GetPlatform())))

shlex := *dopt.shlex
shlex.RawQuotes = true
shlex.SkipUnsetEnv = true

opt = append(opt, llb.WithCustomName(prefixCommand(d, uppercaseCmd(processCmdEnv(&shlex, c.String(), env)), d.prefixPlatform, d.state.GetPlatform())))
for _, h := range dopt.extraHosts {
opt = append(opt, llb.AddExtraHost(h.Host, h.IP))
}
Expand Down
72 changes: 52 additions & 20 deletions frontend/dockerfile/shell/lex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package shell

import (
"bytes"
"fmt"
"strings"
"text/scanner"
"unicode"
Expand All @@ -17,7 +18,9 @@ import (
// 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 Lex struct {
escapeToken rune
escapeToken rune
RawQuotes bool
SkipUnsetEnv bool
}

// NewLex creates a new Lex which uses escapeToken to escape quotes.
Expand Down Expand Up @@ -58,17 +61,21 @@ func (s *Lex) ProcessWordsWithMap(word string, env map[string]string) ([]string,

func (s *Lex) process(word string, env map[string]string) (string, []string, error) {
sw := &shellWord{
envs: env,
escapeToken: s.escapeToken,
envs: env,
escapeToken: s.escapeToken,
skipUnsetEnv: s.SkipUnsetEnv,
rawQuotes: s.RawQuotes,
}
sw.scanner.Init(strings.NewReader(word))
return sw.process(word)
}

type shellWord struct {
scanner scanner.Scanner
envs map[string]string
escapeToken rune
scanner scanner.Scanner
envs map[string]string
escapeToken rune
rawQuotes bool
skipUnsetEnv bool
}

func (sw *shellWord) process(source string) (string, []string, error) {
Expand Down Expand Up @@ -103,10 +110,8 @@ func (w *wordsStruct) addRawChar(ch rune) {
}

func (w *wordsStruct) addString(str string) {
var scan scanner.Scanner
scan.Init(strings.NewReader(str))
for scan.Peek() != scanner.EOF {
w.addChar(scan.Next())
for _, ch := range str {
w.addChar(ch)
}
}

Expand Down Expand Up @@ -196,14 +201,20 @@ func (sw *shellWord) processSingleQuote() (string, error) {

var result bytes.Buffer

sw.scanner.Next()
ch := sw.scanner.Next()
if sw.rawQuotes {
result.WriteRune(ch)
}

for {
ch := sw.scanner.Next()
ch = sw.scanner.Next()
switch ch {
case scanner.EOF:
return "", errors.New("unexpected end of statement while looking for matching single-quote")
case '\'':
if sw.rawQuotes {
result.WriteRune(ch)
}
return result.String(), nil
}
result.WriteRune(ch)
Expand All @@ -225,14 +236,20 @@ func (sw *shellWord) processDoubleQuote() (string, error) {

var result bytes.Buffer

sw.scanner.Next()
ch := sw.scanner.Next()
if sw.rawQuotes {
result.WriteRune(ch)
}

for {
switch sw.scanner.Peek() {
case scanner.EOF:
return "", errors.New("unexpected end of statement while looking for matching double-quote")
case '"':
sw.scanner.Next()
ch := sw.scanner.Next()
if sw.rawQuotes {
result.WriteRune(ch)
}
return result.String(), nil
case '$':
value, err := sw.processDollar()
Expand Down Expand Up @@ -269,7 +286,11 @@ func (sw *shellWord) processDollar() (string, error) {
if name == "" {
return "$", nil
}
return sw.getEnv(name), nil
value, found := sw.getEnv(name)
if !found && sw.skipUnsetEnv {
return "$" + name, nil
}
return value, nil
}

sw.scanner.Next()
Expand All @@ -285,7 +306,11 @@ func (sw *shellWord) processDollar() (string, error) {
switch ch {
case '}':
// Normal ${xx} case
return sw.getEnv(name), nil
value, found := sw.getEnv(name)
if !found && sw.skipUnsetEnv {
return fmt.Sprintf("${%s}", name), nil
}
return value, nil
case ':':
// Special ${xx:...} format processing
// Yes it allows for recursive $'s in the ... spot
Expand All @@ -301,19 +326,26 @@ func (sw *shellWord) processDollar() (string, error) {

// Grab the current value of the variable in question so we
// can use to to determine what to do based on the modifier
newValue := sw.getEnv(name)
newValue, found := sw.getEnv(name)

switch modifier {
case '+':
if newValue != "" {
newValue = word
}
if !found && sw.skipUnsetEnv {
return fmt.Sprintf("${%s:%s%s}", name, string(modifier), word), nil
}
return newValue, nil

case '-':
if newValue == "" {
newValue = word
}
if !found && sw.skipUnsetEnv {
return fmt.Sprintf("${%s:%s%s}", name, string(modifier), word), nil
}

return newValue, nil

default:
Expand Down Expand Up @@ -364,13 +396,13 @@ func isSpecialParam(char rune) bool {
return false
}

func (sw *shellWord) getEnv(name string) string {
func (sw *shellWord) getEnv(name string) (string, bool) {
for key, value := range sw.envs {
if EqualEnvKeys(name, key) {
return value
return value, true
}
}
return ""
return "", false
}

func BuildEnvs(env []string) map[string]string {
Expand Down
112 changes: 63 additions & 49 deletions frontend/dockerfile/shell/lex_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,99 +78,113 @@ func TestShellParser4Words(t *testing.T) {
}
defer file.Close()

var envs []string
shlex := NewLex('\\')
scanner := bufio.NewScanner(file)
lineNum := 0
for scanner.Scan() {
line := scanner.Text()
lineNum = lineNum + 1

if strings.HasPrefix(line, "#") {
continue
const (
modeNormal = iota
modeOnlySetEnv
)
for _, mode := range []int{modeNormal, modeOnlySetEnv} {
var envs []string
shlex := NewLex('\\')
if mode == modeOnlySetEnv {
shlex.RawQuotes = true
shlex.SkipUnsetEnv = true
}
scanner := bufio.NewScanner(file)
lineNum := 0
for scanner.Scan() {
line := scanner.Text()
lineNum = lineNum + 1

if strings.HasPrefix(line, "#") {
continue
}

if strings.HasPrefix(line, "ENV ") {
line = strings.TrimLeft(line[3:], " ")
envs = append(envs, 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'(line %d) - should be exactly one | in: %q", fn, lineNum, line)
}
test := strings.TrimSpace(words[0])
expected := strings.Split(strings.TrimLeft(words[1], " "), ",")
words := strings.Split(line, "|")
if len(words) != 2 {
t.Fatalf("Error in '%s'(line %d) - should be exactly one | in: %q", fn, lineNum, line)
}
test := strings.TrimSpace(words[0])
expected := strings.Split(strings.TrimLeft(words[1], " "), ",")

// test for ProcessWords
result, err := shlex.ProcessWords(test, envs)
// test for ProcessWords
result, err := shlex.ProcessWords(test, envs)

if err != nil {
result = []string{"error"}
}
if err != nil {
result = []string{"error"}
}

if len(result) != len(expected) {
t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
}
for i, w := range expected {
if w != result[i] {
if len(result) != len(expected) {
t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
}
}
for i, w := range expected {
if w != result[i] {
t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
}
}

// test for ProcessWordsWithMap
result, err = shlex.ProcessWordsWithMap(test, BuildEnvs(envs))
// test for ProcessWordsWithMap
result, err = shlex.ProcessWordsWithMap(test, BuildEnvs(envs))

if err != nil {
result = []string{"error"}
}
if err != nil {
result = []string{"error"}
}

if len(result) != len(expected) {
t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
}
for i, w := range expected {
if w != result[i] {
if len(result) != len(expected) {
t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
}
for i, w := range expected {
if w != result[i] {
t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
}
}
}
}
}

func TestGetEnv(t *testing.T) {
sw := &shellWord{envs: nil}

getEnv := func(name string) string {
value, _ := sw.getEnv(name)
return value
}
sw.envs = BuildEnvs([]string{})
if sw.getEnv("foo") != "" {
if getEnv("foo") != "" {
t.Fatal("2 - 'foo' should map to ''")
}

sw.envs = BuildEnvs([]string{"foo"})
if sw.getEnv("foo") != "" {
if getEnv("foo") != "" {
t.Fatal("3 - 'foo' should map to ''")
}

sw.envs = BuildEnvs([]string{"foo="})
if sw.getEnv("foo") != "" {
if getEnv("foo") != "" {
t.Fatal("4 - 'foo' should map to ''")
}

sw.envs = BuildEnvs([]string{"foo=bar"})
if sw.getEnv("foo") != "bar" {
if getEnv("foo") != "bar" {
t.Fatal("5 - 'foo' should map to 'bar'")
}

sw.envs = BuildEnvs([]string{"foo=bar", "car=hat"})
if sw.getEnv("foo") != "bar" {
if getEnv("foo") != "bar" {
t.Fatal("6 - 'foo' should map to 'bar'")
}
if sw.getEnv("car") != "hat" {
if getEnv("car") != "hat" {
t.Fatal("7 - 'car' should map to 'hat'")
}

// Make sure we grab the first 'car' in the list
sw.envs = BuildEnvs([]string{"foo=bar", "car=hat", "car=bike"})
if sw.getEnv("car") != "hat" {
if getEnv("car") != "hat" {
t.Fatal("8 - 'car' should map to 'hat'")
}
}

0 comments on commit 85935a3

Please sign in to comment.