From fc4599ec07bb8def91fedf5afa9bb469bb54362a Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 8 Dec 2024 12:02:58 -0500 Subject: [PATCH] fix rate limit scaling (it's no longer inverted) --- .../backend/src/server/api/SkRateLimiterService.ts | 4 ++-- .../test/unit/server/api/SkRateLimiterServiceTests.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/backend/src/server/api/SkRateLimiterService.ts b/packages/backend/src/server/api/SkRateLimiterService.ts index d9abbfd406..1aee8aa799 100644 --- a/packages/backend/src/server/api/SkRateLimiterService.ts +++ b/packages/backend/src/server/api/SkRateLimiterService.ts @@ -95,7 +95,7 @@ export class SkRateLimiterService { if (limit.minInterval < 0) throw new Error(`Invalid rate limit ${limit.key}: minInterval is negative (${limit.minInterval})`); const counter = await this.getLimitCounter(limit, actor, 'min'); - const minInterval = Math.max(Math.ceil(limit.minInterval / factor), 0); + const minInterval = Math.max(Math.ceil(limit.minInterval * factor), 0); // Update expiration if (counter.c > 0) { @@ -132,7 +132,7 @@ export class SkRateLimiterService { if (limit.dripSize != null && limit.dripSize < 1) throw new Error(`Invalid rate limit ${limit.key}: dripSize is less than 1 (${limit.dripSize})`); const counter = await this.getLimitCounter(limit, actor, 'bucket'); - const bucketSize = Math.max(Math.ceil(limit.size * factor), 1); + const bucketSize = Math.max(Math.ceil(limit.size / factor), 1); const dripRate = Math.ceil(limit.dripRate ?? 1000); const dripSize = Math.ceil(limit.dripSize ?? 1); diff --git a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts index c7fac85706..910a3e5582 100644 --- a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts +++ b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts @@ -274,8 +274,8 @@ describe(SkRateLimiterService, () => { it('should scale limit by factor', async () => { counter = { c: 1, t: 0 }; - const i1 = await serviceUnderTest().limit(limit, actor, 2); // 1 + 1 = 2 - const i2 = await serviceUnderTest().limit(limit, actor, 2); // 2 + 1 = 3 + const i1 = await serviceUnderTest().limit(limit, actor, 0.5); // 1 + 1 = 2 + const i2 = await serviceUnderTest().limit(limit, actor, 0.5); // 2 + 1 = 3 expect(i1.blocked).toBeFalsy(); expect(i2.blocked).toBeTruthy(); @@ -514,7 +514,7 @@ describe(SkRateLimiterService, () => { minCounter = { c: 1, t: 0 }; mockTimeService.now += 500; - const info = await serviceUnderTest().limit(limit, actor, 2); + const info = await serviceUnderTest().limit(limit, actor, 0.5); expect(info.blocked).toBeFalsy(); }); @@ -657,7 +657,7 @@ describe(SkRateLimiterService, () => { it('should scale limit by factor', async () => { counter = { c: 10, t: 0 }; - const info = await serviceUnderTest().limit(limit, actor, 2); // 10 + 1 = 11 + const info = await serviceUnderTest().limit(limit, actor, 0.5); // 10 + 1 = 11 expect(info.blocked).toBeTruthy(); }); @@ -825,7 +825,7 @@ describe(SkRateLimiterService, () => { minCounter = { c: 1, t: 0 }; mockTimeService.now += 500; - const info = await serviceUnderTest().limit(limit, actor, 2); + const info = await serviceUnderTest().limit(limit, actor, 0.5); expect(info.blocked).toBeFalsy(); });