From 4ad0012f62cfae54e0c9832218aed820458063cf Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Mon, 6 Apr 2026 18:26:15 +0200 Subject: [PATCH] fix(handlers): guard GetBoostTimeLimit against past boost_time Live-server testing via protbot surfaced an inconsistency between GetBoostTimeLimit and GetBoostRight: on the same character, the former reported a far-future boost limit (2288912640 = year 2042) while the latter correctly reported "expired / available". The two handlers read the same boost_time row and disagreed. Root cause: GetBoostTimeLimit was doing a naked uint32(int64) cast on boostLimit.Unix(). The test character's boost_time was actually year 1906 (a pre-1970 sentinel left behind by the pre-#187 bug), whose negative int64 Unix timestamp wraps through uint32 to a huge positive value the client interprets as a permanently active boost. Harmonise the guard with GetBoostRight: return 0 whenever the stored boost_time is not strictly after TimeAdjusted(), covering both the pre-1970 wraparound and the "already expired" case. Add a healing migration (0011_fix_stale_boost_time) that NULLs out any boost_time column older than 1970 or more than 10 years in the future, so affected characters recover on upgrade without waiting for a fresh boost start. Regression test uses the exact year-1906 value observed on the live frontier.mogapedia.fr test account. --- server/channelserver/handlers_cafe.go | 11 ++++++--- server/channelserver/handlers_cafe_test.go | 23 +++++++++++++++++++ .../sql/0011_fix_stale_boost_time.sql | 18 +++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 server/migrations/sql/0011_fix_stale_boost_time.sql diff --git a/server/channelserver/handlers_cafe.go b/server/channelserver/handlers_cafe.go index c59e04d16..c7983c421 100644 --- a/server/channelserver/handlers_cafe.go +++ b/server/channelserver/handlers_cafe.go @@ -219,9 +219,14 @@ func handleMsgMhfGetBoostTimeLimit(s *Session, p mhfpacket.MHFPacket) { pkt := p.(*mhfpacket.MsgMhfGetBoostTimeLimit) bf := byteframe.NewByteFrame() boostLimit, err := s.server.charRepo.ReadTime(s.charID, "boost_time", time.Time{}) - // Return 0 when disabled, on read error, or when boost_time is unset - // (zero time.Time.Unix() wraps to a large uint32 the client interprets as active). - if err != nil || s.server.erupeConfig.GameplayOptions.DisableBoostTime || boostLimit.IsZero() { + // Return 0 when disabled, on read error, when boost_time is unset, + // or when boost_time is already in the past. A naked uint32() cast + // on a pre-1970 int64 wraps to a huge future-looking timestamp the + // client interprets as permanently active (see the year-1906 rows + // left behind by the pre-#187 bug). Harmonise with GetBoostRight + // so the two handlers always agree on the same row. + if err != nil || s.server.erupeConfig.GameplayOptions.DisableBoostTime || + boostLimit.IsZero() || !boostLimit.After(TimeAdjusted()) { bf.WriteUint32(0) } else { bf.WriteUint32(uint32(boostLimit.Unix())) diff --git a/server/channelserver/handlers_cafe_test.go b/server/channelserver/handlers_cafe_test.go index 5ee147faa..31b93f11f 100644 --- a/server/channelserver/handlers_cafe_test.go +++ b/server/channelserver/handlers_cafe_test.go @@ -617,6 +617,29 @@ func TestHandleMsgMhfGetBoostTimeLimit_DisableBoostTime(t *testing.T) { } } +// Regression: GetBoostTimeLimit must return 0 for a stale past boost_time +// (e.g. a pre-1970 sentinel left over from the pre-#187 bug). Without the +// guard, uint32(negative int64) wraps to a huge future-looking timestamp +// the client interprets as a permanently active boost — while +// GetBoostRight correctly reports it as expired, producing an observable +// inconsistency between the two handlers. +func TestHandleMsgMhfGetBoostTimeLimit_PastBoostTime(t *testing.T) { + server := createMockServer() + charMock := newMockCharacterRepo() + // Year 1906 — the actual value found on the live test server. + charMock.times["boost_time"] = time.Date(1906, 1, 1, 0, 0, 0, 0, time.UTC) + server.charRepo = charMock + session := createMockSession(1, server) + + handleMsgMhfGetBoostTimeLimit(session, &mhfpacket.MsgMhfGetBoostTimeLimit{AckHandle: 100}) + + p := <-session.sendPackets + payload := ackBufPayload(t, p.data) + if len(payload) != 4 || payload[0] != 0 || payload[1] != 0 || payload[2] != 0 || payload[3] != 0 { + t.Errorf("expected zero uint32 payload for past boost_time, got %x", payload) + } +} + // Regression for #187: GetBoostRight must report "no right" when disabled. func TestHandleMsgMhfGetBoostRight_DisableBoostTime(t *testing.T) { server := createMockServer() diff --git a/server/migrations/sql/0011_fix_stale_boost_time.sql b/server/migrations/sql/0011_fix_stale_boost_time.sql new file mode 100644 index 000000000..be56d9bdd --- /dev/null +++ b/server/migrations/sql/0011_fix_stale_boost_time.sql @@ -0,0 +1,18 @@ +-- Heal characters whose boost_time column holds a nonsensical value left +-- over from the pre-#187 bug. Two cases observed on live servers: +-- +-- 1. Pre-1970 timestamps (e.g. 1906-xx-xx) written when an uninitialised +-- time.Time wrapped through an int64->uint32 cast. +-- 2. Far-future timestamps that are not consistent with any legitimate +-- BoostTimeDuration config (BoostTimeDuration is capped at a few hours). +-- +-- Both are meaningless and should be NULL so that the boost system is +-- treated as inactive until the character triggers a fresh boost start. +-- NULL is the default for characters that have never boosted; see +-- handlers_cafe.go handleMsgMhfGetBoostTimeLimit / handleMsgMhfGetBoostRight. + +UPDATE public.characters + SET boost_time = NULL + WHERE boost_time IS NOT NULL + AND (boost_time < TIMESTAMP '1970-01-01 00:00:00' + OR boost_time > now() + interval '10 years');