diff --git a/packages/backend/src/server/SkRateLimiterService.md b/packages/backend/src/server/SkRateLimiterService.md index 55786664e1..4ec097ea8e 100644 --- a/packages/backend/src/server/SkRateLimiterService.md +++ b/packages/backend/src/server/SkRateLimiterService.md @@ -81,7 +81,7 @@ The Atomic Leaky Bucket algorithm is described here, in pseudocode: # * Delta Timestamp - Difference between current and expected timestamp value # 0 - Calculations -dripRate = ceil(limit.dripRate ?? 1000); +dripRate = ceil((limit.dripRate ?? 1000) * factor); dripSize = ceil(limit.dripSize ?? 1); bucketSize = max(ceil(limit.size / factor), 1); maxExpiration = max(ceil((dripRate * ceil(bucketSize / dripSize)) / 1000), 1);; diff --git a/packages/backend/src/server/SkRateLimiterService.ts b/packages/backend/src/server/SkRateLimiterService.ts index 35e87b0fe8..a53c58ba5a 100644 --- a/packages/backend/src/server/SkRateLimiterService.ts +++ b/packages/backend/src/server/SkRateLimiterService.ts @@ -206,7 +206,7 @@ export class SkRateLimiterService { // 0 - Calculate const now = this.timeService.now; const bucketSize = Math.max(Math.ceil(limit.size / factor), 1); - const dripRate = Math.ceil(limit.dripRate ?? 1000); + const dripRate = Math.ceil((limit.dripRate ?? 1000) * factor); const dripSize = Math.ceil(limit.dripSize ?? 1); const fullResetMs = dripRate * Math.ceil(bucketSize / dripSize); const fullResetSec = Math.max(Math.ceil(fullResetMs / 1000), 1); diff --git a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts index b1f100698b..f7250600e3 100644 --- a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts +++ b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts @@ -303,9 +303,12 @@ describe(SkRateLimiterService, () => { const i1 = await serviceUnderTest().limit(limit, actor); // 1 + 1 = 2 const i2 = await serviceUnderTest().limit(limit, actor); // 2 + 1 = 3 + mockTimeService.now += 500; // 3 - 1 = 2 (at 1/2 time) + const i3 = await serviceUnderTest().limit(limit, actor); expect(i1.blocked).toBeFalsy(); expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => { @@ -563,11 +566,15 @@ describe(SkRateLimiterService, () => { mockDefaultUserPolicies.rateLimitFactor = 0.5; limitCounter = 1; limitTimestamp = 0; + + const i1 = await serviceUnderTest().limit(limit, actor); + const i2 = await serviceUnderTest().limit(limit, actor); mockTimeService.now += 500; + const i3 = await serviceUnderTest().limit(limit, actor); - const info = await serviceUnderTest().limit(limit, actor); - - expect(info.blocked).toBeFalsy(); + expect(i1.blocked).toBeFalsy(); + expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => { @@ -738,12 +745,17 @@ describe(SkRateLimiterService, () => { it('should scale limit by factor', async () => { mockDefaultUserPolicies.rateLimitFactor = 0.5; - limitCounter = 10; + limitCounter = 1; limitTimestamp = 0; - const info = await serviceUnderTest().limit(limit, actor); // 10 + 1 = 11 + const i1 = await serviceUnderTest().limit(limit, actor); + const i2 = await serviceUnderTest().limit(limit, actor); + mockTimeService.now += 500; + const i3 = await serviceUnderTest().limit(limit, actor); - expect(info.blocked).toBeTruthy(); + expect(i1.blocked).toBeFalsy(); + expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => { @@ -932,13 +944,17 @@ describe(SkRateLimiterService, () => { it('should scale limit and interval by factor', async () => { mockDefaultUserPolicies.rateLimitFactor = 0.5; - limitCounter = 5; + limitCounter = 19; limitTimestamp = 0; + + const i1 = await serviceUnderTest().limit(limit, actor); + const i2 = await serviceUnderTest().limit(limit, actor); mockTimeService.now += 500; + const i3 = await serviceUnderTest().limit(limit, actor); - const info = await serviceUnderTest().limit(limit, actor); - - expect(info.blocked).toBeFalsy(); + expect(i1.blocked).toBeFalsy(); + expect(i2.blocked).toBeTruthy(); + expect(i3.blocked).toBeFalsy(); }); it('should set counter expiration', async () => {