From a6d90babbf747f09fa8a42259607794dcf25121d Mon Sep 17 00:00:00 2001 From: Moss Date: Wed, 12 Feb 2020 11:56:18 +0100 Subject: [PATCH 1/3] Add check for json duplicate fields --- template/parse.go | 86 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 9 deletions(-) diff --git a/template/parse.go b/template/parse.go index 33de19c22..2c2cf8b24 100644 --- a/template/parse.go +++ b/template/parse.go @@ -327,17 +327,12 @@ func (r *rawTemplate) parsePostProcessor( // Parse takes the given io.Reader and parses a Template object out of it. func Parse(r io.Reader) (*Template, error) { - // Create a buffer to copy what we read - var buf bytes.Buffer - if _, err := buf.ReadFrom(r); err != nil { - return nil, err - } - - // First, decode the object into an interface{}. We do this instead of - // the rawTemplate directly because we'd rather use mapstructure to + // First, decode the object into an interface{} and search for duplicate fields. + // We do this instead of the rawTemplate directly because we'd rather use mapstructure to // decode since it has richer errors. var raw interface{} - if err := json.Unmarshal(buf.Bytes(), &raw); err != nil { + buf, err := jsonUnmarshal(r, &raw) + if err != nil { return nil, err } @@ -394,6 +389,79 @@ func Parse(r io.Reader) (*Template, error) { return rawTpl.Template() } +func jsonUnmarshal(r io.Reader, raw *interface{}) (bytes.Buffer, error) { + // Create a buffer to copy what we read + var buf bytes.Buffer + if _, err := buf.ReadFrom(r); err != nil { + return buf, err + } + + // Decode the object into an interface{} + if err := json.Unmarshal(buf.Bytes(), raw); err != nil { + return buf, err + } + + // If Json is valid, check for duplicate fields to avoid silent unwanted override + jsonDecoder := json.NewDecoder(strings.NewReader(buf.String())) + if err := checkForDuplicateFields(jsonDecoder); err != nil { + return buf, err + } + + return buf, nil +} + +func checkForDuplicateFields(d *json.Decoder) error { + // Get next token from JSON + t, err := d.Token() + if err != nil { + return err + } + + delim, ok := t.(json.Delim) + // Do nothing if it's not a delimiter + if !ok { + return nil + } + + // Check for duplicates inside of a delimiter {} or [] + switch delim { + case '{': + keys := make(map[string]bool) + for d.More() { + // Get attribute key + t, err := d.Token() + if err != nil { + return err + } + key := t.(string) + + // Check for duplicates + if keys[key] { + return fmt.Errorf("template has duplicate field: %s", key) + } + keys[key] = true + + // Check value to find duplicates in nested blocks + if err := checkForDuplicateFields(d); err != nil { + return err + } + } + case '[': + for d.More() { + if err := checkForDuplicateFields(d); err != nil { + return err + } + } + } + + // consume closing delimiter } or ] + if _, err := d.Token(); err != nil { + return err + } + + return nil +} + // ParseFile is the same as Parse but is a helper to automatically open // a file for parsing. func ParseFile(path string) (*Template, error) { From d654898ebf141cc66a45cc59fa071c94cbc502b0 Mon Sep 17 00:00:00 2001 From: Moss Date: Wed, 12 Feb 2020 14:34:20 +0100 Subject: [PATCH 2/3] Add tests for check of json duplicate fields --- template/parse_test.go | 19 +++++++++++++++++++ .../test-fixtures/error-duplicate-config.json | 11 +++++++++++ .../error-duplicate-variables.json | 13 +++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 template/test-fixtures/error-duplicate-config.json create mode 100644 template/test-fixtures/error-duplicate-variables.json diff --git a/template/parse_test.go b/template/parse_test.go index 77b63616d..03e9c23b3 100644 --- a/template/parse_test.go +++ b/template/parse_test.go @@ -560,3 +560,22 @@ func TestParse_bad(t *testing.T) { } } } + +func TestParse_checkForDuplicateFields(t *testing.T) { + cases := []struct { + File string + Expected string + }{ + {"error-duplicate-variables.json", "template has duplicate field: variables"}, + {"error-duplicate-config.json", "template has duplicate field: foo"}, + } + for _, tc := range cases { + _, err := ParseFile(fixtureDir(tc.File)) + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains(err.Error(), tc.Expected) { + t.Fatalf("file: %s\nExpected: %s\n%s\n", tc.File, tc.Expected, err.Error()) + } + } +} \ No newline at end of file diff --git a/template/test-fixtures/error-duplicate-config.json b/template/test-fixtures/error-duplicate-config.json new file mode 100644 index 000000000..19d2beeec --- /dev/null +++ b/template/test-fixtures/error-duplicate-config.json @@ -0,0 +1,11 @@ +{ + "variables": { + "var": "value" + }, + "builders": [ + { + "foo": "something", + "foo": "something" + } + ] +} diff --git a/template/test-fixtures/error-duplicate-variables.json b/template/test-fixtures/error-duplicate-variables.json new file mode 100644 index 000000000..c603b7fe0 --- /dev/null +++ b/template/test-fixtures/error-duplicate-variables.json @@ -0,0 +1,13 @@ +{ + "variables": { + "var": "value" + }, + "variables": { + "var": "value" + }, + "builders": [ + { + "foo": "something" + } + ] +} From dc81720dc9cfb486c10e211ef79901a5bb6df781 Mon Sep 17 00:00:00 2001 From: Moss Date: Wed, 12 Feb 2020 14:44:28 +0100 Subject: [PATCH 3/3] Fix format --- template/parse_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/template/parse_test.go b/template/parse_test.go index 03e9c23b3..4c2afbbeb 100644 --- a/template/parse_test.go +++ b/template/parse_test.go @@ -578,4 +578,4 @@ func TestParse_checkForDuplicateFields(t *testing.T) { t.Fatalf("file: %s\nExpected: %s\n%s\n", tc.File, tc.Expected, err.Error()) } } -} \ No newline at end of file +}