diff --git a/components/engine/builder/parser/json_test.go b/components/engine/builder/parser/json_test.go new file mode 100644 index 0000000000..a256f845d3 --- /dev/null +++ b/components/engine/builder/parser/json_test.go @@ -0,0 +1,55 @@ +package parser + +import ( + "testing" +) + +var invalidJSONArraysOfStrings = []string{ + `["a",42,"b"]`, + `["a",123.456,"b"]`, + `["a",{},"b"]`, + `["a",{"c": "d"},"b"]`, + `["a",["c"],"b"]`, + `["a",true,"b"]`, + `["a",false,"b"]`, + `["a",null,"b"]`, +} + +var validJSONArraysOfStrings = map[string][]string{ + `[]`: {}, + `[""]`: {""}, + `["a"]`: {"a"}, + `["a","b"]`: {"a", "b"}, + `[ "a", "b" ]`: {"a", "b"}, + `[ "a", "b" ]`: {"a", "b"}, + ` [ "a", "b" ] `: {"a", "b"}, + `["abc 123", "♥", "☃", "\" \\ \/ \b \f \n \r \t \u0000"]`: {"abc 123", "♥", "☃", "\" \\ / \b \f \n \r \t \u0000"}, +} + +func TestJSONArraysOfStrings(t *testing.T) { + for json, expected := range validJSONArraysOfStrings { + if node, _, err := parseJSON(json); err != nil { + t.Fatalf("%q should be a valid JSON array of strings, but wasn't! (err: %q)", json, err) + } else { + i := 0 + for node != nil { + if i >= len(expected) { + t.Fatalf("expected result is shorter than parsed result (%d vs %d+) in %q", len(expected), i+1, json) + } + if node.Value != expected[i] { + t.Fatalf("expected %q (not %q) in %q at pos %d", expected[i], node.Value, json, i) + } + node = node.Next + i++ + } + if i != len(expected) { + t.Fatalf("expected result is longer than parsed result (%d vs %d) in %q", len(expected), i+1, json) + } + } + } + for _, json := range invalidJSONArraysOfStrings { + if _, _, err := parseJSON(json); err != errDockerfileNotStringArray { + t.Fatalf("%q should be an invalid JSON array of strings, but wasn't!", json) + } + } +} diff --git a/components/engine/builder/parser/line_parsers.go b/components/engine/builder/parser/line_parsers.go index abde85d292..8a94e1e5db 100644 --- a/components/engine/builder/parser/line_parsers.go +++ b/components/engine/builder/parser/line_parsers.go @@ -10,13 +10,12 @@ import ( "encoding/json" "errors" "fmt" - "strconv" "strings" "unicode" ) var ( - errDockerfileJSONNesting = errors.New("You may not nest arrays in Dockerfile statements.") + errDockerfileNotStringArray = errors.New("When using JSON array syntax, arrays must be comprised of strings only.") ) // ignore the current argument. This will still leave a command parsed, but @@ -209,34 +208,27 @@ func parseString(rest string) (*Node, map[string]bool, error) { // parseJSON converts JSON arrays to an AST. func parseJSON(rest string) (*Node, map[string]bool, error) { - var ( - myJson []interface{} - next = &Node{} - orignext = next - prevnode = next - ) - + var myJson []interface{} if err := json.Unmarshal([]byte(rest), &myJson); err != nil { return nil, nil, err } + var top, prev *Node for _, str := range myJson { - switch str.(type) { - case string: - case float64: - str = strconv.FormatFloat(str.(float64), 'G', -1, 64) - default: - return nil, nil, errDockerfileJSONNesting + if s, ok := str.(string); !ok { + return nil, nil, errDockerfileNotStringArray + } else { + node := &Node{Value: s} + if prev == nil { + top = node + } else { + prev.Next = node + } + prev = node } - next.Value = str.(string) - next.Next = &Node{} - prevnode = next - next = next.Next } - prevnode.Next = nil - - return orignext, map[string]bool{"json": true}, nil + return top, map[string]bool{"json": true}, nil } // parseMaybeJSON determines if the argument appears to be a JSON array. If @@ -250,7 +242,7 @@ func parseMaybeJSON(rest string) (*Node, map[string]bool, error) { if err == nil { return node, attrs, nil } - if err == errDockerfileJSONNesting { + if err == errDockerfileNotStringArray { return nil, nil, err } @@ -270,7 +262,7 @@ func parseMaybeJSONToList(rest string) (*Node, map[string]bool, error) { if err == nil { return node, attrs, nil } - if err == errDockerfileJSONNesting { + if err == errDockerfileNotStringArray { return nil, nil, err } diff --git a/components/engine/builder/parser/parser.go b/components/engine/builder/parser/parser.go index a0bb881f80..7bcc255bfe 100644 --- a/components/engine/builder/parser/parser.go +++ b/components/engine/builder/parser/parser.go @@ -90,10 +90,7 @@ func parseLine(line string) (string, *Node, error) { return "", nil, err } - if sexp.Value != "" || sexp.Next != nil || sexp.Children != nil { - node.Next = sexp - } - + node.Next = sexp node.Attributes = attrs node.Original = line diff --git a/components/engine/builder/parser/parser_test.go b/components/engine/builder/parser/parser_test.go index 1b517fcc14..daceb9839c 100644 --- a/components/engine/builder/parser/parser_test.go +++ b/components/engine/builder/parser/parser_test.go @@ -54,18 +54,14 @@ func TestTestData(t *testing.T) { if err != nil { t.Fatalf("Dockerfile missing for %s: %s", dir.Name(), err.Error()) } - - rf, err := os.Open(resultfile) - if err != nil { - t.Fatalf("Result file missing for %s: %s", dir.Name(), err.Error()) - } + defer df.Close() ast, err := Parse(df) if err != nil { t.Fatalf("Error parsing %s's dockerfile: %s", dir.Name(), err.Error()) } - content, err := ioutil.ReadAll(rf) + content, err := ioutil.ReadFile(resultfile) if err != nil { t.Fatalf("Error reading %s's result file: %s", dir.Name(), err.Error()) } @@ -75,8 +71,5 @@ func TestTestData(t *testing.T) { fmt.Fprintln(os.Stderr, "Expected:\n"+string(content)) t.Fatalf("%s: AST dump of dockerfile does not match result", dir.Name()) } - - df.Close() - rf.Close() } } diff --git a/components/engine/builder/parser/testfiles/brimstone-consuldock/result b/components/engine/builder/parser/testfiles/brimstone-consuldock/result index cc8fab2131..227f748cda 100644 --- a/components/engine/builder/parser/testfiles/brimstone-consuldock/result +++ b/components/engine/builder/parser/testfiles/brimstone-consuldock/result @@ -2,4 +2,4 @@ (maintainer "brimstone@the.narro.ws") (env "GOPATH" "/go") (entrypoint "/usr/local/bin/consuldock") -(run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates && apt-get clean && rm -rf /var/lib/apt/lists && go get -v github.com/brimstone/consuldock && mv $GOPATH/bin/consuldock /usr/local/bin/consuldock && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.* && rm -rf $GOPATH") +(run "apt-get update \t&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates && apt-get clean && rm -rf /var/lib/apt/lists \t&& go get -v github.com/brimstone/consuldock && mv $GOPATH/bin/consuldock /usr/local/bin/consuldock \t&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty \t&& apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') \t&& rm /tmp/dpkg.* \t&& rm -rf $GOPATH") diff --git a/components/engine/builder/parser/testfiles/brimstone-docker-consul/result b/components/engine/builder/parser/testfiles/brimstone-docker-consul/result index 8c989e6211..16492e516a 100644 --- a/components/engine/builder/parser/testfiles/brimstone-docker-consul/result +++ b/components/engine/builder/parser/testfiles/brimstone-docker-consul/result @@ -2,8 +2,8 @@ (cmd) (entrypoint "/usr/bin/consul" "agent" "-server" "-data-dir=/consul" "-client=0.0.0.0" "-ui-dir=/webui") (expose "8500" "8600" "8400" "8301" "8302") -(run "apt-get update && apt-get install -y unzip wget && apt-get clean && rm -rf /var/lib/apt/lists") +(run "apt-get update && apt-get install -y unzip wget \t&& apt-get clean \t&& rm -rf /var/lib/apt/lists") (run "cd /tmp && wget https://dl.bintray.com/mitchellh/consul/0.3.1_web_ui.zip -O web_ui.zip && unzip web_ui.zip && mv dist /webui && rm web_ui.zip") -(run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends unzip wget && apt-get clean && rm -rf /var/lib/apt/lists && cd /tmp && wget https://dl.bintray.com/mitchellh/consul/0.3.1_web_ui.zip -O web_ui.zip && unzip web_ui.zip && mv dist /webui && rm web_ui.zip && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.*") +(run "apt-get update \t&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends unzip wget && apt-get clean && rm -rf /var/lib/apt/lists && cd /tmp && wget https://dl.bintray.com/mitchellh/consul/0.3.1_web_ui.zip -O web_ui.zip && unzip web_ui.zip && mv dist /webui && rm web_ui.zip \t&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty \t&& apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') \t&& rm /tmp/dpkg.*") (env "GOPATH" "/go") -(run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates build-essential && apt-get clean && rm -rf /var/lib/apt/lists && go get -v github.com/hashicorp/consul && mv $GOPATH/bin/consul /usr/bin/consul && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.* && rm -rf $GOPATH") +(run "apt-get update \t&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates build-essential && apt-get clean && rm -rf /var/lib/apt/lists \t&& go get -v github.com/hashicorp/consul \t&& mv $GOPATH/bin/consul /usr/bin/consul \t&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty \t&& apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') \t&& rm /tmp/dpkg.* \t&& rm -rf $GOPATH") diff --git a/components/engine/builder/parser/testfiles/docker/result b/components/engine/builder/parser/testfiles/docker/result index 80f219ecb4..773b640a9d 100644 --- a/components/engine/builder/parser/testfiles/docker/result +++ b/components/engine/builder/parser/testfiles/docker/result @@ -1,13 +1,13 @@ (from "ubuntu:14.04") (maintainer "Tianon Gravi (@tianon)") -(run "apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -yq apt-utils aufs-tools automake btrfs-tools build-essential curl dpkg-sig git iptables libapparmor-dev libcap-dev libsqlite3-dev lxc=1.0* mercurial pandoc parallel reprepro ruby1.9.1 ruby1.9.1-dev s3cmd=1.1.0* --no-install-recommends") +(run "apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -yq \tapt-utils \taufs-tools \tautomake \tbtrfs-tools \tbuild-essential \tcurl \tdpkg-sig \tgit \tiptables \tlibapparmor-dev \tlibcap-dev \tlibsqlite3-dev \tlxc=1.0* \tmercurial \tpandoc \tparallel \treprepro \truby1.9.1 \truby1.9.1-dev \ts3cmd=1.1.0* \t--no-install-recommends") (run "git clone --no-checkout https://git.fedorahosted.org/git/lvm2.git /usr/local/lvm2 && cd /usr/local/lvm2 && git checkout -q v2_02_103") (run "cd /usr/local/lvm2 && ./configure --enable-static_link && make device-mapper && make install_device-mapper") (run "curl -sSL https://golang.org/dl/go1.3.src.tar.gz | tar -v -C /usr/local -xz") (env "PATH" "/usr/local/go/bin:$PATH") (env "GOPATH" "/go:/go/src/github.com/docker/docker/vendor") (run "cd /usr/local/go/src && ./make.bash --no-clean 2>&1") -(env "DOCKER_CROSSPLATFORMS" "linux/386 linux/arm darwin/amd64 darwin/386 freebsd/amd64 freebsd/386 freebsd/arm") +(env "DOCKER_CROSSPLATFORMS" "linux/386 linux/arm \tdarwin/amd64 darwin/386 \tfreebsd/amd64 freebsd/386 freebsd/arm") (env "GOARM" "5") (run "cd /usr/local/go/src && bash -xc 'for platform in $DOCKER_CROSSPLATFORMS; do GOOS=${platform%/*} GOARCH=${platform##*/} ./make.bash --no-clean 2>&1; done'") (run "go get golang.org/x/tools/cmd/cover") diff --git a/components/engine/builder/parser/testfiles/json/Dockerfile b/components/engine/builder/parser/testfiles/json/Dockerfile new file mode 100644 index 0000000000..a586917110 --- /dev/null +++ b/components/engine/builder/parser/testfiles/json/Dockerfile @@ -0,0 +1,8 @@ +CMD [] +CMD [""] +CMD ["a"] +CMD ["a","b"] +CMD [ "a", "b" ] +CMD [ "a", "b" ] +CMD [ "a", "b" ] +CMD ["abc 123", "♥", "☃", "\" \\ \/ \b \f \n \r \t \u0000"] diff --git a/components/engine/builder/parser/testfiles/json/result b/components/engine/builder/parser/testfiles/json/result new file mode 100644 index 0000000000..c6553e6e1a --- /dev/null +++ b/components/engine/builder/parser/testfiles/json/result @@ -0,0 +1,8 @@ +(cmd) +(cmd "") +(cmd "a") +(cmd "a" "b") +(cmd "a" "b") +(cmd "a" "b") +(cmd "a" "b") +(cmd "abc 123" "♥" "☃" "\" \\ / \b \f \n \r \t \x00") diff --git a/components/engine/builder/parser/utils.go b/components/engine/builder/parser/utils.go index 096c4e31e9..3a8cd24e8c 100644 --- a/components/engine/builder/parser/utils.go +++ b/components/engine/builder/parser/utils.go @@ -2,30 +2,10 @@ package parser import ( "fmt" + "strconv" "strings" ) -// QuoteString walks characters (after trimming), escapes any quotes and -// escapes, then wraps the whole thing in quotes. Very useful for generating -// argument output in nodes. -func QuoteString(str string) string { - result := "" - chars := strings.Split(strings.TrimSpace(str), "") - - for _, char := range chars { - switch char { - case `"`: - result += `\"` - case `\`: - result += `\\` - default: - result += char - } - } - - return `"` + result + `"` -} - // dumps the AST defined by `node` as a list of sexps. Returns a string // suitable for printing. func (node *Node) Dump() string { @@ -41,7 +21,7 @@ func (node *Node) Dump() string { if len(n.Children) > 0 { str += " " + n.Dump() } else { - str += " " + QuoteString(n.Value) + str += " " + strconv.Quote(n.Value) } } }