Merge pull request #622 from jmarais/master

Upstream commits from golang/protobuf

Author: jmarais

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.
@@ -163,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 {
@@ -210,19 +216,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")
@@ -543,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")
@@ -1414,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
 }

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,

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()
+	}()
+}

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
 	}
 

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
 			}