From 7a8602c54ca730b03e9a47553078d73e478f158e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 14 May 2016 05:51:06 +0200 Subject: [PATCH] Add paranoidUnmarshalJSONObject() helper This allows unmarshaling JSON data and refusing any ambiguous input, to make sure users don't make mistakes when writing policy. This might be a bit easier with reflection, but we will need the non-reflection variant (for unmarshaling a map type) anyway, and quite a few users which do ultimately unmarshal into a struct need to override the type of one or more fields, so reflection would force them to define temporary fields - not necessarily all that better. --- signature/json.go | 58 ++++++++++++++++++++++++++++++++- signature/json_test.go | 73 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/signature/json.go b/signature/json.go index 55915c84..a612db08 100644 --- a/signature/json.go +++ b/signature/json.go @@ -1,6 +1,11 @@ package signature -import "fmt" +import ( + "bytes" + "encoding/json" + "fmt" + "io" +) // jsonFormatError is returned when JSON does not match expected format. type jsonFormatError string @@ -49,3 +54,54 @@ func stringField(m map[string]interface{}, fieldName string) (string, error) { } return v, nil } + +// paranoidUnmarshalJSONObject unmarshals data as a JSON object, but failing on the slightest unexpected aspect +// (including duplicated keys, unrecognized keys, and non-matching types). Uses fieldResolver to +// determine the destination for a field value, which should return a pointer to the destination if valid, or nil if the key is rejected. +// +// The fieldResolver approach is useful for decoding the Policy.Specific map; using it for structs is a bit lazy, +// we could use reflection to automate this. Later? +func paranoidUnmarshalJSONObject(data []byte, fieldResolver func(string) interface{}) error { + seenKeys := map[string]struct{}{} + + dec := json.NewDecoder(bytes.NewReader(data)) + t, err := dec.Token() + if err != nil { + return jsonFormatError(err.Error()) + } + if t != json.Delim('{') { + return jsonFormatError(fmt.Sprintf("JSON object expected, got \"%s\"", t)) + } + for { + t, err := dec.Token() + if err != nil { + return jsonFormatError(err.Error()) + } + if t == json.Delim('}') { + break + } + + key, ok := t.(string) + if !ok { + // Coverage: This should never happen, dec.Token() rejects non-string-literals in this state. + return jsonFormatError(fmt.Sprintf("Key string literal expected, got \"%s\"", t)) + } + if _, ok := seenKeys[key]; ok { + return jsonFormatError(fmt.Sprintf("Duplicate key \"%s\"", key)) + } + seenKeys[key] = struct{}{} + + valuePtr := fieldResolver(key) + if valuePtr == nil { + return jsonFormatError(fmt.Sprintf("Unknown key \"%s\"", key)) + } + // This works like json.Unmarshal, in particular it allows us to implement UnmarshalJSON to implement strict parsing of the field value. + if err := dec.Decode(valuePtr); err != nil { + return jsonFormatError(err.Error()) + } + } + if _, err := dec.Token(); err != io.EOF { + return jsonFormatError("Unexpected data after JSON object") + } + return nil +} diff --git a/signature/json_test.go b/signature/json_test.go index 1c79b268..8d0f63c3 100644 --- a/signature/json_test.go +++ b/signature/json_test.go @@ -1,6 +1,7 @@ package signature import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -74,3 +75,75 @@ func TestStringField(t *testing.T) { require.NoError(t, err) assert.Equal(t, "x", s) } + +// implementsUnmarshalJSON is a minimalistic type used to detect that +// paranoidUnmarshalJSONObject uses the json.Unmarshaler interface of resolved +// pointers. +type implementsUnmarshalJSON bool + +// Compile-time check that Policy implements json.Unmarshaler. +var _ json.Unmarshaler = (*implementsUnmarshalJSON)(nil) + +func (dest *implementsUnmarshalJSON) UnmarshalJSON(data []byte) error { + _ = data // We don't care, not really. + *dest = true // Mark handler as called + return nil +} + +func TestParanoidUnmarshalJSONObject(t *testing.T) { + type testStruct struct { + A string + B int + } + ts := testStruct{} + var unmarshalJSONCalled implementsUnmarshalJSON + tsResolver := func(key string) interface{} { + switch key { + case "a": + return &ts.A + case "b": + return &ts.B + case "implementsUnmarshalJSON": + return &unmarshalJSONCalled + default: + return nil + } + } + + // Empty object + ts = testStruct{} + err := paranoidUnmarshalJSONObject([]byte(`{}`), tsResolver) + require.NoError(t, err) + assert.Equal(t, testStruct{}, ts) + + // Success + ts = testStruct{} + err = paranoidUnmarshalJSONObject([]byte(`{"a":"x", "b":2}`), tsResolver) + require.NoError(t, err) + assert.Equal(t, testStruct{A: "x", B: 2}, ts) + + // json.Unamarshaler is used for decoding values + ts = testStruct{} + unmarshalJSONCalled = implementsUnmarshalJSON(false) + err = paranoidUnmarshalJSONObject([]byte(`{"implementsUnmarshalJSON":true}`), tsResolver) + require.NoError(t, err) + assert.Equal(t, unmarshalJSONCalled, implementsUnmarshalJSON(true)) + + // Various kinds of invalid input + for _, input := range []string{ + ``, // Empty input + `&`, // Entirely invalid JSON + `1`, // Not an object + `{&}`, // Invalid key JSON + `{1:1}`, // Key not a string + `{"b":1, "b":1}`, // Duplicate key + `{"thisdoesnotexist":1}`, // Key rejected by resolver + `{"a":&}`, // Invalid value JSON + `{"a":1}`, // Type mismatch + `{"a":"value"}{}`, // Extra data after object + } { + ts = testStruct{} + err := paranoidUnmarshalJSONObject([]byte(input), tsResolver) + assert.Error(t, err) + } +}