Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(codec): MarshalInterface should err when UnmarshalInterface will fail #12964

Merged
merged 13 commits into from
Aug 19, 2022
Merged
4 changes: 4 additions & 0 deletions client/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,7 @@ func (f failingInterfaceRegistry) ListAllInterfaces() []string {
func (f failingInterfaceRegistry) ListImplementations(ifaceTypeURL string) []string {
panic("cannot be called")
}

func (f failingInterfaceRegistry) EnsureRegistered(iface interface{}) error {
panic("cannot be called")
}
4 changes: 4 additions & 0 deletions codec/proto_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ func (pc *ProtoCodec) MarshalInterface(i gogoproto.Message) ([]byte, error) {
if err != nil {
return nil, err
}
err = pc.interfaceRegistry.EnsureRegistered(i)
if err != nil {
return nil, err
}

return pc.Marshal(any)
}
Expand Down
47 changes: 47 additions & 0 deletions codec/proto_codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package codec_test
import (
"errors"
"fmt"
"reflect"
"testing"

"github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -46,6 +47,52 @@ func (lpm *lyingProtoMarshaler) Size() int {
return lpm.falseSize
}

func TestProtoCodecMarshal(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
interfaceRegistry.RegisterInterface("testdata.Animal",
(*testdata.Animal)(nil),
&testdata.Cat{},
)
cdc := codec.NewProtoCodec(interfaceRegistry)

cartonRegistry := types.NewInterfaceRegistry()
cartonRegistry.RegisterInterface("testdata.Cartoon",
(*testdata.Cartoon)(nil),
&testdata.Bird{},
)
cartoonCdc := codec.NewProtoCodec(cartonRegistry)

cat := &testdata.Cat{Moniker: "Garfield", Lives: 6}
bird := &testdata.Bird{Species: "Passerina ciris"}
require.NoError(t, interfaceRegistry.EnsureRegistered(cat))

var (
animal testdata.Animal
cartoon testdata.Cartoon
)

// sanity check
require.True(t, reflect.TypeOf(cat).Implements(reflect.TypeOf((*testdata.Animal)(nil)).Elem()))

bz, err := cdc.MarshalInterface(cat)
require.NoError(t, err)

err = cdc.UnmarshalInterface(bz, &animal)
require.NoError(t, err)

bz, err = cdc.MarshalInterface(bird)
require.ErrorContains(t, err, "does not have a registered interface")

bz, err = cartoonCdc.MarshalInterface(bird)
require.NoError(t, err)

err = cdc.UnmarshalInterface(bz, &cartoon)
require.ErrorContains(t, err, "no registered implementations")

err = cartoonCdc.UnmarshalInterface(bz, &cartoon)
require.NoError(t, err)
}

func TestProtoCodecUnmarshalLengthPrefixedChecks(t *testing.T) {
cdc := codec.NewProtoCodec(createTestInterfaceRegistry())

Expand Down
23 changes: 22 additions & 1 deletion codec/types/interface_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ type InterfaceRegistry interface {
// ListImplementations lists the valid type URLs for the given interface name that can be used
// for the provided interface type URL.
ListImplementations(ifaceTypeURL string) []string

// EnsureRegistered ensures there is a registered implementation for the given concrete type.
EnsureRegistered(iface interface{}) error
}

// UnpackInterfacesMessage is meant to extend protobuf types (which implement
Expand Down Expand Up @@ -81,6 +84,7 @@ type UnpackInterfacesMessage interface {
type interfaceRegistry struct {
interfaceNames map[string]reflect.Type
interfaceImpls map[reflect.Type]interfaceMap
implInterfaces map[reflect.Type]reflect.Type
typeURLMap map[string]reflect.Type
}

Expand All @@ -91,6 +95,7 @@ func NewInterfaceRegistry() InterfaceRegistry {
return &interfaceRegistry{
interfaceNames: map[string]reflect.Type{},
interfaceImpls: map[reflect.Type]interfaceMap{},
implInterfaces: map[reflect.Type]reflect.Type{},
typeURLMap: map[string]reflect.Type{},
}
}
Expand All @@ -100,10 +105,26 @@ func (registry *interfaceRegistry) RegisterInterface(protoName string, iface int
if typ.Elem().Kind() != reflect.Interface {
panic(fmt.Errorf("%T is not an interface type", iface))
}

registry.interfaceNames[protoName] = typ
registry.RegisterImplementations(iface, impls...)
}

// EnsureRegistered ensures there is a registered implementation for the given concrete type.
//
// Returns an error if not, and nil if so.
func (registry *interfaceRegistry) EnsureRegistered(impl interface{}) error {
if reflect.ValueOf(impl).Kind() != reflect.Ptr {
return fmt.Errorf("%T is not a pointer", impl)
}

if _, found := registry.implInterfaces[reflect.TypeOf(impl)]; !found {
return fmt.Errorf("%T does not have a registered interface", impl)
}

return nil
}

// RegisterImplementations registers a concrete proto Message which implements
// the given interface.
//
Expand Down Expand Up @@ -162,7 +183,7 @@ func (registry *interfaceRegistry) registerImpl(iface interface{}, typeURL strin

imap[typeURL] = implType
registry.typeURLMap[typeURL] = implType

registry.implInterfaces[implType] = ityp
registry.interfaceImpls[ityp] = imap
}

Expand Down
12 changes: 11 additions & 1 deletion testutil/testdata/animal.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,20 @@ type Animal interface {
Greet() string
}

func (c Cat) Greet() string {
type Cartoon interface {
proto.Message

Identify() string
}

func (c *Cat) Greet() string {
return fmt.Sprintf("Meow, my name is %s", c.Moniker)
}

func (c *Bird) Identify() string {
return "This is Tweety."
}

func (d Dog) Greet() string {
return fmt.Sprintf("Roof, my name is %s", d.Name)
}
Expand Down
Loading