fix(clientlimit): keep the submission limits free of the domain import
- commit
- 2576c64bc88fb5af75b6f80be82c97315168e839
- author
- brice · · 2026-10-19 07:04 UTC
- parent
- 8d2ab49
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +28 -14
M internal/clientlimit/submissions.go +16 -10
| @@ -8,23 +8,29 @@ import ( | |||
| 8 | 8 | "sync" | |
| 9 | 9 | "time" | |
| 10 | 10 | ||
| 11 | - | "bnema.dev/ermite/internal/domain" | |
| 12 | 11 | "bnema.dev/ermite/internal/ports" | |
| 13 | 12 | ) | |
| 14 | 13 | ||
| 15 | 14 | // Submission windows of the public issue form. | |
| 16 | 15 | const ( | |
| 17 | 16 | // HourWindow and DayWindow are the sliding windows of the limits | |
| 18 | - | // domain.SubmitPerHour and domain.SubmitPerDay. | |
| 17 | + | // PerHour and PerDay. | |
| 19 | 18 | HourWindow = time.Hour | |
| 20 | 19 | DayWindow = 24 * time.Hour | |
| 21 | 20 | ||
| 21 | + | // PerHour and PerDay are the submissions one client may make in each | |
| 22 | + | // window. They are the limits of the form (PerHour and | |
| 23 | + | // PerDay), repeated here because this package may not import | |
| 24 | + | // the domain; a test keeps them equal. | |
| 25 | + | PerHour = 3 | |
| 26 | + | PerDay = 10 | |
| 27 | + | ||
| 22 | 28 | submissionSweepInterval = HourWindow / 4 | |
| 23 | 29 | ) | |
| 24 | 30 | ||
| 25 | 31 | // SubmissionLimiter limits the submissions of the public issue form per client | |
| 26 | - | // (an IPv4 address or an IPv6 /64, see Key): domain.SubmitPerHour in a sliding | |
| 27 | - | // hour and domain.SubmitPerDay in a sliding day. It also counts the | |
| 32 | + | // (an IPv4 address or an IPv6 /64, see Key): PerHour in a sliding | |
| 33 | + | // hour and PerDay in a sliding day. It also counts the | |
| 28 | 34 | // submissions of all clients in the last hour. It lives in memory, is bounded | |
| 29 | 35 | // like Limiter, and is safe for concurrent use. | |
| 30 | 36 | type SubmissionLimiter struct { | |
| @@ -55,11 +61,11 @@ func (l *SubmissionLimiter) Allow(_ context.Context, addr string) (bool, time.Du | |||
| 55 | 61 | defer l.mu.Unlock() | |
| 56 | 62 | ts := l.prune(key, now) | |
| 57 | 63 | var wait time.Duration | |
| 58 | - | if h := withinWindow(ts, now, HourWindow); len(h) >= domain.SubmitPerHour { | |
| 59 | - | wait = h[len(h)-domain.SubmitPerHour].Add(HourWindow).Sub(now) | |
| 64 | + | if h := withinWindow(ts, now, HourWindow); len(h) >= PerHour { | |
| 65 | + | wait = h[len(h)-PerHour].Add(HourWindow).Sub(now) | |
| 60 | 66 | } | |
| 61 | - | if len(ts) >= domain.SubmitPerDay { | |
| 62 | - | wait = max(wait, ts[len(ts)-domain.SubmitPerDay].Add(DayWindow).Sub(now)) | |
| 67 | + | if len(ts) >= PerDay { | |
| 68 | + | wait = max(wait, ts[len(ts)-PerDay].Add(DayWindow).Sub(now)) | |
| 63 | 69 | } | |
| 64 | 70 | if wait <= 0 { | |
| 65 | 71 | return true, 0 | |
| @@ -75,8 +81,8 @@ func (l *SubmissionLimiter) Record(_ context.Context, addr string) { | |||
| 75 | 81 | defer l.mu.Unlock() | |
| 76 | 82 | l.maintain(now) | |
| 77 | 83 | ts := append(l.prune(key, now), now) | |
| 78 | - | if len(ts) > domain.SubmitPerDay*2 { | |
| 79 | - | ts = ts[len(ts)-domain.SubmitPerDay*2:] // enough to compute the wait | |
| 84 | + | if len(ts) > PerDay*2 { | |
| 85 | + | ts = ts[len(ts)-PerDay*2:] // enough to compute the wait | |
| 80 | 86 | } | |
| 81 | 87 | l.subs[key] = ts | |
| 82 | 88 | l.global = append(l.pruneGlobal(now), now) | |
| @@ -8,23 +8,29 @@ import ( | |||||
| 8 | "sync" | 8 | "sync" | ||
| 9 | "time" | 9 | "time" | ||
| 10 | 10 | ||||
| 11 | - | "bnema.dev/ermite/internal/domain" | |||
| 12 | "bnema.dev/ermite/internal/ports" | 11 | "bnema.dev/ermite/internal/ports" | ||
| 13 | ) | 12 | ) | ||
| 14 | 13 | ||||
| 15 | // Submission windows of the public issue form. | 14 | // Submission windows of the public issue form. | ||
| 16 | const ( | 15 | const ( | ||
| 17 | // HourWindow and DayWindow are the sliding windows of the limits | 16 | // HourWindow and DayWindow are the sliding windows of the limits | ||
| 18 | - | // domain.SubmitPerHour and domain.SubmitPerDay. | 17 | + | // PerHour and PerDay. |
| 19 | HourWindow = time.Hour | 18 | HourWindow = time.Hour | ||
| 20 | DayWindow = 24 * time.Hour | 19 | DayWindow = 24 * time.Hour | ||
| 21 | 20 | ||||
| 21 | + | // PerHour and PerDay are the submissions one client may make in each | |||
| 22 | + | // window. They are the limits of the form (PerHour and | |||
| 23 | + | // PerDay), repeated here because this package may not import | |||
| 24 | + | // the domain; a test keeps them equal. | |||
| 25 | + | PerHour = 3 | |||
| 26 | + | PerDay = 10 | |||
| 27 | + | ||||
| 22 | submissionSweepInterval = HourWindow / 4 | 28 | submissionSweepInterval = HourWindow / 4 | ||
| 23 | ) | 29 | ) | ||
| 24 | 30 | ||||
| 25 | // SubmissionLimiter limits the submissions of the public issue form per client | 31 | // SubmissionLimiter limits the submissions of the public issue form per client | ||
| 26 | - | // (an IPv4 address or an IPv6 /64, see Key): domain.SubmitPerHour in a sliding | 32 | + | // (an IPv4 address or an IPv6 /64, see Key): PerHour in a sliding |
| 27 | - | // hour and domain.SubmitPerDay in a sliding day. It also counts the | 33 | + | // hour and PerDay in a sliding day. It also counts the |
| 28 | // submissions of all clients in the last hour. It lives in memory, is bounded | 34 | // submissions of all clients in the last hour. It lives in memory, is bounded | ||
| 29 | // like Limiter, and is safe for concurrent use. | 35 | // like Limiter, and is safe for concurrent use. | ||
| 30 | type SubmissionLimiter struct { | 36 | type SubmissionLimiter struct { | ||
| @@ -55,11 +61,11 @@ func (l *SubmissionLimiter) Allow(_ context.Context, addr string) (bool, time.Du | |||||
| 55 | defer l.mu.Unlock() | 61 | defer l.mu.Unlock() | ||
| 56 | ts := l.prune(key, now) | 62 | ts := l.prune(key, now) | ||
| 57 | var wait time.Duration | 63 | var wait time.Duration | ||
| 58 | - | if h := withinWindow(ts, now, HourWindow); len(h) >= domain.SubmitPerHour { | 64 | + | if h := withinWindow(ts, now, HourWindow); len(h) >= PerHour { |
| 59 | - | wait = h[len(h)-domain.SubmitPerHour].Add(HourWindow).Sub(now) | 65 | + | wait = h[len(h)-PerHour].Add(HourWindow).Sub(now) |
| 60 | } | 66 | } | ||
| 61 | - | if len(ts) >= domain.SubmitPerDay { | 67 | + | if len(ts) >= PerDay { |
| 62 | - | wait = max(wait, ts[len(ts)-domain.SubmitPerDay].Add(DayWindow).Sub(now)) | 68 | + | wait = max(wait, ts[len(ts)-PerDay].Add(DayWindow).Sub(now)) |
| 63 | } | 69 | } | ||
| 64 | if wait <= 0 { | 70 | if wait <= 0 { | ||
| 65 | return true, 0 | 71 | return true, 0 | ||
| @@ -75,8 +81,8 @@ func (l *SubmissionLimiter) Record(_ context.Context, addr string) { | |||||
| 75 | defer l.mu.Unlock() | 81 | defer l.mu.Unlock() | ||
| 76 | l.maintain(now) | 82 | l.maintain(now) | ||
| 77 | ts := append(l.prune(key, now), now) | 83 | ts := append(l.prune(key, now), now) | ||
| 78 | - | if len(ts) > domain.SubmitPerDay*2 { | 84 | + | if len(ts) > PerDay*2 { |
| 79 | - | ts = ts[len(ts)-domain.SubmitPerDay*2:] // enough to compute the wait | 85 | + | ts = ts[len(ts)-PerDay*2:] // enough to compute the wait |
| 80 | } | 86 | } | ||
| 81 | l.subs[key] = ts | 87 | l.subs[key] = ts | ||
| 82 | l.global = append(l.pruneGlobal(now), now) | 88 | l.global = append(l.pruneGlobal(now), now) | ||
M internal/clientlimit/submissions_test.go +3 -4
| @@ -11,7 +11,6 @@ import ( | |||
| 11 | 11 | "github.com/stretchr/testify/require" | |
| 12 | 12 | ||
| 13 | 13 | "bnema.dev/ermite/internal/clientlimit" | |
| 14 | - | "bnema.dev/ermite/internal/domain" | |
| 15 | 14 | ) | |
| 16 | 15 | ||
| 17 | 16 | func newSubs() (*clientlimit.SubmissionLimiter, *fakeClock) { | |
| @@ -22,7 +21,7 @@ func newSubs() (*clientlimit.SubmissionLimiter, *fakeClock) { | |||
| 22 | 21 | func TestSubmissionHourWindow(t *testing.T) { | |
| 23 | 22 | t.Parallel() | |
| 24 | 23 | l, clk := newSubs() | |
| 25 | - | for i := range domain.SubmitPerHour { | |
| 24 | + | for i := range clientlimit.PerHour { | |
| 26 | 25 | ok, _ := l.Allow(bg, "1.2.3.4") | |
| 27 | 26 | require.True(t, ok, "submission %d", i) | |
| 28 | 27 | l.Record(bg, "1.2.3.4") | |
| @@ -45,7 +44,7 @@ func TestSubmissionDayWindow(t *testing.T) { | |||
| 45 | 44 | l, clk := newSubs() | |
| 46 | 45 | start := clk.t | |
| 47 | 46 | // 3 per hour, spaced so the hourly limit never bites: one every 2 hours. | |
| 48 | - | for range domain.SubmitPerDay { | |
| 47 | + | for range clientlimit.PerDay { | |
| 49 | 48 | ok, _ := l.Allow(bg, "1.2.3.4") | |
| 50 | 49 | require.True(t, ok) | |
| 51 | 50 | l.Record(bg, "1.2.3.4") | |
| @@ -64,7 +63,7 @@ func TestSubmissionDayWindow(t *testing.T) { | |||
| 64 | 63 | func TestSubmissionIPv6GroupsBy64(t *testing.T) { | |
| 65 | 64 | t.Parallel() | |
| 66 | 65 | l, _ := newSubs() | |
| 67 | - | for i := range domain.SubmitPerHour { | |
| 66 | + | for i := range clientlimit.PerHour { | |
| 68 | 67 | l.Record(bg, fmt.Sprintf("2001:db8:1:2::%x", i+1)) // same /64 | |
| 69 | 68 | } | |
| 70 | 69 | ok, _ := l.Allow(bg, "2001:db8:1:2:ffff::1") | |
| @@ -11,7 +11,6 @@ import ( | |||||
| 11 | "github.com/stretchr/testify/require" | 11 | "github.com/stretchr/testify/require" | ||
| 12 | 12 | ||||
| 13 | "bnema.dev/ermite/internal/clientlimit" | 13 | "bnema.dev/ermite/internal/clientlimit" | ||
| 14 | - | "bnema.dev/ermite/internal/domain" | |||
| 15 | ) | 14 | ) | ||
| 16 | 15 | ||||
| 17 | func newSubs() (*clientlimit.SubmissionLimiter, *fakeClock) { | 16 | func newSubs() (*clientlimit.SubmissionLimiter, *fakeClock) { | ||
| @@ -22,7 +21,7 @@ func newSubs() (*clientlimit.SubmissionLimiter, *fakeClock) { | |||||
| 22 | func TestSubmissionHourWindow(t *testing.T) { | 21 | func TestSubmissionHourWindow(t *testing.T) { | ||
| 23 | t.Parallel() | 22 | t.Parallel() | ||
| 24 | l, clk := newSubs() | 23 | l, clk := newSubs() | ||
| 25 | - | for i := range domain.SubmitPerHour { | 24 | + | for i := range clientlimit.PerHour { |
| 26 | ok, _ := l.Allow(bg, "1.2.3.4") | 25 | ok, _ := l.Allow(bg, "1.2.3.4") | ||
| 27 | require.True(t, ok, "submission %d", i) | 26 | require.True(t, ok, "submission %d", i) | ||
| 28 | l.Record(bg, "1.2.3.4") | 27 | l.Record(bg, "1.2.3.4") | ||
| @@ -45,7 +44,7 @@ func TestSubmissionDayWindow(t *testing.T) { | |||||
| 45 | l, clk := newSubs() | 44 | l, clk := newSubs() | ||
| 46 | start := clk.t | 45 | start := clk.t | ||
| 47 | // 3 per hour, spaced so the hourly limit never bites: one every 2 hours. | 46 | // 3 per hour, spaced so the hourly limit never bites: one every 2 hours. | ||
| 48 | - | for range domain.SubmitPerDay { | 47 | + | for range clientlimit.PerDay { |
| 49 | ok, _ := l.Allow(bg, "1.2.3.4") | 48 | ok, _ := l.Allow(bg, "1.2.3.4") | ||
| 50 | require.True(t, ok) | 49 | require.True(t, ok) | ||
| 51 | l.Record(bg, "1.2.3.4") | 50 | l.Record(bg, "1.2.3.4") | ||
| @@ -64,7 +63,7 @@ func TestSubmissionDayWindow(t *testing.T) { | |||||
| 64 | func TestSubmissionIPv6GroupsBy64(t *testing.T) { | 63 | func TestSubmissionIPv6GroupsBy64(t *testing.T) { | ||
| 65 | t.Parallel() | 64 | t.Parallel() | ||
| 66 | l, _ := newSubs() | 65 | l, _ := newSubs() | ||
| 67 | - | for i := range domain.SubmitPerHour { | 66 | + | for i := range clientlimit.PerHour { |
| 68 | l.Record(bg, fmt.Sprintf("2001:db8:1:2::%x", i+1)) // same /64 | 67 | l.Record(bg, fmt.Sprintf("2001:db8:1:2::%x", i+1)) // same /64 | ||
| 69 | } | 68 | } | ||
| 70 | ok, _ := l.Allow(bg, "2001:db8:1:2:ffff::1") | 69 | ok, _ := l.Allow(bg, "2001:db8:1:2:ffff::1") | ||
M internal/domain/pending_issue_test.go +9 -0
| @@ -5,6 +5,7 @@ package domain_test | |||
| 5 | 5 | import ( | |
| 6 | 6 | "strings" | |
| 7 | 7 | "testing" | |
| 8 | + | "time" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/stretchr/testify/assert" | |
| 10 | 11 | "github.com/stretchr/testify/require" | |
| @@ -57,3 +58,11 @@ func TestIssueSubmittedKind(t *testing.T) { | |||
| 57 | 58 | t.Parallel() | |
| 58 | 59 | assert.Equal(t, "issue.submitted", domain.IssueSubmitted{}.EventKind()) | |
| 59 | 60 | } | |
| 61 | + | ||
| 62 | + | func TestSubmissionLimitsAreTheDocumentedOnes(t *testing.T) { | |
| 63 | + | t.Parallel() | |
| 64 | + | assert.Equal(t, 3, domain.SubmitPerHour) | |
| 65 | + | assert.Equal(t, 10, domain.SubmitPerDay) | |
| 66 | + | assert.Equal(t, 200, domain.MaxPendingIssues) | |
| 67 | + | assert.Equal(t, 30*24*time.Hour, domain.BanDuration) | |
| 68 | + | } | |
| @@ -5,6 +5,7 @@ package domain_test | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "strings" | 6 | "strings" | ||
| 7 | "testing" | 7 | "testing" | ||
| 8 | + | "time" | |||
| 8 | 9 | ||||
| 9 | "github.com/stretchr/testify/assert" | 10 | "github.com/stretchr/testify/assert" | ||
| 10 | "github.com/stretchr/testify/require" | 11 | "github.com/stretchr/testify/require" | ||
| @@ -57,3 +58,11 @@ func TestIssueSubmittedKind(t *testing.T) { | |||||
| 57 | t.Parallel() | 58 | t.Parallel() | ||
| 58 | assert.Equal(t, "issue.submitted", domain.IssueSubmitted{}.EventKind()) | 59 | assert.Equal(t, "issue.submitted", domain.IssueSubmitted{}.EventKind()) | ||
| 59 | } | 60 | } | ||
| 61 | + | ||||
| 62 | + | func TestSubmissionLimitsAreTheDocumentedOnes(t *testing.T) { | |||
| 63 | + | t.Parallel() | |||
| 64 | + | assert.Equal(t, 3, domain.SubmitPerHour) | |||
| 65 | + | assert.Equal(t, 10, domain.SubmitPerDay) | |||
| 66 | + | assert.Equal(t, 200, domain.MaxPendingIssues) | |||
| 67 | + | assert.Equal(t, 30*24*time.Hour, domain.BanDuration) | |||
| 68 | + | } | |||