Skip to content

Commit

Permalink
Fix marshalling of empty oneOf messages
Browse files Browse the repository at this point in the history
Fixes #61
  • Loading branch information
howardjohn committed Jan 18, 2024
1 parent e7d7219 commit c69309d
Show file tree
Hide file tree
Showing 13 changed files with 211 additions and 9 deletions.
58 changes: 58 additions & 0 deletions conformance/internal/conformance/fuzz_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package conformance

import (
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/encoding/prototext"
"google.golang.org/protobuf/proto"
"strings"
"testing"
)

func roundTripUpstream(b []byte) ([]byte, error) {
msg := &TestAllTypesProto3{}
if err := proto.Unmarshal(b, msg); err != nil {
return nil, err
}
res, err := proto.Marshal(msg)
if err != nil {
return nil, err
}
return res, nil
}

func roundTripVtprotobuf(b []byte) ([]byte, error) {
msg := &TestAllTypesProto3{}
if err := msg.UnmarshalVT(b); err != nil {
return nil, err
}
res, err := msg.MarshalVT()
if err != nil {
return nil, err
}
return res, nil
}

func FuzzProto(f *testing.F) {
f.Fuzz(func(t *testing.T, b []byte) {
u, uerr := roundTripUpstream(b)
v, verr := roundTripVtprotobuf(b)
if verr != nil && strings.Contains(verr.Error(), "wrong wireType") {
t.Skip()
}
if uerr != nil && strings.Contains(uerr.Error(), "cannot parse invalid wire-format data") {
t.Skip()
}
if (uerr != nil) != (verr != nil) {
t.Fatalf("upstream err: %v (%v), vtprotobuf err: %v (%v)", uerr, u, verr, v)
}
vt := &TestAllTypesProto3{}
_ = vt.UnmarshalVT(b)
t.Logf("vtprotobuf: %v, %v", protojson.Format(vt), prototext.Format(vt))
us := &TestAllTypesProto3{}
_ = proto.Unmarshal(b, us)

t.Logf("upstream: %v, %v", protojson.Format(us), prototext.Format(us))
require.Equal(t, u, v)
})
}
16 changes: 16 additions & 0 deletions conformance/internal/conformance/oneof_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package conformance

import (
"testing"

"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
)

func TestEmptyOneoff(t *testing.T) {
// Regression test for https://github.com/planetscale/vtprotobuf/issues/61
msg := &TestAllTypesProto3{OneofField: &TestAllTypesProto3_OneofNestedMessage{}}
upstream, _ := proto.Marshal(msg)
vt, _ := msg.MarshalVTStrict()
require.Equal(t, upstream, vt)
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("\xe30$")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("8\xb30")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("\x80\xff\x000")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("X\xb30")
11 changes: 7 additions & 4 deletions features/marshal/marshalto.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"strings"

"github.com/planetscale/vtprotobuf/generator"

"google.golang.org/protobuf/compiler/protogen"
"google.golang.org/protobuf/encoding/protowire"
"google.golang.org/protobuf/reflect/protoreflect"
Expand Down Expand Up @@ -520,7 +519,12 @@ func (p *marshal) field(oneof bool, numGen *counter, field *protogen.Field) {
default:
panic("not implemented")
}
if repeated || nullable {
if oneof && field.Desc.Kind() == protoreflect.MessageKind && !field.Desc.IsMap() && !field.Desc.IsList() {
p.P("} else {")
p.P("i = protohelpers.EncodeVarint(dAtA, i, 0)")
p.encodeKey(fieldNumber, wireType)
p.P("}")
} else if repeated || nullable {
p.P(`}`)
}
}
Expand Down Expand Up @@ -676,7 +680,7 @@ func (p *marshal) message(message *protogen.Message) {
p.P(`}`)
p.P()

//Generate MarshalToVT methods for oneof fields
// Generate MarshalToVT methods for oneof fields
for _, field := range message.Fields {
if field.Oneof == nil || field.Oneof.Desc.IsSynthetic() {
continue
Expand Down Expand Up @@ -709,7 +713,6 @@ func (p *marshal) marshalBackwardSize(varInt bool) {
if varInt {
p.encodeVarint(`size`)
}

}

func (p *marshal) marshalBackward(varName string, varInt bool, message *protogen.Message) {
Expand Down
9 changes: 4 additions & 5 deletions features/size/size.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ package size
import (
"strconv"

"github.com/planetscale/vtprotobuf/generator"
"google.golang.org/protobuf/compiler/protogen"
"google.golang.org/protobuf/encoding/protowire"
"google.golang.org/protobuf/reflect/protoreflect"

"github.com/planetscale/vtprotobuf/generator"
)

func init() {
Expand Down Expand Up @@ -266,7 +265,9 @@ func (p *size) field(oneof bool, field *protogen.Field, sizeName string) {
default:
panic("not implemented")
}
if repeated || nullable {
if oneof && field.Desc.Kind() == protoreflect.MessageKind && !field.Desc.IsMap() && !field.Desc.IsList() {
p.P("} else { n += 3 }")
} else if repeated || nullable {
p.P(`}`)
}
}
Expand Down Expand Up @@ -310,8 +311,6 @@ func (p *size) message(message *protogen.Message) {
}
p.P(`}`)
} else {
//if _, ok := oneofs[fieldname]; !ok {
//oneofs[fieldname] = struct{}{}
p.P(`if vtmsg, ok := m.`, fieldname, `.(interface{ SizeVT() int }); ok {`)
p.P(`n+=vtmsg.`, sizeName, `()`)
p.P(`}`)
Expand Down
30 changes: 30 additions & 0 deletions testproto/pool/pool_with_oneof_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit c69309d

Please sign in to comment.