From 722a6588c981c2f7f822501459cb14b51500c4b6 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 26 Apr 2019 19:59:09 +0300 Subject: [PATCH] switch the json backend to json-iterator add a couple of json interator configurations: 1) jsonIterator should be compatible with the old encoding/json there is a breaking change here because the "JSONOpt" is now a no-op, although the options in encoding/json are not very useful. 2) caseSensitiveStrictJSONIterator is compatible with econding/json WRT to DisallowUnknownFields, but it also has CaseSensitive. This is a breaking change for strict unmarshal users that previously allowed case-insensitive fields. Something else to note is that json-iter does not seem to tolerate field keys called "true", which the one from stdlib does. Unit tests had to be adapted because of that. Other changes: - add a new public method UnmarshalWithConfig that allows passing a json-iter configuration - update/add unit tests - update go.mod/sum - remove yaml_go110*.go These files had the purpose to handle DisallowUnknownFields for json.Decoder which is no longer needed. --- go.mod | 8 +++- go.sum | 14 +++++++ yaml.go | 92 ++++++++++++++++++++++++++++++---------------- yaml_go110.go | 14 ------- yaml_go110_test.go | 46 ----------------------- yaml_test.go | 60 ++++++++++++++++++++++-------- 6 files changed, 127 insertions(+), 107 deletions(-) delete mode 100644 yaml_go110.go delete mode 100644 yaml_go110_test.go diff --git a/go.mod b/go.mod index 9d60b89..8e79b48 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,10 @@ module github.com/kubernetes-sigs/yaml go 1.12 -require gopkg.in/yaml.v2 v2.2.2 +require ( + github.com/json-iterator/go v1.1.6 + github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect + github.com/modern-go/reflect2 v1.0.1 // indirect + github.com/stretchr/testify v1.3.0 // indirect + gopkg.in/yaml.v2 v2.2.2 +) diff --git a/go.sum b/go.sum index bd555a3..1de24c2 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,17 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/json-iterator/go v1.1.6 h1:MrUvLMLTMxbqFJ9kzlvat/rYZqZnW3u4wkLzWTaFwKs= +github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= +github.com/modern-go/reflect2 v1.0.1 h1:9f412s+6RmYXLWZSEzVVgPGK7C2PphHj5RJrvfx9AWI= +github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/yaml.go b/yaml.go index 0245961..b49ca93 100644 --- a/yaml.go +++ b/yaml.go @@ -4,17 +4,60 @@ import ( "bytes" "encoding/json" "fmt" - "io" "reflect" "strconv" + jsoniter "github.com/json-iterator/go" "gopkg.in/yaml.v2" ) +// The json-iterator implementation is based on: +// https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go + +type customNumberExtension struct { + jsoniter.DummyExtension +} + +// createJSONIterator returns a jsoniterator API that's configured to be +// compatible with the encoding/json standard library. +func createJSONIterator() jsoniter.API { + config := jsoniter.Config{ + EscapeHTML: true, + SortMapKeys: true, + ValidateJsonRawMessage: true, + }.Froze() + // Force jsoniter to decode number to interface{} via int64/float64, if possible. + config.RegisterExtension(&customNumberExtension{}) + return config +} + +// createCaseSensitiveStrictJSONIterator returns a jsoniterator API that's configured to be +// case-sensitive, but also disallow unknown fields when unmarshalling. It is compatible with +// the encoding/json standard library. +func createCaseSensitiveStrictJSONIterator() jsoniter.API { + config := jsoniter.Config{ + EscapeHTML: true, + SortMapKeys: true, + ValidateJsonRawMessage: true, + CaseSensitive: true, + DisallowUnknownFields: true, + }.Froze() + // Force jsoniter to decode number to interface{} via int64/float64, if possible. + config.RegisterExtension(&customNumberExtension{}) + return config +} + +// Private copies of jsoniter to try to shield against possible mutations +// from outside. Still does not protect from package level jsoniter.Register*() functions - someone calling them +// in some other library will mess with every usage of the jsoniter library in the whole program. +// See https://github.com/json-iterator/go/issues/265 +var jsonIterator = createJSONIterator() +var caseSensitiveStrictJSONIterator = createCaseSensitiveStrictJSONIterator() + // Marshal marshals the object into JSON then converts JSON to YAML and returns the // YAML. func Marshal(o interface{}) ([]byte, error) { - j, err := json.Marshal(o) + j, err := caseSensitiveStrictJSONIterator.Marshal(o) if err != nil { return nil, fmt.Errorf("error marshaling into JSON: %v", err) } @@ -28,23 +71,30 @@ func Marshal(o interface{}) ([]byte, error) { } // JSONOpt is a decoding option for decoding from JSON format. +// Deprecated as the backend for parsing JSON is now json-iter. type JSONOpt func(*json.Decoder) *json.Decoder -// Unmarshal converts YAML to JSON then uses JSON to unmarshal into an object, -// optionally configuring the behavior of the JSON unmarshal. +// Unmarshal converts YAML to JSON then uses JSON to unmarshal into an object. +// Usage of JSONOpt is deprecated. func Unmarshal(y []byte, o interface{}, opts ...JSONOpt) error { - return yamlUnmarshal(y, o, false, opts...) + return yamlUnmarshal(y, o, false, jsonIterator) } // UnmarshalStrict strictly converts YAML to JSON then uses JSON to unmarshal -// into an object, optionally configuring the behavior of the JSON unmarshal. +// into an object. Usage of JSONOpt is deprecated. func UnmarshalStrict(y []byte, o interface{}, opts ...JSONOpt) error { - return yamlUnmarshal(y, o, true, append(opts, DisallowUnknownFields)...) + return yamlUnmarshal(y, o, true, caseSensitiveStrictJSONIterator) +} + +// UnmarshalWithConfig converts YAML to JSON, optionally by using strict mode and then +// uses a custom json-iterator Config object to unmarshal the JSON bytes. +func UnmarshalWithConfig(y []byte, o interface{}, strictYAML bool, jsonConfig jsoniter.API) error { + return yamlUnmarshal(y, o, strictYAML, jsonConfig) } // yamlUnmarshal unmarshals the given YAML byte stream into the given interface, -// optionally performing the unmarshalling strictly -func yamlUnmarshal(y []byte, o interface{}, strict bool, opts ...JSONOpt) error { +// optionally performing the unmarshalling strictly. +func yamlUnmarshal(y []byte, o interface{}, strict bool, jsonConfig jsoniter.API) error { vo := reflect.ValueOf(o) unmarshalFn := yaml.Unmarshal if strict { @@ -55,27 +105,7 @@ func yamlUnmarshal(y []byte, o interface{}, strict bool, opts ...JSONOpt) error return fmt.Errorf("error converting YAML to JSON: %v", err) } - err = jsonUnmarshal(bytes.NewReader(j), o, opts...) - if err != nil { - return fmt.Errorf("error unmarshaling JSON: %v", err) - } - - return nil -} - -// jsonUnmarshal unmarshals the JSON byte stream from the given reader into the -// object, optionally applying decoder options prior to decoding. We are not -// using json.Unmarshal directly as we want the chance to pass in non-default -// options. -func jsonUnmarshal(r io.Reader, o interface{}, opts ...JSONOpt) error { - d := json.NewDecoder(r) - for _, opt := range opts { - d = opt(d) - } - if err := d.Decode(&o); err != nil { - return fmt.Errorf("while decoding JSON: %v", err) - } - return nil + return jsonConfig.Unmarshal(j, &o) } // JSONToYAML Converts JSON to YAML. @@ -136,7 +166,7 @@ func yamlToJSON(y []byte, jsonTarget *reflect.Value, yamlUnmarshal func([]byte, } // Convert this object to JSON and return the data. - return json.Marshal(jsonObj) + return caseSensitiveStrictJSONIterator.Marshal(jsonObj) } func convertToJSONableObject(yamlObj interface{}, jsonTarget *reflect.Value) (interface{}, error) { diff --git a/yaml_go110.go b/yaml_go110.go deleted file mode 100644 index ab3e06a..0000000 --- a/yaml_go110.go +++ /dev/null @@ -1,14 +0,0 @@ -// This file contains changes that are only compatible with go 1.10 and onwards. - -// +build go1.10 - -package yaml - -import "encoding/json" - -// DisallowUnknownFields configures the JSON decoder to error out if unknown -// fields come along, instead of dropping them by default. -func DisallowUnknownFields(d *json.Decoder) *json.Decoder { - d.DisallowUnknownFields() - return d -} diff --git a/yaml_go110_test.go b/yaml_go110_test.go deleted file mode 100644 index e4707e8..0000000 --- a/yaml_go110_test.go +++ /dev/null @@ -1,46 +0,0 @@ -// +build go1.10 - -package yaml - -import ( - "fmt" - "testing" -) - -func TestUnmarshalWithTags(t *testing.T) { - type WithTaggedField struct { - Field string `json:"field"` - } - - t.Run("Known tagged field", func(t *testing.T) { - y := []byte(`field: "hello"`) - v := WithTaggedField{} - if err := Unmarshal(y, &v, DisallowUnknownFields); err != nil { - t.Errorf("unexpected error: %v", err) - } - if v.Field != "hello" { - t.Errorf("v.Field=%v, want 'hello'", v.Field) - } - - }) - t.Run("With unknown tagged field", func(t *testing.T) { - y := []byte(`unknown: "hello"`) - v := WithTaggedField{} - err := Unmarshal(y, &v, DisallowUnknownFields) - if err == nil { - t.Errorf("want error because of unknown field, got : v=%#v", v) - } - }) - -} - -func exampleUnknown() { - type WithTaggedField struct { - Field string `json:"field"` - } - y := []byte(`unknown: "hello"`) - v := WithTaggedField{} - fmt.Printf("%v\n", Unmarshal(y, &v, DisallowUnknownFields)) - // Ouptut: - // unmarshaling JSON: while decoding JSON: json: unknown field "unknown" -} diff --git a/yaml_test.go b/yaml_test.go index 42a2315..dac2136 100644 --- a/yaml_test.go +++ b/yaml_test.go @@ -6,6 +6,8 @@ import ( "reflect" "strconv" "testing" + + jsoniter "github.com/json-iterator/go" ) type MarshalTest struct { @@ -33,8 +35,8 @@ func TestMarshal(t *testing.T) { } type UnmarshalString struct { - A string - True string + A string + B string } type UnmarshalStringMap struct { @@ -69,9 +71,9 @@ func TestUnmarshal(t *testing.T) { e1 = UnmarshalString{A: "true"} unmarshal(t, y, &s1, &e1) - y = []byte("true: 1") + y = []byte("b: true") s1 = UnmarshalString{} - e1 = UnmarshalString{True: "1"} + e1 = UnmarshalString{B: "true"} unmarshal(t, y, &s1, &e1) y = []byte("a:\n a: 1") @@ -119,34 +121,34 @@ func unmarshal(t *testing.T, y []byte, s, e interface{}, opts ...JSONOpt) { } func TestUnmarshalStrict(t *testing.T) { - y := []byte("a: 1") + y := []byte("A: 1") s1 := UnmarshalString{} e1 := UnmarshalString{A: "1"} unmarshalStrict(t, y, &s1, &e1) - y = []byte("a: true") + y = []byte("A: true") s1 = UnmarshalString{} e1 = UnmarshalString{A: "true"} unmarshalStrict(t, y, &s1, &e1) - y = []byte("true: 1") + y = []byte("B: true") s1 = UnmarshalString{} - e1 = UnmarshalString{True: "1"} + e1 = UnmarshalString{B: "true"} unmarshalStrict(t, y, &s1, &e1) - y = []byte("a:\n a: 1") + y = []byte("A:\n A: 1") s2 := UnmarshalNestedString{} e2 := UnmarshalNestedString{NestedString{"1"}} unmarshalStrict(t, y, &s2, &e2) - y = []byte("a:\n - b: abc\n c: def\n - b: 123\n c: 456\n") + y = []byte("A:\n - B: abc\n C: def\n - B: 123\n C: 456\n") s3 := UnmarshalSlice{} e3 := UnmarshalSlice{[]NestedSlice{NestedSlice{"abc", strPtr("def")}, NestedSlice{"123", strPtr("456")}}} unmarshalStrict(t, y, &s3, &e3) - y = []byte("a:\n b: 1") + y = []byte("A:\n B: 1") s4 := UnmarshalStringMap{} - e4 := UnmarshalStringMap{map[string]string{"b": "1"}} + e4 := UnmarshalStringMap{map[string]string{"B": "1"}} unmarshalStrict(t, y, &s4, &e4) y = []byte(` @@ -185,15 +187,15 @@ a: } func TestUnmarshalStrictFails(t *testing.T) { - y := []byte("a: true\na: false") + y := []byte("A: true\nA: false") s1 := UnmarshalString{} unmarshalStrictFail(t, y, &s1) - y = []byte("a:\n - b: abc\n c: 32\n b: 123") + y = []byte("A:\n - B: abc\n C: 32\n B: 123") s2 := UnmarshalSlice{} unmarshalStrictFail(t, y, &s2) - y = []byte("a:\n b: 1\n c: 3") + y = []byte("A:\n B: 1\n C: 3") s3 := UnmarshalStringMap{} unmarshalStrictFail(t, y, &s3) @@ -220,6 +222,12 @@ unknown: Some-Value `) s5 := NamedThing{} unmarshalStrictFail(t, y, &s5) + + // Strict unmarshal should fail for case-sensitive fields; 'a' should be 'A'. + y = []byte("a: test") + s6 := UnmarshalString{} + unmarshalStrictFail(t, y, &s6) + } func unmarshalStrict(t *testing.T, y []byte, s, e interface{}, opts ...JSONOpt) { @@ -421,3 +429,25 @@ foo: baz t.Error("expected YAMLtoJSONStrict to fail on duplicate field names") } } + +func TestUnmarshalWithConfig(t *testing.T) { + config := jsoniter.Config{ + EscapeHTML: true, + SortMapKeys: true, + ValidateJsonRawMessage: true, + CaseSensitive: true, + DisallowUnknownFields: true, + }.Froze() + + b := []byte("A: test") + s := &UnmarshalString{} + e := &UnmarshalString{A: "test"} + + if err := UnmarshalWithConfig(b, s, true, config); err != nil { + t.Fatal("expected no error when using UnmarshalWithConfig") + } + if !reflect.DeepEqual(s, e) { + t.Fatalf("unmarshal YAML was unsuccessful, expected: %+#v, got: %+#v", + e, s) + } +}