From d1969a75bbab28eebbb658f7e53586ea09b32c2c Mon Sep 17 00:00:00 2001 From: Jacques Marais Date: Sun, 8 Sep 2019 19:11:35 +0000 Subject: [PATCH 1/3] merged in golang/protobuf commit b285ee9cfc6c881bb20c0d8dc73370ea9b9ec90f - Log parsing errors using log pkg --- proto/properties.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/proto/properties.go b/proto/properties.go index 62c55624a8..28da1475fb 100644 --- a/proto/properties.go +++ b/proto/properties.go @@ -43,7 +43,6 @@ package proto import ( "fmt" "log" - "os" "reflect" "sort" "strconv" @@ -205,7 +204,7 @@ func (p *Properties) Parse(s string) { // "bytes,49,opt,name=foo,def=hello!" fields := strings.Split(s, ",") // breaks def=, but handled below. if len(fields) < 2 { - fmt.Fprintf(os.Stderr, "proto: tag has too few fields: %q\n", s) + log.Printf("proto: tag has too few fields: %q", s) return } @@ -225,7 +224,7 @@ func (p *Properties) Parse(s string) { p.WireType = WireBytes // no numeric converter for non-numeric types default: - fmt.Fprintf(os.Stderr, "proto: tag has unknown wire type: %q\n", s) + log.Printf("proto: tag has unknown wire type: %q", s) return } From f99a381921dabdec18227ac91d868ffc3f80fd49 Mon Sep 17 00:00:00 2001 From: Jacques Marais Date: Sun, 8 Sep 2019 19:35:26 +0000 Subject: [PATCH 2/3] merged in golang/protubf commit 6c65a5562fc06764971b7c5d05c76c75e84bdbf7 - jsonpb: fix marshaling of Duration --- jsonpb/jsonpb.go | 16 ++++++++++++---- jsonpb/jsonpb_test.go | 41 +++++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 20 deletions(-) mode change 100755 => 100644 jsonpb/jsonpb.go mode change 100755 => 100644 jsonpb/jsonpb_test.go diff --git a/jsonpb/jsonpb.go b/jsonpb/jsonpb.go old mode 100755 new mode 100644 index 3893c02d46..1f64aabb87 --- a/jsonpb/jsonpb.go +++ b/jsonpb/jsonpb.go @@ -56,6 +56,7 @@ import ( ) const secondInNanos = int64(time.Second / time.Nanosecond) +const maxSecondsInDuration = 315576000000 // Marshaler is a configurable object for converting between // protocol buffer objects and a JSON representation for them. @@ -210,19 +211,26 @@ func (m *Marshaler) marshalObject(out *errWriter, v proto.Message, indent, typeU // Any is a bit more involved. return m.marshalAny(out, v, indent) case "Duration": - // "Generated output always contains 0, 3, 6, or 9 fractional digits, - // depending on required precision." s, ns := s.Field(0).Int(), s.Field(1).Int() + if s < -maxSecondsInDuration || s > maxSecondsInDuration { + return fmt.Errorf("seconds out of range %v", s) + } if ns <= -secondInNanos || ns >= secondInNanos { return fmt.Errorf("ns out of range (%v, %v)", -secondInNanos, secondInNanos) } if (s > 0 && ns < 0) || (s < 0 && ns > 0) { return errors.New("signs of seconds and nanos do not match") } - if s < 0 { + // Generated output always contains 0, 3, 6, or 9 fractional digits, + // depending on required precision, followed by the suffix "s". + f := "%d.%09d" + if ns < 0 { ns = -ns + if s == 0 { + f = "-%d.%09d" + } } - x := fmt.Sprintf("%d.%09d", s, ns) + x := fmt.Sprintf(f, s, ns) x = strings.TrimSuffix(x, "000") x = strings.TrimSuffix(x, "000") x = strings.TrimSuffix(x, ".000") diff --git a/jsonpb/jsonpb_test.go b/jsonpb/jsonpb_test.go old mode 100755 new mode 100644 index 9de2b3f156..64eb275bdb --- a/jsonpb/jsonpb_test.go +++ b/jsonpb/jsonpb_test.go @@ -363,7 +363,7 @@ var ( An: &types.Any{ TypeUrl: "type.googleapis.com/google.protobuf.Duration", Value: []byte{ - // &durpb.Duration{Seconds: 1, Nanos: 212000000 } + // &types.Duration{Seconds: 1, Nanos: 212000000 } 1 << 3, 1, // seconds 2 << 3, 0x80, 0xba, 0x8b, 0x65, // nanos }, @@ -472,10 +472,18 @@ var marshalingTests = []struct { {"Any with message and indent", marshalerAllOptions, anySimple, anySimplePrettyJSON}, {"Any with WKT", marshaler, anyWellKnown, anyWellKnownJSON}, {"Any with WKT and indent", marshalerAllOptions, anyWellKnown, anyWellKnownPrettyJSON}, - {"Duration", marshaler, &pb.KnownTypes{Dur: &types.Duration{Seconds: 3}}, `{"dur":"3s"}`}, - {"Duration", marshaler, &pb.KnownTypes{Dur: &types.Duration{Seconds: 3, Nanos: 1e6}}, `{"dur":"3.001s"}`}, - {"Duration beyond float64 precision", marshaler, &pb.KnownTypes{Dur: &types.Duration{Seconds: 100000000, Nanos: 1}}, `{"dur":"100000000.000000001s"}`}, - {"negative Duration", marshaler, &pb.KnownTypes{Dur: &types.Duration{Seconds: -123, Nanos: -456}}, `{"dur":"-123.000000456s"}`}, + {"Duration empty", marshaler, &types.Duration{}, `"0s"`}, + {"Duration with secs", marshaler, &types.Duration{Seconds: 3}, `"3s"`}, + {"Duration with -secs", marshaler, &types.Duration{Seconds: -3}, `"-3s"`}, + {"Duration with nanos", marshaler, &types.Duration{Nanos: 1e6}, `"0.001s"`}, + {"Duration with -nanos", marshaler, &types.Duration{Nanos: -1e6}, `"-0.001s"`}, + {"Duration with large secs", marshaler, &types.Duration{Seconds: 1e10, Nanos: 1}, `"10000000000.000000001s"`}, + {"Duration with 6-digit nanos", marshaler, &types.Duration{Nanos: 1e4}, `"0.000010s"`}, + {"Duration with 3-digit nanos", marshaler, &types.Duration{Nanos: 1e6}, `"0.001s"`}, + {"Duration with -secs -nanos", marshaler, &types.Duration{Seconds: -123, Nanos: -450}, `"-123.000000450s"`}, + {"Duration max value", marshaler, &types.Duration{Seconds: 315576000000, Nanos: 999999999}, `"315576000000.999999999s"`}, + {"Duration small negative", marshaler, &types.Duration{Nanos: -1}, `"-0.000000001s"`}, + {"Duration min value", marshaler, &types.Duration{Seconds: -315576000000, Nanos: -999999999}, `"-315576000000.999999999s"`}, {"Struct", marshaler, &pb.KnownTypes{St: &types.Struct{ Fields: map[string]*types.Value{ "one": {Kind: &types.Value_StringValue{StringValue: "loneliest number"}}, @@ -546,15 +554,17 @@ func TestMarshalIllegalTime(t *testing.T) { pb proto.Message fail bool }{ - {&pb.KnownTypes{Dur: &types.Duration{Seconds: 1, Nanos: 0}}, false}, - {&pb.KnownTypes{Dur: &types.Duration{Seconds: -1, Nanos: 0}}, false}, - {&pb.KnownTypes{Dur: &types.Duration{Seconds: 1, Nanos: -1}}, true}, - {&pb.KnownTypes{Dur: &types.Duration{Seconds: -1, Nanos: 1}}, true}, - {&pb.KnownTypes{Dur: &types.Duration{Seconds: 1, Nanos: 1000000000}}, true}, - {&pb.KnownTypes{Dur: &types.Duration{Seconds: -1, Nanos: -1000000000}}, true}, - {&pb.KnownTypes{Ts: &types.Timestamp{Seconds: 1, Nanos: 1}}, false}, - {&pb.KnownTypes{Ts: &types.Timestamp{Seconds: 1, Nanos: -1}}, true}, - {&pb.KnownTypes{Ts: &types.Timestamp{Seconds: 1, Nanos: 1000000000}}, true}, + {&types.Duration{Seconds: 1, Nanos: 0}, false}, + {&types.Duration{Seconds: -1, Nanos: 0}, false}, + {&types.Duration{Seconds: 1, Nanos: -1}, true}, + {&types.Duration{Seconds: -1, Nanos: 1}, true}, + {&types.Duration{Seconds: 315576000001}, true}, + {&types.Duration{Seconds: -315576000001}, true}, + {&types.Duration{Seconds: 1, Nanos: 1000000000}, true}, + {&types.Duration{Seconds: -1, Nanos: -1000000000}, true}, + {&types.Timestamp{Seconds: 1, Nanos: 1}, false}, + {&types.Timestamp{Seconds: 1, Nanos: -1}, true}, + {&types.Timestamp{Seconds: 1, Nanos: 1000000000}, true}, } for _, tt := range tests { _, err := marshaler.MarshalToString(tt.pb) @@ -604,8 +614,7 @@ func TestMarshalAnyJSONPBMarshaler(t *testing.T) { t.Errorf("an unexpected error occurred when marshalling Any to JSON: %v", err) } // same as expected above, but pretty-printed w/ indentation - expected = - `{ + expected = `{ "@type": "type.googleapis.com/` + dynamicMessageName + `", "baz": [ 0, From 33d47608f2cc12f4c1e590655e6175596f05e6bf Mon Sep 17 00:00:00 2001 From: Jacques Marais Date: Sun, 8 Sep 2019 19:51:52 +0000 Subject: [PATCH 3/3] merged in golang/protobuf commit 4c88cc3f1a34ffade77b79abc53335d1e511f25b - all: fix reflect.Value.Interface races. --- jsonpb/jsonpb.go | 12 +++++++++--- proto/all_test.go | 32 ++++++++++++++++++++++++++++++++ proto/text.go | 6 ++++-- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/jsonpb/jsonpb.go b/jsonpb/jsonpb.go index 1f64aabb87..e8134ec8ba 100644 --- a/jsonpb/jsonpb.go +++ b/jsonpb/jsonpb.go @@ -164,6 +164,11 @@ type isWkt interface { XXX_WellKnownType() string } +var ( + wktType = reflect.TypeOf((*isWkt)(nil)).Elem() + messageType = reflect.TypeOf((*proto.Message)(nil)).Elem() +) + // marshalObject writes a struct to the Writer. func (m *Marshaler) marshalObject(out *errWriter, v proto.Message, indent, typeURL string) error { if jsm, ok := v.(JSONPBMarshaler); ok { @@ -551,7 +556,8 @@ func (m *Marshaler) marshalValue(out *errWriter, prop *proto.Properties, v refle // Handle well-known types. // Most are handled up in marshalObject (because 99% are messages). - if wkt, ok := v.Interface().(isWkt); ok { + if v.Type().Implements(wktType) { + wkt := v.Interface().(isWkt) switch wkt.XXX_WellKnownType() { case "NullValue": out.write("null") @@ -1422,8 +1428,8 @@ func checkRequiredFields(pb proto.Message) error { } func checkRequiredFieldsInValue(v reflect.Value) error { - if pm, ok := v.Interface().(proto.Message); ok { - return checkRequiredFields(pm) + if v.Type().Implements(messageType) { + return checkRequiredFields(v.Interface().(proto.Message)) } return nil } diff --git a/proto/all_test.go b/proto/all_test.go index f391af74c3..966ceb567d 100644 --- a/proto/all_test.go +++ b/proto/all_test.go @@ -45,9 +45,11 @@ import ( "testing" "time" + "github.com/gogo/protobuf/jsonpb" . "github.com/gogo/protobuf/proto" pb3 "github.com/gogo/protobuf/proto/proto3_proto" . "github.com/gogo/protobuf/proto/test_proto" + descriptorpb "github.com/gogo/protobuf/protoc-gen-gogo/descriptor" ) var globalO *Buffer @@ -2509,3 +2511,33 @@ func BenchmarkUnmarshalUnrecognizedFields(b *testing.B) { p2.Unmarshal(pbd) } } + +// TestRace tests whether there are races among the different marshalers. +func TestRace(t *testing.T) { + m := &descriptorpb.FileDescriptorProto{ + Options: &descriptorpb.FileOptions{ + GoPackage: String("path/to/my/package"), + }, + } + + wg := &sync.WaitGroup{} + defer wg.Wait() + + wg.Add(1) + go func() { + defer wg.Done() + Marshal(m) + }() + + wg.Add(1) + go func() { + defer wg.Done() + (&jsonpb.Marshaler{}).MarshalToString(m) + }() + + wg.Add(1) + go func() { + defer wg.Done() + _ = m.String() + }() +} diff --git a/proto/text.go b/proto/text.go index 0407ba85d0..87416afe95 100644 --- a/proto/text.go +++ b/proto/text.go @@ -476,6 +476,8 @@ func (tm *TextMarshaler) writeStruct(w *textWriter, sv reflect.Value) error { return nil } +var textMarshalerType = reflect.TypeOf((*encoding.TextMarshaler)(nil)).Elem() + // writeAny writes an arbitrary field. func (tm *TextMarshaler) writeAny(w *textWriter, v reflect.Value, props *Properties) error { v = reflect.Indirect(v) @@ -589,8 +591,8 @@ func (tm *TextMarshaler) writeAny(w *textWriter, v reflect.Value, props *Propert // mutating this value. v = v.Addr() } - if etm, ok := v.Interface().(encoding.TextMarshaler); ok { - text, err := etm.MarshalText() + if v.Type().Implements(textMarshalerType) { + text, err := v.Interface().(encoding.TextMarshaler).MarshalText() if err != nil { return err }