From daa88480cb6989875c784ee8f47de10239010d2a Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Tue, 16 Apr 2024 13:58:08 +0200 Subject: [PATCH] fix potential endless loop during queue msg/hook pagination when environment has TZ UTC, triggered by tests introduced in previous test time.Now() returns a timestamp with timezone Local. if you marshal & unmarshal it again, it'll get the Local timezone again. unless the local timezone is UTC. then it will get the UTC timezone. the same time.Time but with explicit UTC timezone vs explicit UTC-as-Local timezone are not the same when comparing with ==. so comparison should be done with time.Time.Equal, or comparison should be done after having called .Local() on parsed timestamps (so the explicit UTC timezone gets converted to the UTC-as-Local timezone). somewhat surprising that time.Local isn't the same as time.UTC if TZ=/TZ=UTC. there are warnings throughout the time package about handling of UTC. --- develop.txt | 2 +- queue/hook.go | 8 ++++---- queue/hook_test.go | 1 + queue/queue.go | 8 ++++---- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/develop.txt b/develop.txt index 6fcbea29bb..09ea215d0f 100644 --- a/develop.txt +++ b/develop.txt @@ -309,7 +309,7 @@ done - Update features & roadmap in README.md - Write release notes, copy from previous. - Build and run tests with previous major Go release. -- Run tests, including with race detector. +- Run tests, including with race detector, also with TZ= for UTC-behaviour. - Run integration and upgrade tests. - Run fuzzing tests for a while. - Deploy to test environment. Test the update instructions. diff --git a/queue/hook.go b/queue/hook.go index 3753bf0b72..0a66f7c171 100644 --- a/queue/hook.go +++ b/queue/hook.go @@ -313,9 +313,9 @@ func (s HookSort) apply(q *bstore.Query[Hook]) error { q.FilterNotEqual("ID", s.LastID) var fieldEqual func(h Hook) bool if s.Field == "NextAttempt" { - fieldEqual = func(h Hook) bool { return h.NextAttempt == last } + fieldEqual = func(h Hook) bool { return h.NextAttempt.Equal(last) } } else { - fieldEqual = func(h Hook) bool { return h.Submitted == last } + fieldEqual = func(h Hook) bool { return h.Submitted.Equal(last) } } if s.Asc { q.FilterGreaterEqual(s.Field, last) @@ -454,9 +454,9 @@ func (s HookRetiredSort) apply(q *bstore.Query[HookRetired]) error { q.FilterNotEqual("ID", s.LastID) var fieldEqual func(hr HookRetired) bool if s.Field == "LastActivity" { - fieldEqual = func(hr HookRetired) bool { return hr.LastActivity == last } + fieldEqual = func(hr HookRetired) bool { return hr.LastActivity.Equal(last) } } else { - fieldEqual = func(hr HookRetired) bool { return hr.Submitted == last } + fieldEqual = func(hr HookRetired) bool { return hr.Submitted.Equal(last) } } if s.Asc { q.FilterGreaterEqual(s.Field, last) diff --git a/queue/hook_test.go b/queue/hook_test.go index 0efa0af107..45346911d5 100644 --- a/queue/hook_test.go +++ b/queue/hook_test.go @@ -82,6 +82,7 @@ func TestHookIncoming(t *testing.T) { dec := json.NewDecoder(strings.NewReader(h.Payload)) err = dec.Decode(&in) tcheck(t, err, "decode incoming webhook") + in.Meta.Received = in.Meta.Received.Local() // For TZ UTC. expIncoming := webhook.Incoming{ From: []webhook.NameAddress{{Address: "mjl@mox.example"}}, diff --git a/queue/queue.go b/queue/queue.go index 09000bd3da..045d680914 100644 --- a/queue/queue.go +++ b/queue/queue.go @@ -492,9 +492,9 @@ func (s Sort) apply(q *bstore.Query[Msg]) error { q.FilterNotEqual("ID", s.LastID) var fieldEqual func(m Msg) bool if s.Field == "NextAttempt" { - fieldEqual = func(m Msg) bool { return m.NextAttempt == last } + fieldEqual = func(m Msg) bool { return m.NextAttempt.Equal(last) } } else { - fieldEqual = func(m Msg) bool { return m.Queued == last } + fieldEqual = func(m Msg) bool { return m.Queued.Equal(last) } } if s.Asc { q.FilterGreaterEqual(s.Field, last) @@ -1015,9 +1015,9 @@ func (s RetiredSort) apply(q *bstore.Query[MsgRetired]) error { q.FilterNotEqual("ID", s.LastID) var fieldEqual func(m MsgRetired) bool if s.Field == "LastActivity" { - fieldEqual = func(m MsgRetired) bool { return m.LastActivity == last } + fieldEqual = func(m MsgRetired) bool { return m.LastActivity.Equal(last) } } else { - fieldEqual = func(m MsgRetired) bool { return m.Queued == last } + fieldEqual = func(m MsgRetired) bool { return m.Queued.Equal(last) } } if s.Asc { q.FilterGreaterEqual(s.Field, last)