Skip to content

Commit

Permalink
jsonpb: fix marshaling of Duration
Browse files Browse the repository at this point in the history 
Fix output for negative nanoseconds with zero second.
Add validation on min/max seconds.

Fixes #883.
  • Loading branch information
@cybrcodr
cybrcodr committed Jul 1, 2019
1 parent b285ee9 commit 6c65a55
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 20 deletions.
16 changes: 12 additions & 4 deletions jsonpb/jsonpb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down
39 changes: 23 additions & 16 deletions jsonpb/jsonpb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"}},
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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{}}

Expand Down

0 comments on commit 6c65a55

Please sign in to comment.