Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xdr: Add a custom marshaller for claim predicate timestamp #3183

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions xdr/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
tamirms marked this conversation as resolved.
Show resolved Hide resolved
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
tamirms marked this conversation as resolved.
Show resolved Hide resolved
tamirms marked this conversation as resolved.
Show resolved Hide resolved
}

// 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] != '"' {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check len(b), otherwise it will panic for len(b) < 2.

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
Copy link
Member

@leighmcculloch leighmcculloch Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 I think we're doing more lifting than we need to do with the parsing we're doing here. Checking null, and looking for quotes is all work that the json package will do for us. We also don't need to parse out the full date with a regex manually given that the time package will do that for us. I think we should leverage json.Unmarshal if we can, so that we reduce our chance of not handling edge cases as much as possible.

This is one approach:

Suggested change
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
var s *string
err := json.Unmarshal(data, &s)
if err != nil {
return err
}
if s == nil {
return nil
}
parts := strings.SplitN(*s, "-", 2)
year, err := strconv.ParseInt(parts[0], 10, 64)
if err != nil {
return err
}
rest, err := time.Parse(time.RFC3339, "0000-"+parts[1])
if err != nil {
return err
}
t.Time = rest.AddDate(int(year), 0, 0)
return nil

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately splitting on the first "-" doesn't work on timestamps which have a negative year

}

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"`
}

Expand Down Expand Up @@ -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())
Expand Down
45 changes: 45 additions & 0 deletions xdr/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package xdr

import (
"encoding/json"
"math"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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)
}
}