From f16baadec42a514b7b247c325f82176e7efdaec3 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 25 Oct 2026 21:11:36 +0000 Subject: [PATCH] Let a zero message, call or bandwidth limit admit nothing again The limit service reads a zero cap as no cap, so a tier with no calls (or no messages, or no attachment bandwidth on upload) became unlimited where main's FixedLimiter(0) and bytes limiter admitted nothing. The visitor now answers zero limits itself before asking the service, like it already did for requests, emails and bandwidth downloads. --- server/visitor.go | 9 +++++++++ server/visitor_test.go | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/server/visitor.go b/server/visitor.go index 37592dbf..49c460dc 100644 --- a/server/visitor.go +++ b/server/visitor.go @@ -169,6 +169,9 @@ func (v *visitor) FirebaseTemporarilyDeny() { func (v *visitor) MessageAllowed() bool { v.mu.RLock() // limits could be replaced! defer v.mu.RUnlock() + if v.limits.MessageLimit <= 0 { + return false // A daily limit of zero admits nothing; the limit service reads it as "no cap" + } allowed := v.limiter.Allow(v.key, v.messageRule(), 1).OK if allowed { v.localMessages.Add(1) @@ -192,6 +195,9 @@ func (v *visitor) EmailAllowed() bool { func (v *visitor) CallAllowed() bool { v.mu.RLock() // limits could be replaced! defer v.mu.RUnlock() + if v.limits.CallLimit <= 0 { + return false // A daily limit of zero admits nothing (e.g. a tier without calls, verified number or not) + } allowed := v.limiter.Allow(v.key, v.callRule(), 1).OK if allowed { v.localCalls.Add(1) @@ -282,6 +288,9 @@ func (v *visitor) Keepalive() { func (v *visitor) BandwidthLimiter() util.Limiter { v.mu.RLock() // limits could be replaced! defer v.mu.RUnlock() + if v.limits.AttachmentBandwidthLimit <= 0 { + return util.NewFixedLimiter(0) // A bandwidth limit of zero admits nothing, as BandwidthAllowed + } return v.limiter.Limiter(v.key, v.bandwidthRule()) } diff --git a/server/visitor_test.go b/server/visitor_test.go index 9757769a..29e6836a 100644 --- a/server/visitor_test.go +++ b/server/visitor_test.go @@ -163,3 +163,23 @@ func TestVisitor_EmailDailyLimitCapsBelowBurst(t *testing.T) { require.Equal(t, 429, response.Code) require.Equal(t, 42902, toHTTPError(t, response.Body.String()).Code) } + +func TestVisitor_ZeroLimitsAdmitNothing(t *testing.T) { + // A limit of zero admits nothing, as main's FixedLimiter(0) and bytes limiter did; the limit + // service reads a zero cap as "no cap", so the visitor must not hand it a zero (Codex review #3: + // a user downgraded to a tier without calls kept calling) + forEachBackend(t, func(t *testing.T, databaseURL string) { + c := newTestConfigWithAuthFile(t, databaseURL) + s := newTestServer(t, c) + require.Nil(t, s.userManager.AddTier(&user.Tier{Code: "zero", MessageLimit: 0, CallLimit: 0, AttachmentBandwidthLimit: 0})) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) + require.Nil(t, s.userManager.ChangeTier("phil", "zero")) + u, err := s.userManager.User("phil") + require.Nil(t, err) + v := s.visitor(netip.MustParseAddr("9.9.9.9"), u) + require.False(t, v.MessageAllowed()) + require.False(t, v.CallAllowed()) + require.False(t, v.BandwidthAllowed(1)) + require.False(t, v.BandwidthLimiter().AllowN(1)) + }) +}