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

## Description Closes: #12894 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [x] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
2022-08-19 18:42:55 +00:00
parent f430528ebc
commit 28a7e33bc9
13 changed files with 1009 additions and 118 deletions
@@ -26,35 +26,47 @@ func NewTestInterfaceRegistry() types.InterfaceRegistry {
 }

 func TestMarshalAny(t *testing.T) {
+	catRegistry := types.NewInterfaceRegistry()
+	catRegistry.RegisterImplementations((*testdata.Animal)(nil), &testdata.Cat{})
+
 	registry := types.NewInterfaceRegistry()

 	cdc := codec.NewProtoCodec(registry)

 	kitty := &testdata.Cat{Moniker: "Kitty"}
-	bz, err := cdc.MarshalInterface(kitty)
+	emptyBz, err := cdc.MarshalInterface(kitty)
+	require.ErrorContains(t, err, "does not have a registered interface")
+
+	catBz, err := codec.NewProtoCodec(catRegistry).MarshalInterface(kitty)
 	require.NoError(t, err)
+	require.NotEmpty(t, catBz)

 	var animal testdata.Animal

-	// empty registry should fail
-	err = cdc.UnmarshalInterface(bz, &animal)
-	require.Error(t, err)
+	// deserializing cat bytes should error in an empty registry
+	err = cdc.UnmarshalInterface(catBz, &animal)
+	require.ErrorContains(t, err, "no registered implementations of type testdata.Animal")
+
+	// deserializing an empty byte array will return nil, but no error
+	err = cdc.UnmarshalInterface(emptyBz, &animal)
+	require.Nil(t, animal)
+	require.NoError(t, err)

 	// wrong type registration should fail
 	registry.RegisterImplementations((*testdata.Animal)(nil), &testdata.Dog{})
-	err = cdc.UnmarshalInterface(bz, &animal)
+	err = cdc.UnmarshalInterface(catBz, &animal)
 	require.Error(t, err)

 	// should pass
 	registry = NewTestInterfaceRegistry()
 	cdc = codec.NewProtoCodec(registry)
-	err = cdc.UnmarshalInterface(bz, &animal)
+	err = cdc.UnmarshalInterface(catBz, &animal)
 	require.NoError(t, err)
 	require.Equal(t, kitty, animal)

 	// nil should fail
 	registry = NewTestInterfaceRegistry()
-	err = cdc.UnmarshalInterface(bz, nil)
+	err = cdc.UnmarshalInterface(catBz, nil)
 	require.Error(t, err)
 }

@@ -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)
 }
@@ -3,6 +3,7 @@ package codec_test
 import (
 	"errors"
 	"fmt"
+	"reflect"
 	"testing"

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

+func TestEnsureRegistered(t *testing.T) {
+	interfaceRegistry := types.NewInterfaceRegistry()
+	cat := &testdata.Cat{Moniker: "Garfield"}
+
+	err := interfaceRegistry.EnsureRegistered(*cat)
+	require.ErrorContains(t, err, "testdata.Cat is not a pointer")
+
+	err = interfaceRegistry.EnsureRegistered(cat)
+	require.ErrorContains(t, err, "testdata.Cat does not have a registered interface")
+
+	interfaceRegistry.RegisterInterface("testdata.Animal",
+		(*testdata.Animal)(nil),
+		&testdata.Cat{},
+	)
+
+	require.NoError(t, interfaceRegistry.EnsureRegistered(cat))
+}
+
+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())

@@ -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 interface for the given concrete type.
+	EnsureRegistered(iface interface{}) error
 }

 // UnpackInterfacesMessage is meant to extend protobuf types (which implement
@@ -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
 }

@@ -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{},
 	}
 }
@@ -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 interface 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.
 //
@@ -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
 }