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 <admwiggin@gmail.com>

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)
+		}
+	}
+}

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 (
+	var myJson []interface{}
-		myJson   []interface{}
-		next     = &Node{}
-		orignext = next
-		prevnode = next
-	)
-
 	if err := json.Unmarshal([]byte(rest), &myJson); err != nil {
 		return nil, nil, err
 	}
 
+	var top, prev *Node
 	for _, str := range myJson {
-		switch str.(type) {
+		if s, ok := str.(string); !ok {
-		case string:
+			return nil, nil, errDockerfileNotStringArray
-		case float64:
+		} else {
-			str = strconv.FormatFloat(str.(float64), 'G', -1, 64)
+			node := &Node{Value: s}
-		default:
+			if prev == nil {
-			return nil, nil, errDockerfileJSONNesting
+				top = node
+			} else {
+				prev.Next = node
+			}
+			prev = node
 		}
-		next.Value = str.(string)
-		next.Next = &Node{}
-		prevnode = next
-		next = next.Next
 	}
 
-	prevnode.Next = nil
+	return top, map[string]bool{"json": true}, nil
-
-	return orignext, 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
 	}
 

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.Attributes = attrs
 	node.Original = line
 

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"]

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")