From 29e4b9c9ff5de5a6a88757a60937d8758950bb41 Mon Sep 17 00:00:00 2001 From: Tamir Sen Date: Mon, 2 Nov 2020 20:46:05 +0100 Subject: [PATCH 1/3] Add custom marshaller for claim predicate timestamp --- xdr/json.go | 53 +++++++++++++++++++++++++++++++++++++++++++++--- xdr/json_test.go | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/xdr/json.go b/xdr/json.go index 47acc3ed77..9c1c679fd6 100644 --- a/xdr/json.go +++ b/xdr/json.go @@ -3,15 +3,62 @@ package xdr import ( "encoding/json" "fmt" + "regexp" + "strconv" "time" + + "github.com/stellar/go/support/errors" ) +// ISO8601Time is a timestamp which supports parsing dates which have a year outside the 0000..9999 range +type ISO8601Time struct { + time.Time +} + +// reISO8601 is the regular expression used to parse date strings in the +// ISO 8601 extended format, with or without an expanded year representation. +var reISO8601 = regexp.MustCompile(`^([-+]?\d{4,})-(\d{2})-(\d{2})`) + +// MarshalJSON serializes the timestamp to a string +func (t ISO8601Time) MarshalJSON() ([]byte, error) { + return []byte(fmt.Sprintf(`"%s"`, t.Format(time.RFC3339))), nil +} + +// UnmarshalJSON parses a JSON string into a ISO8601Time instance. +func (t *ISO8601Time) UnmarshalJSON(b []byte) error { + if string(b) == "null" { + return nil + } + if b[0] != '"' || b[len(b)-1] != '"' { + return fmt.Errorf("%s does not begin and end with double quotes", b) + } + trimmed := string(b[1 : len(b)-1]) + m := reISO8601.FindStringSubmatch(trimmed) + + if len(m) != 4 { + return fmt.Errorf("UnmarshalJSON: cannot parse %s", trimmed) + } + // No need to check for errors since the regexp guarantees the matches + // are valid integers + year, _ := strconv.Atoi(m[1]) + month, _ := strconv.Atoi(m[2]) + day, _ := strconv.Atoi(m[3]) + + ts, err := time.Parse(time.RFC3339, "2006-01-02"+trimmed[len(m[0]):]) + if err != nil { + return errors.Wrap(err, "Could not extract time") + } + + t.Time = time.Date(year, time.Month(month), day, ts.Hour(), ts.Minute(), ts.Second(), ts.Nanosecond(), ts.Location()) + return nil +} + type claimPredicateJSON struct { And *[]claimPredicateJSON `json:"and,omitempty"` Or *[]claimPredicateJSON `json:"or,omitempty"` Not *claimPredicateJSON `json:"not,omitempty"` Unconditional bool `json:"unconditional,omitempty"` - AbsBefore *time.Time `json:"abs_before,omitempty"` + AbsBefore *ISO8601Time `json:"abs_before,omitempty"` RelBefore *int64 `json:"rel_before,string,omitempty"` } @@ -98,8 +145,8 @@ func (c ClaimPredicate) toJSON() (claimPredicateJSON, error) { payload.Not = new(claimPredicateJSON) *payload.Not, err = c.MustNotPredicate().toJSON() case ClaimPredicateTypeClaimPredicateBeforeAbsoluteTime: - payload.AbsBefore = new(time.Time) - *payload.AbsBefore = time.Unix(int64(c.MustAbsBefore()), 0).UTC() + payload.AbsBefore = new(ISO8601Time) + *payload.AbsBefore = ISO8601Time{time.Unix(int64(c.MustAbsBefore()), 0).UTC()} case ClaimPredicateTypeClaimPredicateBeforeRelativeTime: payload.RelBefore = new(int64) *payload.RelBefore = int64(c.MustRelBefore()) diff --git a/xdr/json_test.go b/xdr/json_test.go index 02c3fd8add..9a698c4606 100644 --- a/xdr/json_test.go +++ b/xdr/json_test.go @@ -2,6 +2,7 @@ package xdr import ( "encoding/json" + "math" "testing" "github.com/stretchr/testify/assert" @@ -57,3 +58,47 @@ func TestClaimPredicateJSON(t *testing.T) { assert.Equal(t, serializedBase64, parsedBase64) } + +func TestAbsBeforeTimestamps(t *testing.T) { + const year = 365 * 24 * 60 * 60 + for _, testCase := range []struct { + unix int64 + expected string + }{ + { + 0, + `{"abs_before":"1970-01-01T00:00:00Z"}`, + }, + { + 900 * year, + `{"abs_before":"2869-05-27T00:00:00Z"}`, + }, + { + math.MaxInt64, + `{"abs_before":"292277026596-12-04T15:30:07Z"}`, + }, + { + -10, + `{"abs_before":"1969-12-31T23:59:50Z"}`, + }, + { + math.MinInt64, + // this serialization doesn't make sense but at least it doesn't crash the marshaller + `{"abs_before":"292277026596-12-04T15:30:08Z"}`, + }, + } { + xdrSec := Int64(testCase.unix) + source := ClaimPredicate{ + Type: ClaimPredicateTypeClaimPredicateBeforeAbsoluteTime, + AbsBefore: &xdrSec, + } + + serialized, err := json.Marshal(source) + assert.NoError(t, err) + assert.JSONEq(t, testCase.expected, string(serialized)) + + var parsed ClaimPredicate + assert.NoError(t, json.Unmarshal(serialized, &parsed)) + assert.Equal(t, *parsed.AbsBefore, *source.AbsBefore) + } +} From 1fefea8ea8099a104ceec2cbe8ab01f0c0dd0261 Mon Sep 17 00:00:00 2001 From: Tamir Sen Date: Mon, 2 Nov 2020 21:43:11 +0100 Subject: [PATCH 2/3] Add extra defensive checks --- xdr/json.go | 7 ++++++- xdr/json_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/xdr/json.go b/xdr/json.go index 9c1c679fd6..7305fff21d 100644 --- a/xdr/json.go +++ b/xdr/json.go @@ -26,11 +26,16 @@ func (t ISO8601Time) MarshalJSON() ([]byte, error) { // UnmarshalJSON parses a JSON string into a ISO8601Time instance. func (t *ISO8601Time) UnmarshalJSON(b []byte) error { + if len(b) < 2 { + return fmt.Errorf("%s is too short", string(b)) + } + if string(b) == "null" { return nil } + if b[0] != '"' || b[len(b)-1] != '"' { - return fmt.Errorf("%s does not begin and end with double quotes", b) + return fmt.Errorf("%s does not begin and end with double quotes", string(b)) } trimmed := string(b[1 : len(b)-1]) m := reISO8601.FindStringSubmatch(trimmed) diff --git a/xdr/json_test.go b/xdr/json_test.go index 9a698c4606..0b196c5208 100644 --- a/xdr/json_test.go +++ b/xdr/json_test.go @@ -102,3 +102,54 @@ func TestAbsBeforeTimestamps(t *testing.T) { assert.Equal(t, *parsed.AbsBefore, *source.AbsBefore) } } + +func TestISO8601Time_UnmarshalJSON(t *testing.T) { + for _, testCase := range []struct { + name string + timestamp string + expectedParsed ISO8601Time + expectedError string + }{ + { + "null timestamp", + "null", + ISO8601Time{}, + "", + }, + { + "empty string", + "", + ISO8601Time{}, + " is too short", + }, + { + "too short", + "1", + ISO8601Time{}, + "1 is too short", + }, + { + "does not begin and end with double quotes", + "'1'", + ISO8601Time{}, + "'1' does not begin and end with double quotes", + }, + { + "could not extract time", + "\"2006-01-02aldfd\"", + ISO8601Time{}, + "Could not extract time: parsing time \"2006-01-02aldfd\" as \"2006-01-02T15:04:05Z07:00\": cannot parse \"aldfd\" as \"T\"", + }, + } { + t.Run(testCase.name, func(t *testing.T) { + ts := &ISO8601Time{} + err := ts.UnmarshalJSON([]byte(testCase.timestamp)) + if len(testCase.expectedError) == 0 { + assert.NoError(t, err) + assert.Equal(t, *ts, testCase.expectedParsed) + } else { + assert.EqualError(t, err, testCase.expectedError) + } + }) + } +} From 8eade2d588fcb02b75050a79f3a3c6a51bed430b Mon Sep 17 00:00:00 2001 From: Tamir Sen Date: Mon, 2 Nov 2020 22:05:28 +0100 Subject: [PATCH 3/3] Code review feedback --- xdr/json.go | 42 ++++++++++++++++++++++-------------------- xdr/json_test.go | 40 +++++++++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 35 deletions(-) diff --git a/xdr/json.go b/xdr/json.go index 7305fff21d..78915e0afb 100644 --- a/xdr/json.go +++ b/xdr/json.go @@ -10,8 +10,8 @@ import ( "github.com/stellar/go/support/errors" ) -// ISO8601Time is a timestamp which supports parsing dates which have a year outside the 0000..9999 range -type ISO8601Time struct { +// iso8601Time is a timestamp which supports parsing dates which have a year outside the 0000..9999 range +type iso8601Time struct { time.Time } @@ -20,28 +20,30 @@ type ISO8601Time struct { var reISO8601 = regexp.MustCompile(`^([-+]?\d{4,})-(\d{2})-(\d{2})`) // MarshalJSON serializes the timestamp to a string -func (t ISO8601Time) MarshalJSON() ([]byte, error) { - return []byte(fmt.Sprintf(`"%s"`, t.Format(time.RFC3339))), nil +func (t iso8601Time) MarshalJSON() ([]byte, error) { + ts := t.Format(time.RFC3339) + if t.Year() > 9999 { + ts = "+" + ts + } + + return json.Marshal(ts) } -// UnmarshalJSON parses a JSON string into a ISO8601Time instance. -func (t *ISO8601Time) UnmarshalJSON(b []byte) error { - if len(b) < 2 { - return fmt.Errorf("%s is too short", string(b)) +// UnmarshalJSON parses a JSON string into a iso8601Time instance. +func (t *iso8601Time) UnmarshalJSON(b []byte) error { + var s *string + if err := json.Unmarshal(b, &s); err != nil { + return err } - - if string(b) == "null" { + if s == nil { return nil } - if b[0] != '"' || b[len(b)-1] != '"' { - return fmt.Errorf("%s does not begin and end with double quotes", string(b)) - } - trimmed := string(b[1 : len(b)-1]) - m := reISO8601.FindStringSubmatch(trimmed) + text := *s + m := reISO8601.FindStringSubmatch(text) if len(m) != 4 { - return fmt.Errorf("UnmarshalJSON: cannot parse %s", trimmed) + return fmt.Errorf("UnmarshalJSON: cannot parse %s", text) } // No need to check for errors since the regexp guarantees the matches // are valid integers @@ -49,7 +51,7 @@ func (t *ISO8601Time) UnmarshalJSON(b []byte) error { month, _ := strconv.Atoi(m[2]) day, _ := strconv.Atoi(m[3]) - ts, err := time.Parse(time.RFC3339, "2006-01-02"+trimmed[len(m[0]):]) + ts, err := time.Parse(time.RFC3339, "2006-01-02"+text[len(m[0]):]) if err != nil { return errors.Wrap(err, "Could not extract time") } @@ -63,7 +65,7 @@ type claimPredicateJSON struct { Or *[]claimPredicateJSON `json:"or,omitempty"` Not *claimPredicateJSON `json:"not,omitempty"` Unconditional bool `json:"unconditional,omitempty"` - AbsBefore *ISO8601Time `json:"abs_before,omitempty"` + AbsBefore *iso8601Time `json:"abs_before,omitempty"` RelBefore *int64 `json:"rel_before,string,omitempty"` } @@ -150,8 +152,8 @@ func (c ClaimPredicate) toJSON() (claimPredicateJSON, error) { payload.Not = new(claimPredicateJSON) *payload.Not, err = c.MustNotPredicate().toJSON() case ClaimPredicateTypeClaimPredicateBeforeAbsoluteTime: - payload.AbsBefore = new(ISO8601Time) - *payload.AbsBefore = ISO8601Time{time.Unix(int64(c.MustAbsBefore()), 0).UTC()} + payload.AbsBefore = new(iso8601Time) + *payload.AbsBefore = iso8601Time{time.Unix(int64(c.MustAbsBefore()), 0).UTC()} case ClaimPredicateTypeClaimPredicateBeforeRelativeTime: payload.RelBefore = new(int64) *payload.RelBefore = int64(c.MustRelBefore()) diff --git a/xdr/json_test.go b/xdr/json_test.go index 0b196c5208..6ecc5a6818 100644 --- a/xdr/json_test.go +++ b/xdr/json_test.go @@ -75,16 +75,20 @@ func TestAbsBeforeTimestamps(t *testing.T) { }, { math.MaxInt64, - `{"abs_before":"292277026596-12-04T15:30:07Z"}`, + `{"abs_before":"+292277026596-12-04T15:30:07Z"}`, }, { -10, `{"abs_before":"1969-12-31T23:59:50Z"}`, }, + { + -9000 * year, + `{"abs_before":"-7025-12-23T00:00:00Z"}`, + }, { math.MinInt64, // this serialization doesn't make sense but at least it doesn't crash the marshaller - `{"abs_before":"292277026596-12-04T15:30:08Z"}`, + `{"abs_before":"+292277026596-12-04T15:30:08Z"}`, }, } { xdrSec := Int64(testCase.unix) @@ -107,42 +111,48 @@ func TestISO8601Time_UnmarshalJSON(t *testing.T) { for _, testCase := range []struct { name string timestamp string - expectedParsed ISO8601Time + expectedParsed iso8601Time expectedError string }{ { "null timestamp", "null", - ISO8601Time{}, + iso8601Time{}, "", }, { "empty string", "", - ISO8601Time{}, - " is too short", + iso8601Time{}, + "unexpected end of JSON input", }, { - "too short", + "not string", "1", - ISO8601Time{}, - "1 is too short", + iso8601Time{}, + "json: cannot unmarshal number into Go value of type string", + }, + { + "does not begin with double quotes", + "'1\"", + iso8601Time{}, + "invalid character '\\'' looking for beginning of value", }, { - "does not begin and end with double quotes", - "'1'", - ISO8601Time{}, - "'1' does not begin and end with double quotes", + "does not end with double quotes", + "\"1", + iso8601Time{}, + "unexpected end of JSON input", }, { "could not extract time", "\"2006-01-02aldfd\"", - ISO8601Time{}, + iso8601Time{}, "Could not extract time: parsing time \"2006-01-02aldfd\" as \"2006-01-02T15:04:05Z07:00\": cannot parse \"aldfd\" as \"T\"", }, } { t.Run(testCase.name, func(t *testing.T) { - ts := &ISO8601Time{} + ts := &iso8601Time{} err := ts.UnmarshalJSON([]byte(testCase.timestamp)) if len(testCase.expectedError) == 0 { assert.NoError(t, err)