mirror/protobuf-go
1
0
Fork 0
mirror of https://github.com/protocolbuffers/protobuf-go.git synced 2025-03-09 13:13:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

proto, internal/impl: make wire output more consistent with v1

Browse Source 
The v1 wire marshaler sorts fields as follows:
  - All extensions, sorted by field number.
  - All non-oneof fields, sorted by field number.
  - All oneof fields, in indeterminate order.

We already make some steps toward supporting this ordering: The
fast path encoder places extensions in sorted order at the start
of the message.

This commit moves oneof fields to the end of the message, makes the
reflection-based encoder use this ordering when deterministic marshaling
is enabled, and adds a test to catch unintentional changes to the
ordering.

Users SHOULD NOT depend on stability of the marshal output. It is
subject to change over time. Without deterministic marshaling enabled,
it is subject to change over calls to Marshal.

Change-Id: I6cfd89090d790a3bb50785f32b94d2781d7d08db
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/206800
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
This commit is contained in:
Damien Neil 2019-11-12 12:33:12 -08:00
parent 1605775be0
commit 01c0e8d680
6 changed files with 389 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
internal/fieldsort/fieldsort.go Normal file
View File

@ -0,0 +1,40 @@
// 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 fieldsort defines an ordering of fields.
//
// The ordering defined by this package matches the historic behavior of the proto
// package, placing extensions first and oneofs last.
//
// There is no guarantee about stability of the wire encoding, and users should not
// depend on the order defined in this package as it is subject to change without
// notice.
package fieldsort
import (
"google.golang.org/protobuf/reflect/protoreflect"
)
// Less returns true if field a comes before field j in ordered wire marshal output.
func Less(a, b protoreflect.FieldDescriptor) bool {
ea := a.IsExtension()
eb := b.IsExtension()
oa := a.ContainingOneof()
ob := b.ContainingOneof()
switch {
case ea != eb:
return ea
case oa != nil && ob != nil:
if oa == ob {
return a.Number() < b.Number()
}
return oa.Index() < ob.Index()
case oa != nil:
return false
case ob != nil:
return true
default:
return a.Number() < b.Number()
}
}

10
internal/impl/codec_message.go
View File

@ -12,6 +12,7 @@ import (
"google.golang.org/protobuf/internal/encoding/messageset"
"google.golang.org/protobuf/internal/encoding/wire"
"google.golang.org/protobuf/internal/fieldsort"
pref "google.golang.org/protobuf/reflect/protoreflect"
piface "google.golang.org/protobuf/runtime/protoiface"
)
@ -122,6 +123,15 @@ func (mi *MessageInfo) makeCoderMethods(t reflect.Type, si structInfo) {
mi.denseCoderFields[cf.num] = cf
}
// To preserve compatibility with historic wire output, marshal oneofs last.
if mi.Desc.Oneofs().Len() > 0 {
sort.Slice(mi.orderedCoderFields, func(i, j int) bool {
fi := fields.ByNumber(mi.orderedCoderFields[i].num)
fj := fields.ByNumber(mi.orderedCoderFields[j].num)
return fieldsort.Less(fi, fj)
})
}
mi.needsInitCheck = needsInitCheck(mi.Desc)
if mi.methods.MarshalAppend == nil && mi.methods.Size == nil {
mi.methods.Flags |= piface.SupportMarshalDeterministic

257
internal/testprotos/order/order.pb.go Normal file
View File

@ -0,0 +1,257 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Messages in this file are used to test wire encoding order.
// Code generated by protoc-gen-go. DO NOT EDIT.
// source: order/order.proto
package order
import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoiface "google.golang.org/protobuf/runtime/protoiface"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect"
sync "sync"
)
type Message struct {
state protoimpl.MessageState
sizeCache protoimpl.SizeCache
unknownFields protoimpl.UnknownFields
extensionFields protoimpl.ExtensionFields
Field_2 *string `protobuf:"bytes,2,opt,name=field_2,json=field2" json:"field_2,omitempty"`
Field_1 *string `protobuf:"bytes,1,opt,name=field_1,json=field1" json:"field_1,omitempty"`
// Types that are assignable to Oneof_1:
// *Message_Field_10
Oneof_1 isMessage_Oneof_1 `protobuf_oneof:"oneof_1"`
Field_20 *string `protobuf:"bytes,20,opt,name=field_20,json=field20" json:"field_20,omitempty"`
}
func (x *Message) Reset() {
*x = Message{}
if protoimpl.UnsafeEnabled {
mi := &file_order_order_proto_msgTypes[0]
ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
ms.StoreMessageInfo(mi)
}
}
func (x *Message) String() string {
return protoimpl.X.MessageStringOf(x)
}
func (*Message) ProtoMessage() {}
func (x *Message) ProtoReflect() protoreflect.Message {
mi := &file_order_order_proto_msgTypes[0]
if protoimpl.UnsafeEnabled && x != nil {
ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
if ms.LoadMessageInfo() == nil {
ms.StoreMessageInfo(mi)
}
return ms
}
return mi.MessageOf(x)
}
// Deprecated: Use Message.ProtoReflect.Descriptor instead.
func (*Message) Descriptor() ([]byte, []int) {
return file_order_order_proto_rawDescGZIP(), []int{0}
}
var extRange_Message = []protoiface.ExtensionRangeV1{
{Start: 30, End: 40},
}
// Deprecated: Use Message.ProtoReflect.Descriptor.ExtensionRanges instead.
func (*Message) ExtensionRangeArray() []protoiface.ExtensionRangeV1 {
return extRange_Message
}
func (x *Message) GetField_2() string {
if x != nil && x.Field_2 != nil {
return *x.Field_2
}
return ""
}
func (x *Message) GetField_1() string {
if x != nil && x.Field_1 != nil {
return *x.Field_1
}
return ""
}
func (m *Message) GetOneof_1() isMessage_Oneof_1 {
if m != nil {
return m.Oneof_1
}
return nil
}
func (x *Message) GetField_10() string {
if x, ok := x.GetOneof_1().(*Message_Field_10); ok {
return x.Field_10
}
return ""
}
func (x *Message) GetField_20() string {
if x != nil && x.Field_20 != nil {
return *x.Field_20
}
return ""
}
type isMessage_Oneof_1 interface {
isMessage_Oneof_1()
}
type Message_Field_10 struct {
Field_10 string `protobuf:"bytes,10,opt,name=field_10,json=field10,oneof"`
}
func (*Message_Field_10) isMessage_Oneof_1() {}
var file_order_order_proto_extTypes = []protoimpl.ExtensionInfo{
{
ExtendedType: (*Message)(nil),
ExtensionType: (*string)(nil),
Field: 30,
Name: "goproto.proto.order.field_30",
Tag: "bytes,30,opt,name=field_30",
Filename: "order/order.proto",
},
{
ExtendedType: (*Message)(nil),
ExtensionType: (*string)(nil),
Field: 31,
Name: "goproto.proto.order.field_31",
Tag: "bytes,31,opt,name=field_31",
Filename: "order/order.proto",
},
{
ExtendedType: (*Message)(nil),
ExtensionType: (*string)(nil),
Field: 32,
Name: "goproto.proto.order.field_32",
Tag: "bytes,32,opt,name=field_32",
Filename: "order/order.proto",
},
}
// Extension fields to Message.
var (
// optional string field_30 = 30;
E_Field_30 = &file_order_order_proto_extTypes[0]
// optional string field_31 = 31;
E_Field_31 = &file_order_order_proto_extTypes[1]
// optional string field_32 = 32;
E_Field_32 = &file_order_order_proto_extTypes[2]
)
var File_order_order_proto protoreflect.FileDescriptor
var file_order_order_proto_rawDesc = []byte{
0x0a, 0x11, 0x6f, 0x72, 0x64, 0x65, 0x72, 0x2f, 0x6f, 0x72, 0x64, 0x65, 0x72, 0x2e, 0x70, 0x72,
0x6f, 0x74, 0x6f, 0x12, 0x13, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f,
0x74, 0x6f, 0x2e, 0x6f, 0x72, 0x64, 0x65, 0x72, 0x22, 0x84, 0x01, 0x0a, 0x07, 0x4d, 0x65, 0x73,
0x73, 0x61, 0x67, 0x65, 0x12, 0x17, 0x0a, 0x07, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f, 0x32, 0x18,
0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x32, 0x12, 0x17, 0x0a,
0x07, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f, 0x31, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06,
0x66, 0x69, 0x65, 0x6c, 0x64, 0x31, 0x12, 0x1b, 0x0a, 0x08, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f,
0x31, 0x30, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x09, 0x48, 0x00, 0x52, 0x07, 0x66, 0x69, 0x65, 0x6c,
0x64, 0x31, 0x30, 0x12, 0x19, 0x0a, 0x08, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f, 0x32, 0x30, 0x18,
0x14, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x32, 0x30, 0x2a, 0x04,
0x08, 0x1e, 0x10, 0x29, 0x42, 0x09, 0x0a, 0x07, 0x6f, 0x6e, 0x65, 0x6f, 0x66, 0x5f, 0x31, 0x3a,
0x37, 0x0a, 0x08, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f, 0x33, 0x30, 0x12, 0x1c, 0x2e, 0x67, 0x6f,
0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x6f, 0x72, 0x64, 0x65,
0x72, 0x2e, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x1e, 0x20, 0x01, 0x28, 0x09, 0x52,
0x07, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x33, 0x30, 0x3a, 0x37, 0x0a, 0x08, 0x66, 0x69, 0x65, 0x6c,
0x64, 0x5f, 0x33, 0x31, 0x12, 0x1c, 0x2e, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70,
0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x6f, 0x72, 0x64, 0x65, 0x72, 0x2e, 0x4d, 0x65, 0x73, 0x73, 0x61,
0x67, 0x65, 0x18, 0x1f, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x33,
0x31, 0x3a, 0x37, 0x0a, 0x08, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f, 0x33, 0x32, 0x12, 0x1c, 0x2e,
0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x6f, 0x72,
0x64, 0x65, 0x72, 0x2e, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x20, 0x20, 0x01, 0x28,
0x09, 0x52, 0x07, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x33, 0x32, 0x42, 0x36, 0x5a, 0x34, 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, 0x6f, 0x72, 0x64,
0x65, 0x72,
}
var (
file_order_order_proto_rawDescOnce sync.Once
file_order_order_proto_rawDescData = file_order_order_proto_rawDesc
)
func file_order_order_proto_rawDescGZIP() []byte {
file_order_order_proto_rawDescOnce.Do(func() {
file_order_order_proto_rawDescData = protoimpl.X.CompressGZIP(file_order_order_proto_rawDescData)
})
return file_order_order_proto_rawDescData
}
var file_order_order_proto_msgTypes = make([]protoimpl.MessageInfo, 1)
var file_order_order_proto_goTypes = []interface{}{
(*Message)(nil), // 0: goproto.proto.order.Message
}
var file_order_order_proto_depIdxs = []int32{
0, // 0: goproto.proto.order.field_30:extendee -> goproto.proto.order.Message
0, // 1: goproto.proto.order.field_31:extendee -> goproto.proto.order.Message
0, // 2: goproto.proto.order.field_32:extendee -> goproto.proto.order.Message
3, // [3:3] is the sub-list for method output_type
3, // [3:3] is the sub-list for method input_type
3, // [3:3] is the sub-list for extension type_name
0, // [0:3] is the sub-list for extension extendee
0, // [0:0] is the sub-list for field type_name
}
func init() { file_order_order_proto_init() }
func file_order_order_proto_init() {
if File_order_order_proto != nil {
return
}
if !protoimpl.UnsafeEnabled {
file_order_order_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} {
switch v := v.(*Message); i {
case 0:
return &v.state
case 1:
return &v.sizeCache
case 2:
return &v.unknownFields
case 3:
return &v.extensionFields
default:
return nil
}
}
}
file_order_order_proto_msgTypes[0].OneofWrappers = []interface{}{
(*Message_Field_10)(nil),
}
type x struct{}
out := protoimpl.TypeBuilder{
File: protoimpl.DescBuilder{
GoPackagePath: reflect.TypeOf(x{}).PkgPath(),
RawDescriptor: file_order_order_proto_rawDesc,
NumEnums: 0,
NumMessages: 1,
NumExtensions: 3,
NumServices: 0,
},
GoTypes: file_order_order_proto_goTypes,
DependencyIndexes: file_order_order_proto_depIdxs,
MessageInfos: file_order_order_proto_msgTypes,
ExtensionInfos: file_order_order_proto_extTypes,
}.Build()
File_order_order_proto = out.File
file_order_order_proto_rawDesc = nil
file_order_order_proto_goTypes = nil
file_order_order_proto_depIdxs = nil
}

29
internal/testprotos/order/order.proto Normal file
View File

@ -0,0 +1,29 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Messages in this file are used to test wire encoding order.
syntax = "proto2";
package goproto.proto.order;
option go_package = "google.golang.org/protobuf/internal/testprotos/order";
message Message {
optional string field_2 = 2;
optional string field_1 = 1;
oneof oneof_1 {
string field_10 = 10;
}
extensions 30 to 40;
optional string field_20 = 20;
}
extend Message {
optional string field_30 = 30;
optional string field_31 = 31;
optional string field_32 = 32;
}

8
proto/encode.go
View File

@ -9,6 +9,7 @@ import (
"google.golang.org/protobuf/internal/encoding/messageset"
"google.golang.org/protobuf/internal/encoding/wire"
"google.golang.org/protobuf/internal/fieldsort"
"google.golang.org/protobuf/internal/mapsort"
"google.golang.org/protobuf/internal/pragma"
"google.golang.org/protobuf/reflect/protoreflect"
@ -146,7 +147,7 @@ func (o MarshalOptions) marshalMessageSlow(b []byte, m protoreflect.Message) ([]
// It is not deterministic, since Message.Range does not return fields in any
// defined order.
//
// When using deterministic serialization, we sort the known fields by field number.
// When using deterministic serialization, we sort the known fields.
var err error
o.rangeFields(m, func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
b, err = o.marshalField(b, fd, v)
@ -159,8 +160,7 @@ func (o MarshalOptions) marshalMessageSlow(b []byte, m protoreflect.Message) ([]
return b, nil
}
// rangeFields visits fields in field number order when deterministic
// serialization is enabled.
// rangeFields visits fields in a defined order when deterministic serialization is enabled.
func (o MarshalOptions) rangeFields(m protoreflect.Message, f func(protoreflect.FieldDescriptor, protoreflect.Value) bool) {
if !o.Deterministic {
m.Range(f)
@ -172,7 +172,7 @@ func (o MarshalOptions) rangeFields(m protoreflect.Message, f func(protoreflect.
return true
})
sort.Slice(fds, func(a, b int) bool {
return fds[a].Number() < fds[b].Number()
return fieldsort.Less(fds[a], fds[b])
})
for _, fd := range fds {
if !f(fd, m.Get(fd)) {

49
proto/encode_test.go
View File

@ -7,12 +7,16 @@ package proto_test
import (
"bytes"
"fmt"
"reflect"
"testing"
"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/internal/encoding/wire"
"google.golang.org/protobuf/internal/flags"
"google.golang.org/protobuf/proto"
pref "google.golang.org/protobuf/reflect/protoreflect"
orderpb "google.golang.org/protobuf/internal/testprotos/order"
testpb "google.golang.org/protobuf/internal/testprotos/test"
test3pb "google.golang.org/protobuf/internal/testprotos/test3"
)
@ -182,3 +186,48 @@ func TestMarshalAppendAllocations(t *testing.T) {
t.Errorf("expect amortized allocs/op to be identical")
}
}
func TestEncodeOrder(t *testing.T) {
// We make no guarantees about the stability of wire marshal output.
// The order in which fields are marshaled may change over time.
// If deterministic marshaling is not enabled, it may change over
// successive calls to proto.Marshal in the same binary.
//
// Unfortunately, many users have come to rely on the specific current
// wire marshal output. Perhaps someday we will choose to deliberately
// change the marshal output; until that day comes, this test verifies
// that we don't unintentionally change it.
m := &orderpb.Message{
Field_1: proto.String("one"),
Field_2: proto.String("two"),
Field_20: proto.String("twenty"),
Oneof_1: &orderpb.Message_Field_10{"ten"},
}
proto.SetExtension(m, orderpb.E_Field_30, "thirty")
proto.SetExtension(m, orderpb.E_Field_31, "thirty-one")
proto.SetExtension(m, orderpb.E_Field_32, "thirty-two")
want := []pref.FieldNumber{
30, 31, 32, // extensions first, in number order
1, 2, 20, // non-extension, non-oneof in number order
10, // oneofs last, undefined order
}
// Test with deterministic serialization, since fields are not sorted without
// it when -tags=protoreflect.
b, err := proto.MarshalOptions{Deterministic: true}.Marshal(m)
if err != nil {
t.Fatal(err)
}
var got []pref.FieldNumber
for len(b) > 0 {
num, _, n := wire.ConsumeField(b)
if n < 0 {
t.Fatal(wire.ParseError(n))
}
b = b[n:]
got = append(got, num)
}
if !reflect.DeepEqual(got, want) {
t.Errorf("unexpected field marshal order:\ngot: %v\nwant: %v\nmessage:\n%v", got, want, m)
}
}