From 05c2d2db9a95266217a639e2109be6fc6482a716 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Fri, 2 Jan 2015 22:40:43 -0700 Subject: [PATCH] Adjust builder to validate that JSON in Dockerfiles are arrays of strings and nothing else to match how we describe them to people (and what all our existing tests already assumed) This also adds more tests to help verify this, including unicode and nonprintable characters (hence the earlier commit switching to strconv.Quote). As a bonus, this fixes a subtle bug where [] was turned into [""] and then turned back into [] (and thus [""] was impossible to actually round-trip correctly in a Dockerfile). Signed-off-by: Andrew "Tianon" Page --- builder/parser/json_test.go | 55 ++++++++++++++++++++++++ builder/parser/line_parsers.go | 40 +++++++---------- builder/parser/parser.go | 5 +-- builder/parser/testfiles/json/Dockerfile | 8 ++++ builder/parser/testfiles/json/result | 8 ++++ 5 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 builder/parser/json_test.go create mode 100644 builder/parser/testfiles/json/Dockerfile create mode 100644 builder/parser/testfiles/json/result diff --git a/builder/parser/json_test.go b/builder/parser/json_test.go new file mode 100644 index 0000000000..a256f845d3 --- /dev/null +++ b/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/builder/parser/line_parsers.go b/builder/parser/line_parsers.go index abde85d292..8a94e1e5db 100644 --- a/builder/parser/line_parsers.go +++ b/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/builder/parser/parser.go b/builder/parser/parser.go index ad42a1586e..4360623e6f 100644 --- a/builder/parser/parser.go +++ b/builder/parser/parser.go @@ -85,10 +85,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/builder/parser/testfiles/json/Dockerfile b/builder/parser/testfiles/json/Dockerfile new file mode 100644 index 0000000000..a586917110 --- /dev/null +++ b/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/builder/parser/testfiles/json/result b/builder/parser/testfiles/json/result new file mode 100644 index 0000000000..c6553e6e1a --- /dev/null +++ b/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")