From 446ae07067139dafe346a0add1f5d4a43df129cd Mon Sep 17 00:00:00 2001 From: Herbie Ong Date: Fri, 28 Jun 2019 15:13:13 -0700 Subject: [PATCH] jsonpb: fix marshaling of Duration Fix output for negative nanoseconds with zero second. Add validation on min/max seconds. --- jsonpb/jsonpb.go | 16 ++++++++++++---- jsonpb/jsonpb_test.go | 39 +++++++++++++++++++++++---------------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/jsonpb/jsonpb.go b/jsonpb/jsonpb.go index 4e4ddc77be..e9cc202585 100644 --- a/jsonpb/jsonpb.go +++ b/jsonpb/jsonpb.go @@ -57,6 +57,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. @@ -211,19 +212,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 index 5e876bc599..fd06fc2fee 100644 --- a/jsonpb/jsonpb_test.go +++ b/jsonpb/jsonpb_test.go @@ -473,10 +473,17 @@ 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: &durpb.Duration{Seconds: 3}}, `{"dur":"3s"}`}, - {"Duration", marshaler, &pb.KnownTypes{Dur: &durpb.Duration{Seconds: 3, Nanos: 1e6}}, `{"dur":"3.001s"}`}, - {"Duration beyond float64 precision", marshaler, &pb.KnownTypes{Dur: &durpb.Duration{Seconds: 100000000, Nanos: 1}}, `{"dur":"100000000.000000001s"}`}, - {"negative Duration", marshaler, &pb.KnownTypes{Dur: &durpb.Duration{Seconds: -123, Nanos: -456}}, `{"dur":"-123.000000456s"}`}, + {"Duration empty", marshaler, &durpb.Duration{}, `"0s"`}, + {"Duration with secs", marshaler, &durpb.Duration{Seconds: 3}, `"3s"`}, + {"Duration with -secs", marshaler, &durpb.Duration{Seconds: -3}, `"-3s"`}, + {"Duration with nanos", marshaler, &durpb.Duration{Nanos: 1e6}, `"0.001s"`}, + {"Duration with -nanos", marshaler, &durpb.Duration{Nanos: -1e6}, `"-0.001s"`}, + {"Duration with large secs", marshaler, &durpb.Duration{Seconds: 1e10, Nanos: 1}, `"10000000000.000000001s"`}, + {"Duration with 6-digit nanos", marshaler, &durpb.Duration{Nanos: 1e4}, `"0.000010s"`}, + {"Duration with 3-digit nanos", marshaler, &durpb.Duration{Nanos: 1e6}, `"0.001s"`}, + {"Duration with -secs -nanos", marshaler, &durpb.Duration{Seconds: -123, Nanos: -450}, `"-123.000000450s"`}, + {"Duration max value", marshaler, &durpb.Duration{Seconds: 315576000000, Nanos: 999999999}, `"315576000000.999999999s"`}, + {"Duration min value", marshaler, &durpb.Duration{Seconds: -315576000000, Nanos: -999999999}, `"-315576000000.999999999s"`}, {"Struct", marshaler, &pb.KnownTypes{St: &stpb.Struct{ Fields: map[string]*stpb.Value{ "one": {Kind: &stpb.Value_StringValue{"loneliest number"}}, @@ -549,15 +556,17 @@ func TestMarshalIllegalTime(t *testing.T) { pb proto.Message fail bool }{ - {&pb.KnownTypes{Dur: &durpb.Duration{Seconds: 1, Nanos: 0}}, false}, - {&pb.KnownTypes{Dur: &durpb.Duration{Seconds: -1, Nanos: 0}}, false}, - {&pb.KnownTypes{Dur: &durpb.Duration{Seconds: 1, Nanos: -1}}, true}, - {&pb.KnownTypes{Dur: &durpb.Duration{Seconds: -1, Nanos: 1}}, true}, - {&pb.KnownTypes{Dur: &durpb.Duration{Seconds: 1, Nanos: 1000000000}}, true}, - {&pb.KnownTypes{Dur: &durpb.Duration{Seconds: -1, Nanos: -1000000000}}, true}, - {&pb.KnownTypes{Ts: &tspb.Timestamp{Seconds: 1, Nanos: 1}}, false}, - {&pb.KnownTypes{Ts: &tspb.Timestamp{Seconds: 1, Nanos: -1}}, true}, - {&pb.KnownTypes{Ts: &tspb.Timestamp{Seconds: 1, Nanos: 1000000000}}, true}, + {&durpb.Duration{Seconds: 1, Nanos: 0}, false}, + {&durpb.Duration{Seconds: -1, Nanos: 0}, false}, + {&durpb.Duration{Seconds: 1, Nanos: -1}, true}, + {&durpb.Duration{Seconds: -1, Nanos: 1}, true}, + {&durpb.Duration{Seconds: 315576000001}, true}, + {&durpb.Duration{Seconds: -315576000001}, true}, + {&durpb.Duration{Seconds: 1, Nanos: 1000000000}, true}, + {&durpb.Duration{Seconds: -1, Nanos: -1000000000}, true}, + {&tspb.Timestamp{Seconds: 1, Nanos: 1}, false}, + {&tspb.Timestamp{Seconds: 1, Nanos: -1}, true}, + {&tspb.Timestamp{Seconds: 1, Nanos: 1000000000}, true}, } for _, tt := range tests { _, err := marshaler.MarshalToString(tt.pb) @@ -607,8 +616,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, @@ -623,7 +631,6 @@ func TestMarshalAnyJSONPBMarshaler(t *testing.T) { } } - func TestMarshalWithCustomValidation(t *testing.T) { msg := dynamicMessage{RawJson: `{ "foo": "bar", "baz": [0, 1, 2, 3] }`, Dummy: &dynamicMessage{}}