internal/impl: handle irregular messages implementing proto.Message

When encountering a type that does not have a MessageInfo, don't assume
that it's a legacy message that doesn't implement proto.Message. Add a
set of test messages exercising this case (panics prior to the
internal/impl change).

Change-Id: Ic1ec5ecfbe92278fbef44284ff52a0e0622a158c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/182477
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
This commit is contained in:
Damien Neil 2019-06-14 11:54:07 -07:00
parent 05e11b806f
commit 5b6d0471e5
7 changed files with 409 additions and 5 deletions

View File

@ -105,10 +105,11 @@ func generateLocalProtos() {
path string
grpcPlugin bool
annotateFor map[string]bool
exclude map[string]bool
}{
{path: "cmd/protoc-gen-go/testdata", annotateFor: map[string]bool{"annotations/annotations.proto": true}},
{path: "cmd/protoc-gen-go-grpc/testdata", grpcPlugin: true},
{path: "internal/testprotos"},
{path: "internal/testprotos", exclude: map[string]bool{"irregular/irregular.proto": true}},
{path: "encoding/testprotos"},
{path: "reflect/protoregistry/testprotos"},
}
@ -128,6 +129,10 @@ func generateLocalProtos() {
check(err)
subDirs[filepath.Dir(relPath)] = true
if d.exclude[filepath.ToSlash(relPath)] {
return nil
}
// Emit a .meta file for certain files.
var opts string
if d.annotateFor[filepath.ToSlash(relPath)] {

View File

@ -82,11 +82,11 @@ func makeMessageFieldCoder(fd pref.FieldDescriptor, ft reflect.Type) pointerCode
} else {
return pointerCoderFuncs{
size: func(p pointer, tagsize int, opts marshalOptions) int {
m := legacyWrapMessage(p.AsValueOf(ft).Elem())
m := asMessage(p.AsValueOf(ft).Elem())
return sizeMessage(m, tagsize, opts)
},
marshal: func(b []byte, p pointer, wiretag uint64, opts marshalOptions) ([]byte, error) {
m := legacyWrapMessage(p.AsValueOf(ft).Elem())
m := asMessage(p.AsValueOf(ft).Elem())
return appendMessage(b, m, wiretag, opts)
},
}
@ -141,11 +141,11 @@ func makeGroupFieldCoder(fd pref.FieldDescriptor, ft reflect.Type) pointerCoderF
} else {
return pointerCoderFuncs{
size: func(p pointer, tagsize int, opts marshalOptions) int {
m := legacyWrapMessage(p.AsValueOf(ft).Elem())
m := asMessage(p.AsValueOf(ft).Elem())
return sizeGroup(m, tagsize, opts)
},
marshal: func(b []byte, p pointer, wiretag uint64, opts marshalOptions) ([]byte, error) {
m := legacyWrapMessage(p.AsValueOf(ft).Elem())
m := asMessage(p.AsValueOf(ft).Elem())
return appendGroup(b, m, wiretag, opts)
},
}
@ -492,3 +492,10 @@ var coderStringIfaceValidateUTF8 = ifaceCoderFuncs{
size: sizeStringIfaceValidateUTF8,
marshal: appendStringIfaceValidateUTF8,
}
func asMessage(v reflect.Value) pref.ProtoMessage {
if m, ok := v.Interface().(pref.ProtoMessage); ok {
return m
}
return legacyWrapMessage(v)
}

View File

@ -0,0 +1,141 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package irregular
import (
"google.golang.org/protobuf/encoding/prototext"
"google.golang.org/protobuf/reflect/protodesc"
pref "google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/descriptorpb"
)
type IrregularMessage struct {
set bool
value string
}
func (m *IrregularMessage) ProtoReflect() pref.Message { return (*message)(m) }
type message IrregularMessage
func (m *message) Descriptor() pref.MessageDescriptor { return descriptor.Messages().Get(0) }
func (m *message) Type() pref.MessageType { return nil }
func (m *message) KnownFields() pref.KnownFields { return (*known)(m) }
func (m *message) UnknownFields() pref.UnknownFields { return (*unknown)(m) }
func (m *message) New() pref.Message { return &message{} }
func (m *message) Interface() pref.ProtoMessage { return (*IrregularMessage)(m) }
type known IrregularMessage
func (m *known) Len() int {
if m.set {
return 1
}
return 0
}
func (m *known) Has(num pref.FieldNumber) bool {
switch num {
case fieldS:
return m.set
}
return false
}
func (m *known) Get(num pref.FieldNumber) pref.Value {
switch num {
case fieldS:
return pref.ValueOf(m.value)
}
return pref.Value{}
}
func (m *known) Set(num pref.FieldNumber, v pref.Value) {
switch num {
case fieldS:
m.value = v.String()
default:
panic("unknown field")
}
}
func (m *known) Clear(num pref.FieldNumber) {
switch num {
case fieldS:
m.value = ""
m.set = false
default:
panic("unknown field")
}
}
func (m *known) WhichOneof(name pref.Name) pref.FieldNumber {
return 0
}
func (m *known) Range(f func(pref.FieldNumber, pref.Value) bool) {
if m.set {
f(fieldS, pref.ValueOf(m.value))
}
}
func (m *known) NewMessage(num pref.FieldNumber) pref.Message {
panic("not a message field")
}
func (m *known) ExtensionTypes() pref.ExtensionFieldTypes {
return (*exttypes)(m)
}
type unknown IrregularMessage
func (m *unknown) Len() int { return 0 }
func (m *unknown) Get(pref.FieldNumber) pref.RawFields { return nil }
func (m *unknown) Set(pref.FieldNumber, pref.RawFields) {}
func (m *unknown) Range(func(pref.FieldNumber, pref.RawFields) bool) {}
func (m *unknown) IsSupported() bool { return false }
type exttypes IrregularMessage
func (m *exttypes) Len() int { return 0 }
func (m *exttypes) Register(pref.ExtensionType) { panic("not extendable") }
func (m *exttypes) Remove(pref.ExtensionType) {}
func (m *exttypes) ByNumber(pref.FieldNumber) pref.ExtensionType { return nil }
func (m *exttypes) ByName(pref.FullName) pref.ExtensionType { return nil }
func (m *exttypes) Range(func(pref.ExtensionType) bool) {}
const fieldS = pref.FieldNumber(1)
var descriptor = func() pref.FileDescriptor {
p := &descriptorpb.FileDescriptorProto{}
if err := prototext.Unmarshal([]byte(descriptorText), p); err != nil {
panic(err)
}
file, err := protodesc.NewFile(p, nil)
if err != nil {
panic(err)
}
return file
}()
func file_irregular_irregular_proto_init() { _ = descriptor }
const descriptorText = `
name: "internal/testprotos/irregular/irregular.proto"
package: "goproto.proto.thirdparty"
message_type {
name: "IrregularMessage"
field {
name: "s"
number: 1
label: LABEL_OPTIONAL
type: TYPE_STRING
json_name: "s"
}
}
options {
go_package: "google.golang.org/protobuf/internal/testprotos/irregular"
}
`

View File

@ -0,0 +1,16 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
syntax = "proto2";
package goproto.proto.irregular;
option go_package = "google.golang.org/protobuf/internal/testprotos/irregular";
// IrregularMessage is a message with an implementation that does not match the
// usual structure of a generated message.
message IrregularMessage {
optional string s = 1;
}

View File

@ -0,0 +1,208 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// source: irregular/test.proto
package irregular
import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoregistry "google.golang.org/protobuf/reflect/protoregistry"
protoiface "google.golang.org/protobuf/runtime/protoiface"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
sync "sync"
)
const (
// Verify that runtime/protoimpl is sufficiently up-to-date.
_ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 0)
// Verify that this generated code is sufficiently up-to-date.
_ = protoimpl.EnforceVersion(0 - protoimpl.MinVersion)
)
type Message struct {
OptionalMessage *IrregularMessage `protobuf:"bytes,1,opt,name=optional_message,json=optionalMessage" json:"optional_message,omitempty"`
RepeatedMessage []*IrregularMessage `protobuf:"bytes,2,rep,name=repeated_message,json=repeatedMessage" json:"repeated_message,omitempty"`
RequiredMessage *IrregularMessage `protobuf:"bytes,3,req,name=required_message,json=requiredMessage" json:"required_message,omitempty"`
MapMessage map[string]*IrregularMessage `protobuf:"bytes,4,rep,name=map_message,json=mapMessage" json:"map_message,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"`
// Types that are valid to be assigned to Union:
// *Message_OneofMessage
Union isMessage_Union `protobuf_oneof:"union"`
XXX_NoUnkeyedLiteral struct{} `json:"-"`
XXX_unrecognized protoimpl.UnknownFields `json:"-"`
XXX_sizecache protoimpl.SizeCache `json:"-"`
}
func (x *Message) Reset() {
*x = Message{}
}
func (x *Message) String() string {
return protoimpl.X.MessageStringOf(x)
}
func (*Message) ProtoMessage() {}
func (x *Message) ProtoReflect() protoreflect.Message {
return file_irregular_test_proto_msgTypes[0].MessageOf(x)
}
func (m *Message) XXX_Methods() *protoiface.Methods {
return file_irregular_test_proto_msgTypes[0].Methods()
}
// Deprecated: Use Message.ProtoReflect.Type instead.
func (*Message) Descriptor() ([]byte, []int) {
return file_irregular_test_proto_rawDescGZIP(), []int{0}
}
func (x *Message) GetOptionalMessage() *IrregularMessage {
if x != nil {
return x.OptionalMessage
}
return nil
}
func (x *Message) GetRepeatedMessage() []*IrregularMessage {
if x != nil {
return x.RepeatedMessage
}
return nil
}
func (x *Message) GetRequiredMessage() *IrregularMessage {
if x != nil {
return x.RequiredMessage
}
return nil
}
func (x *Message) GetMapMessage() map[string]*IrregularMessage {
if x != nil {
return x.MapMessage
}
return nil
}
func (m *Message) GetUnion() isMessage_Union {
if m != nil {
return m.Union
}
return nil
}
func (x *Message) GetOneofMessage() *IrregularMessage {
if x, ok := x.GetUnion().(*Message_OneofMessage); ok {
return x.OneofMessage
}
return nil
}
// XXX_OneofWrappers is for the internal use of the proto package.
func (*Message) XXX_OneofWrappers() []interface{} {
return []interface{}{
(*Message_OneofMessage)(nil),
}
}
type isMessage_Union interface {
isMessage_Union()
}
type Message_OneofMessage struct {
OneofMessage *IrregularMessage `protobuf:"bytes,5,opt,name=oneof_message,json=oneofMessage,oneof"`
}
func (*Message_OneofMessage) isMessage_Union() {}
var File_irregular_test_proto protoreflect.FileDescriptor
var file_irregular_test_proto_rawDesc = []byte{
0x0a, 0x14, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x2f, 0x74, 0x65, 0x73, 0x74,
0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x17, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e,
0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x1a,
0x19, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x2f, 0x69, 0x72, 0x72, 0x65, 0x67,
0x75, 0x6c, 0x61, 0x72, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xa3, 0x04, 0x0a, 0x07, 0x4d,
0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x54, 0x0a, 0x10, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e,
0x61, 0x6c, 0x5f, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b,
0x32, 0x29, 0x2e, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f,
0x2e, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x2e, 0x49, 0x72, 0x72, 0x65, 0x67,
0x75, 0x6c, 0x61, 0x72, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x52, 0x0f, 0x6f, 0x70, 0x74,
0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x54, 0x0a, 0x10,
0x72, 0x65, 0x70, 0x65, 0x61, 0x74, 0x65, 0x64, 0x5f, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65,
0x18, 0x02, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x29, 0x2e, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f,
0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72,
0x2e, 0x49, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67,
0x65, 0x52, 0x0f, 0x72, 0x65, 0x70, 0x65, 0x61, 0x74, 0x65, 0x64, 0x4d, 0x65, 0x73, 0x73, 0x61,
0x67, 0x65, 0x12, 0x54, 0x0a, 0x10, 0x72, 0x65, 0x71, 0x75, 0x69, 0x72, 0x65, 0x64, 0x5f, 0x6d,
0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x03, 0x20, 0x02, 0x28, 0x0b, 0x32, 0x29, 0x2e, 0x67,
0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x69, 0x72, 0x72,
0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x2e, 0x49, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72,
0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x52, 0x0f, 0x72, 0x65, 0x71, 0x75, 0x69, 0x72, 0x65,
0x64, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x51, 0x0a, 0x0b, 0x6d, 0x61, 0x70, 0x5f,
0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x04, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x30, 0x2e,
0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x69, 0x72,
0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x2e, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x2e,
0x4d, 0x61, 0x70, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x52,
0x0a, 0x6d, 0x61, 0x70, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x50, 0x0a, 0x0d, 0x6f,
0x6e, 0x65, 0x6f, 0x66, 0x5f, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x05, 0x20, 0x01,
0x28, 0x0b, 0x32, 0x29, 0x2e, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f,
0x74, 0x6f, 0x2e, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x2e, 0x49, 0x72, 0x72,
0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x48, 0x00, 0x52,
0x0c, 0x6f, 0x6e, 0x65, 0x6f, 0x66, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x1a, 0x68, 0x0a,
0x0f, 0x4d, 0x61, 0x70, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x45, 0x6e, 0x74, 0x72, 0x79,
0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b,
0x65, 0x79, 0x12, 0x3f, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28,
0x0b, 0x32, 0x29, 0x2e, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74,
0x6f, 0x2e, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72, 0x2e, 0x49, 0x72, 0x72, 0x65,
0x67, 0x75, 0x6c, 0x61, 0x72, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x52, 0x05, 0x76, 0x61,
0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x42, 0x07, 0x0a, 0x05, 0x75, 0x6e, 0x69, 0x6f, 0x6e,
0x42, 0x3a, 0x5a, 0x38, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x67, 0x6f, 0x6c, 0x61, 0x6e,
0x67, 0x2e, 0x6f, 0x72, 0x67, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x69,
0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x70, 0x72, 0x6f, 0x74,
0x6f, 0x73, 0x2f, 0x69, 0x72, 0x72, 0x65, 0x67, 0x75, 0x6c, 0x61, 0x72,
}
var (
file_irregular_test_proto_rawDescOnce sync.Once
file_irregular_test_proto_rawDescData = file_irregular_test_proto_rawDesc
)
func file_irregular_test_proto_rawDescGZIP() []byte {
file_irregular_test_proto_rawDescOnce.Do(func() {
file_irregular_test_proto_rawDescData = protoimpl.X.CompressGZIP(file_irregular_test_proto_rawDescData)
})
return file_irregular_test_proto_rawDescData
}
var file_irregular_test_proto_msgTypes = make([]protoimpl.MessageInfo, 2)
var file_irregular_test_proto_goTypes = []interface{}{
(*Message)(nil), // 0: goproto.proto.irregular.Message
nil, // 1: goproto.proto.irregular.Message.MapMessageEntry
(*IrregularMessage)(nil), // 2: goproto.proto.irregular.IrregularMessage
}
var file_irregular_test_proto_depIdxs = []int32{
2, // goproto.proto.irregular.Message.optional_message:type_name -> goproto.proto.irregular.IrregularMessage
2, // goproto.proto.irregular.Message.repeated_message:type_name -> goproto.proto.irregular.IrregularMessage
2, // goproto.proto.irregular.Message.required_message:type_name -> goproto.proto.irregular.IrregularMessage
1, // goproto.proto.irregular.Message.map_message:type_name -> goproto.proto.irregular.Message.MapMessageEntry
2, // goproto.proto.irregular.Message.oneof_message:type_name -> goproto.proto.irregular.IrregularMessage
2, // goproto.proto.irregular.Message.MapMessageEntry.value:type_name -> goproto.proto.irregular.IrregularMessage
}
func init() { file_irregular_test_proto_init() }
func file_irregular_test_proto_init() {
if File_irregular_test_proto != nil {
return
}
file_irregular_irregular_proto_init()
File_irregular_test_proto = protoimpl.FileBuilder{
RawDescriptor: file_irregular_test_proto_rawDesc,
GoTypes: file_irregular_test_proto_goTypes,
DependencyIndexes: file_irregular_test_proto_depIdxs,
MessageOutputTypes: file_irregular_test_proto_msgTypes,
FilesRegistry: protoregistry.GlobalFiles,
TypesRegistry: protoregistry.GlobalTypes,
}.Init()
file_irregular_test_proto_rawDesc = nil
file_irregular_test_proto_goTypes = nil
file_irregular_test_proto_depIdxs = nil
}

View File

@ -0,0 +1,25 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// This file contains a message which references a message that implements the
// proto.Message interface but does not have the structure of a normal generated
// message.
syntax = "proto2";
package goproto.proto.irregular;
import "irregular/irregular.proto";
option go_package = "google.golang.org/protobuf/internal/testprotos/irregular";
message Message {
optional IrregularMessage optional_message = 1;
repeated IrregularMessage repeated_message = 2;
required IrregularMessage required_message = 3;
map<string,IrregularMessage> map_message = 4;
oneof union {
IrregularMessage oneof_message = 5;
}
}

View File

@ -8,6 +8,7 @@ import (
"fmt"
"testing"
irregularpb "google.golang.org/protobuf/internal/testprotos/irregular"
testpb "google.golang.org/protobuf/internal/testprotos/test"
test3pb "google.golang.org/protobuf/internal/testprotos/test3"
"google.golang.org/protobuf/proto"
@ -20,6 +21,7 @@ func Test(t *testing.T) {
(*test3pb.TestAllTypes)(nil),
(*testpb.TestRequired)(nil),
(*testpb.TestWeak)(nil),
(*irregularpb.Message)(nil),
} {
t.Run(fmt.Sprintf("%T", m), func(t *testing.T) {
prototest.TestMessage(t, m)