From a83034eb2539fb86ef84140f01be125dc3f5fc43 Mon Sep 17 00:00:00 2001 From: Ryan O'Hara-Reid Date: Tue, 12 Nov 2024 14:40:58 +1100 Subject: [PATCH 1/4] types/time: strict usage of time type for usage with unix timestamps --- types/time.go | 15 ++++++++++++++- types/time_test.go | 10 ++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/types/time.go b/types/time.go index f42aeca95e5..f10668b1073 100644 --- a/types/time.go +++ b/types/time.go @@ -28,7 +28,20 @@ func (t *Time) UnmarshalJSON(data []byte) error { s = s[1 : len(s)-1] } - if target := strings.Index(s, "."); target != -1 { + badSyntax := false + target := strings.IndexFunc(s, func(r rune) bool { + if r == '.' { + return true + } + // As a mistake this type can be used instead of time.Time and parse int below will not fail. + badSyntax = r < '0' || r > '9' + return badSyntax // exit early + }) + + if target != -1 { + if badSyntax { + return strconv.ErrSyntax + } s = s[:target] + s[target+1:] } diff --git a/types/time_test.go b/types/time_test.go index 97e529294d4..a7c95459f12 100644 --- a/types/time_test.go +++ b/types/time_test.go @@ -56,17 +56,19 @@ func TestUnmarshalJSON(t *testing.T) { require.ErrorIs(t, json.Unmarshal([]byte(`"blurp"`), &testTime), strconv.ErrSyntax) require.Error(t, json.Unmarshal([]byte(`"123456"`), &testTime)) + + // Captures bad syntax when type should be time.Time (RFC3339) + require.ErrorIs(t, json.Unmarshal([]byte(`"2025-03-28T08:00:00Z"`), &testTime), strconv.ErrSyntax) + require.Error(t, json.Unmarshal([]byte(`"123456"`), &testTime)) } -// 5046307 216.0 ns/op 168 B/op 2 allocs/op (current) +// 5030734 240.1 ns/op 168 B/op 2 allocs/op (current) // 2716176 441.9 ns/op 352 B/op 6 allocs/op (previous) func BenchmarkUnmarshalJSON(b *testing.B) { var testTime Time for i := 0; i < b.N; i++ { err := json.Unmarshal([]byte(`"1691122380942.173000"`), &testTime) - if err != nil { - b.Fatal(err) - } + require.NoError(b, err) } } From e897bf1dd0bbcb2e4dc9a0c823e6578b14c1c975 Mon Sep 17 00:00:00 2001 From: Ryan O'Hara-Reid Date: Wed, 20 Nov 2024 13:45:12 +1100 Subject: [PATCH 2/4] Update types/time_test.go Co-authored-by: Adrian Gallagher --- types/time_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/time_test.go b/types/time_test.go index a7c95459f12..235f8417cc4 100644 --- a/types/time_test.go +++ b/types/time_test.go @@ -59,7 +59,7 @@ func TestUnmarshalJSON(t *testing.T) { // Captures bad syntax when type should be time.Time (RFC3339) require.ErrorIs(t, json.Unmarshal([]byte(`"2025-03-28T08:00:00Z"`), &testTime), strconv.ErrSyntax) - require.Error(t, json.Unmarshal([]byte(`"123456"`), &testTime)) + require.Error(t, json.Unmarshal([]byte(`123456`), &testTime)) } // 5030734 240.1 ns/op 168 B/op 2 allocs/op (current) From 53ed57e829baae6afa6efbbdd6f70f313e47575b Mon Sep 17 00:00:00 2001 From: Ryan O'Hara-Reid Date: Thu, 28 Nov 2024 12:00:23 +1100 Subject: [PATCH 3/4] @cranktakular: nits --- types/time_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/types/time_test.go b/types/time_test.go index 235f8417cc4..4563638934f 100644 --- a/types/time_test.go +++ b/types/time_test.go @@ -59,7 +59,8 @@ func TestUnmarshalJSON(t *testing.T) { // Captures bad syntax when type should be time.Time (RFC3339) require.ErrorIs(t, json.Unmarshal([]byte(`"2025-03-28T08:00:00Z"`), &testTime), strconv.ErrSyntax) - require.Error(t, json.Unmarshal([]byte(`123456`), &testTime)) + // parse int failure + require.ErrorIs(t, json.Unmarshal([]byte(`"1606292218213.45.8"`), &testTime), strconv.ErrSyntax) } // 5030734 240.1 ns/op 168 B/op 2 allocs/op (current) From 9ef3f7c28e536aa8cb9f148757115866a790a474 Mon Sep 17 00:00:00 2001 From: Ryan O'Hara-Reid Date: Tue, 3 Dec 2024 14:10:12 +1100 Subject: [PATCH 4/4] glorious: nits --- types/time.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/types/time.go b/types/time.go index f10668b1073..867a15bbadc 100644 --- a/types/time.go +++ b/types/time.go @@ -33,14 +33,15 @@ func (t *Time) UnmarshalJSON(data []byte) error { if r == '.' { return true } - // As a mistake this type can be used instead of time.Time and parse int below will not fail. + // types.Time may only parse numbers. The below check ensures an error is thrown. time.Time should be used to + // parse RFC3339 strings instead. badSyntax = r < '0' || r > '9' - return badSyntax // exit early + return badSyntax }) if target != -1 { if badSyntax { - return strconv.ErrSyntax + return fmt.Errorf("%w for `%v`", strconv.ErrSyntax, string(data)) } s = s[:target] + s[target+1:] }