diff --git a/pointer.go b/pointer.go index ebc0390..7df49af 100644 --- a/pointer.go +++ b/pointer.go @@ -45,6 +45,7 @@ type JSONSetable interface { // - a go map[K]V is interpreted as an object, with type K assignable to a string // - a go slice []T is interpreted as an array // - a go struct is interpreted as an object, with exported fields interpreted as keys +// - promoted fields from an embedded struct are traversed // - scalars (e.g. int, float64 ...), channels, functions and go arrays cannot be traversed // // For struct s resolved by reflection, key mappings honor the conventional struct tag `json`. @@ -54,7 +55,7 @@ type JSONSetable interface { // # Limitations // // - Unlike go standard marshaling, untagged fields do not default to the go field name and are ignored. -// - anonymous embedded fields are not traversed +// - anonymous fields are not traversed if untagged type Pointer struct { referenceTokens []string } @@ -362,7 +363,7 @@ func getSingleImpl(node any, decodedToken string, nameProvider *jsonname.NamePro case reflect.Slice: tokenIndex, err := strconv.Atoi(decodedToken) if err != nil { - return nil, kind, err + return nil, kind, errors.Join(err, ErrPointer) } sLength := rValue.Len() if tokenIndex < 0 || tokenIndex >= sLength { @@ -378,9 +379,7 @@ func getSingleImpl(node any, decodedToken string, nameProvider *jsonname.NamePro } func setSingleImpl(node, data any, decodedToken string, nameProvider *jsonname.NameProvider) error { - rValue := reflect.Indirect(reflect.ValueOf(node)) - - // Check for nil to prevent panic when calling rValue.Type() + // check for nil to prevent panic when calling rValue.Type() if isNil(node) { return fmt.Errorf("cannot set field %q on nil value: %w", decodedToken, ErrPointer) } @@ -389,28 +388,44 @@ func setSingleImpl(node, data any, decodedToken string, nameProvider *jsonname.N return ns.JSONSet(decodedToken, data) } + rValue := reflect.Indirect(reflect.ValueOf(node)) + switch rValue.Kind() { case reflect.Struct: nm, ok := nameProvider.GetGoNameForType(rValue.Type(), decodedToken) if !ok { return fmt.Errorf("object has no field %q: %w", decodedToken, ErrPointer) } + fld := rValue.FieldByName(nm) - if fld.IsValid() { - fld.Set(reflect.ValueOf(data)) + if !fld.CanSet() { + return fmt.Errorf("can't set struct field %s to %v: %w", nm, data, ErrPointer) } + + value := reflect.ValueOf(data) + valueType := value.Type() + assignedType := fld.Type() + + if !valueType.AssignableTo(assignedType) { + return fmt.Errorf("can't set value with type %T to field %s with type %v: %w", data, nm, assignedType, ErrPointer) + } + + fld.Set(value) + return nil case reflect.Map: kv := reflect.ValueOf(decodedToken) rValue.SetMapIndex(kv, reflect.ValueOf(data)) + return nil case reflect.Slice: tokenIndex, err := strconv.Atoi(decodedToken) if err != nil { - return err + return errors.Join(err, ErrPointer) } + sLength := rValue.Len() if tokenIndex < 0 || tokenIndex >= sLength { return errOutOfBounds(sLength, tokenIndex) @@ -420,7 +435,17 @@ func setSingleImpl(node, data any, decodedToken string, nameProvider *jsonname.N if !elem.CanSet() { return fmt.Errorf("can't set slice index %s to %v: %w", decodedToken, data, ErrPointer) } - elem.Set(reflect.ValueOf(data)) + + value := reflect.ValueOf(data) + valueType := value.Type() + assignedType := elem.Type() + + if !valueType.AssignableTo(assignedType) { + return fmt.Errorf("can't set value with type %T to slice element %d with type %v: %w", data, tokenIndex, assignedType, ErrPointer) + } + + elem.Set(value) + return nil default: diff --git a/pointer_test.go b/pointer_test.go index 4adf5a7..4c68758 100644 --- a/pointer_test.go +++ b/pointer_test.go @@ -6,9 +6,11 @@ package jsonpointer import ( "encoding/json" "fmt" + "reflect" "strconv" "testing" + "github.com/go-openapi/swag/jsonname" "github.com/go-openapi/testify/v2/assert" "github.com/go-openapi/testify/v2/require" ) @@ -246,6 +248,7 @@ func TestPointableInterface(t *testing.T) { t.Run("with pointable type", func(t *testing.T) { p := &pointableImpl{"hello"} + result, _, err := GetForToken(p, "some") require.NoError(t, err) assert.Equal(t, p.a, result) @@ -346,6 +349,95 @@ func TestArray(t *testing.T) { } } +func TestStruct(t *testing.T) { + t.Parallel() + + t.Run("with untagged struct field", func(t *testing.T) { + type Embedded struct { + D int `json:"d"` + } + + s := struct { + Embedded + + A int `json:"a"` + B int + Anonymous struct { + C int `json:"c"` + } + }{} + + { + s.A = 1 + s.B = 2 + s.Anonymous.C = 3 + s.D = 4 + } + + t.Run(`should resolve field A tagged "a"`, func(t *testing.T) { + pointerA, err := New("/a") + require.NoError(t, err) + + value, kind, err := pointerA.Get(s) + require.NoError(t, err) + require.Equal(t, reflect.Int, kind) + require.Equal(t, 1, value) + + _, err = pointerA.Set(&s, 9) + require.NoError(t, err) + + value, _, err = pointerA.Get(s) + require.NoError(t, err) + require.Equal(t, 9, value) + }) + + t.Run(`should resolve embedded field D with tag`, func(t *testing.T) { + pointerD, err := New("/d") + require.NoError(t, err) + + value, kind, err := pointerD.Get(s) + require.NoError(t, err) + require.Equal(t, reflect.Int, kind) + require.Equal(t, 4, value) + + _, err = pointerD.Set(&s, 6) + require.NoError(t, err) + + value, _, err = pointerD.Get(s) + require.NoError(t, err) + require.Equal(t, 6, value) + }) + + t.Run("with known limitations", func(t *testing.T) { + t.Run(`should not resolve field B without tag`, func(t *testing.T) { + pointerB, err := New("/B") + require.NoError(t, err) + + _, _, err = pointerB.Get(s) + require.Error(t, err) + require.ErrorContains(t, err, `has no field "B"`) + + _, err = pointerB.Set(&s, 8) + require.Error(t, err) + require.ErrorContains(t, err, `has no field "B"`) + }) + + t.Run(`should not resolve field C with tag, but anonymous`, func(t *testing.T) { + pointerC, err := New("/c") + require.NoError(t, err) + + _, _, err = pointerC.Get(s) + require.Error(t, err) + require.ErrorContains(t, err, `has no field "c"`) + + _, err = pointerC.Set(&s, 7) + require.Error(t, err) + require.ErrorContains(t, err, `has no field "c"`) + }) + }) + }) +} + func TestOtherThings(t *testing.T) { t.Parallel() @@ -367,11 +459,21 @@ func TestOtherThings(t *testing.T) { }) t.Run("out of bound array index should error", func(t *testing.T) { - p, err := New("/foo/3") - require.NoError(t, err) + t.Run("with index overflow", func(t *testing.T) { + p, err := New("/foo/3") + require.NoError(t, err) - _, _, err = p.Get(testDocumentJSON(t)) - require.Error(t, err) + _, _, err = p.Get(testDocumentJSON(t)) + require.Error(t, err) + }) + + t.Run("with index unerflow", func(t *testing.T) { + p, err := New("/foo/-3") + require.NoError(t, err) + + _, _, err = p.Get(testDocumentJSON(t)) + require.Error(t, err) + }) }) t.Run("referring to a key in an array should error", func(t *testing.T) { @@ -907,4 +1009,101 @@ func TestEdgeCases(t *testing.T) { require.Equal(t, doc, newDoc) }) + + t.Run("with out of bounds index", func(t *testing.T) { + p, err := New("/foo/10") + require.NoError(t, err) + + t.Run("should error on Get", func(t *testing.T) { + _, _, err := p.Get(testStructJSONDoc(t)) + require.Error(t, err) + require.ErrorContains(t, err, "index out of bounds") + }) + + t.Run("should error on Set", func(t *testing.T) { + _, err := p.Set(testStructJSONPtr(t), "peek-a-boo") + require.Error(t, err) + require.ErrorContains(t, err, "index out of bounds") + }) + }) + + t.Run("Set with invalid pointer token", func(t *testing.T) { + doc := testStructJSONDoc(t) + pointer, err := New("/foo/x") + require.NoError(t, err) + + _, err = pointer.Set(&doc, "yay") + require.Error(t, err) + require.ErrorContains(t, err, `Atoi: parsing "x"`) + }) + + t.Run("Set with invalid reference in struct", func(t *testing.T) { + doc := struct { + A func() `json:"a"` + B []int `json:"b"` + }{ + A: func() {}, + B: []int{0, 1}, + } + + t.Run("should error when attempting to set a struct field value that is not assignable", func(t *testing.T) { + pointerA, err := New("/a") + require.NoError(t, err) + + _, err = pointerA.Set(&doc, "waou") + require.Error(t, err) + require.ErrorContains(t, err, `can't set value with type string to field A`) + }) + + t.Run("should error when attempting to set a slice element value that is not assignable", func(t *testing.T) { + pointerB, err := New("/b/0") + require.NoError(t, err) + + _, err = pointerB.Set(&doc, "waou") + require.Error(t, err) + require.ErrorContains(t, err, `can't set value with type string to slice element 0 with type int`) + }) + + t.Run("should error when attempting to set a value that does not exist", func(t *testing.T) { + pointerB, err := New("/x") + require.NoError(t, err) + + _, _, err = pointerB.Get(&doc) + require.Error(t, err) + require.ErrorContains(t, err, `no field`) + + _, err = pointerB.Set(&doc, "oops") + require.Error(t, err) + require.ErrorContains(t, err, `no field`) + }) + }) +} + +func TestInternalEdgeCases(t *testing.T) { + t.Parallel() + + t.Run("setSingleImpl should error on any node not a struct, map or slice", func(t *testing.T) { + var node int + + err := setSingleImpl(&node, 3, "a", jsonname.DefaultJSONNameProvider) + require.Error(t, err) + require.ErrorContains(t, err, `invalid token reference "a"`) + }) + + t.Run("with simulated unsettable", func(t *testing.T) { + type unsettable struct { + A string `json:"a"` + } + doc := unsettable{ + A: "a", + } + + t.Run("setSingleImpl should error on struct field that is not settable", func(t *testing.T) { + node := doc // doesn't pass a pointer: unsettable + + err := setSingleImpl(node, "new value", "a", jsonname.DefaultJSONNameProvider) + require.Error(t, err) + require.ErrorContains(t, err, `can't set struct field`) + }) + }) } diff --git a/struct_example_test.go b/struct_example_test.go index 31fa6f7..2986902 100644 --- a/struct_example_test.go +++ b/struct_example_test.go @@ -10,6 +10,8 @@ import ( var ErrExampleIface = errors.New("example error") type ExampleDoc struct { + PromotedDoc + Promoted EmbeddedDoc `json:"promoted"` AnonPromoted EmbeddedDoc `json:"-"` A string `json:"propA"` @@ -23,8 +25,15 @@ type EmbeddedDoc struct { B string `json:"propB"` } +type PromotedDoc struct { + C string `json:"propC"` +} + func Example_struct() { doc := ExampleDoc{ + PromotedDoc: PromotedDoc{ + C: "c", + }, Promoted: EmbeddedDoc{ B: "promoted", }, @@ -43,15 +52,27 @@ func Example_struct() { } fmt.Printf("a: %v\n", a) } + { - // tagged embedded field is resolved + // tagged struct field is resolved pointerB, _ := jsonpointer.New("/promoted/propB") - a, _, err := pointerB.Get(doc) + b, _, err := pointerB.Get(doc) + if err != nil { + fmt.Println(err) + return + } + fmt.Printf("b: %v\n", b) + } + + { + // tagged embedded field is resolved + pointerC, _ := jsonpointer.New("/propC") + c, _, err := pointerC.Get(doc) if err != nil { fmt.Println(err) return } - fmt.Printf("b: %v\n", a) + fmt.Printf("c: %v\n", c) } { @@ -69,7 +90,7 @@ func Example_struct() { } { - // Limitation: anonymous embedded field is not resolved. + // Limitation: anonymous field is not resolved. pointerC, _ := jsonpointer.New("/propB") _, _, err := pointerC.Get(doc) fmt.Printf("anonymous: %v\n", err) @@ -85,6 +106,7 @@ func Example_struct() { // output: // a: a // b: promoted + // c: c // ignored: object has no field "ignored": JSON pointer error // unexported: object has no field "unexported": JSON pointer error // anonymous: object has no field "propB": JSON pointer error