check for invalid rate limit inputs

This commit is contained in:
Hazelnoot 2024-12-08 09:46:49 -05:00
parent a7a1edc92e
commit 8b091f77ca
2 changed files with 155 additions and 3 deletions

View file

@ -131,6 +131,10 @@ export class SkRateLimiterService {
};
}
if (factor <= 0) {
throw new Error(`Rate limit factor is zero or negative: ${factor}`);
}
if (isLegacyRateLimit(limit)) {
return await this.limitLegacy(limit, actor, factor);
} else {
@ -149,7 +153,7 @@ export class SkRateLimiterService {
}
// Convert the "max" limit into a leaky bucket with 1 drip / second rate.
if (limit.max && limit.duration) {
if (limit.max != null && limit.duration != null) {
promises.push(
this.limitBucket({
type: 'bucket',
@ -172,6 +176,9 @@ export class SkRateLimiterService {
}
private async limitMin(limit: LegacyRateLimit & { minInterval: number }, actor: string, factor: number): Promise<LimitInfo | null> {
if (limit.minInterval === 0) return null;
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);
@ -205,10 +212,14 @@ export class SkRateLimiterService {
}
private async limitBucket(limit: RateLimit, actor: string, factor: number): Promise<LimitInfo> {
if (limit.size < 1) throw new Error(`Invalid rate limit ${limit.key}: size is less than 1 (${limit.size})`);
if (limit.dripRate != null && limit.dripRate < 1) throw new Error(`Invalid rate limit ${limit.key}: dripRate is less than 1 (${limit.dripRate})`);
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 dripRate = (limit.dripRate ?? 1000);
const dripSize = (limit.dripSize ?? 1);
const dripRate = Math.ceil(limit.dripRate ?? 1000);
const dripSize = Math.ceil(limit.dripSize ?? 1);
// Update drips
if (counter.c > 0) {

View file

@ -298,6 +298,90 @@ describe(SkRateLimiterService, () => {
expect(counter?.c).toBe(1);
expect(counter?.t).toBe(0);
});
it('should throw if factor is zero', async () => {
const promise = serviceUnderTest().limit(limit, actor, 0);
await expect(promise).rejects.toThrow(/factor is zero or negative/);
});
it('should throw if factor is negative', async () => {
const promise = serviceUnderTest().limit(limit, actor, -1);
await expect(promise).rejects.toThrow(/factor is zero or negative/);
});
it('should throw if size is zero', async () => {
limit.size = 0;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/size is less than 1/);
});
it('should throw if size is negative', async () => {
limit.size = -1;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/size is less than 1/);
});
it('should throw if size is fraction', async () => {
limit.size = 0.5;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/size is less than 1/);
});
it('should throw if dripRate is zero', async () => {
limit.dripRate = 0;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/dripRate is less than 1/);
});
it('should throw if dripRate is negative', async () => {
limit.dripRate = -1;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/dripRate is less than 1/);
});
it('should throw if dripRate is fraction', async () => {
limit.dripRate = 0.5;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/dripRate is less than 1/);
});
it('should throw if dripSize is zero', async () => {
limit.dripSize = 0;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/dripSize is less than 1/);
});
it('should throw if dripSize is negative', async () => {
limit.dripSize = -1;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/dripSize is less than 1/);
});
it('should throw if dripSize is fraction', async () => {
limit.dripSize = 0.5;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/dripSize is less than 1/);
});
});
describe('with min interval', () => {
@ -451,6 +535,35 @@ describe(SkRateLimiterService, () => {
expect(minCounter?.c).toBe(1);
expect(minCounter?.t).toBe(0);
});
it('should throw if factor is zero', async () => {
const promise = serviceUnderTest().limit(limit, actor, 0);
await expect(promise).rejects.toThrow(/factor is zero or negative/);
});
it('should throw if factor is negative', async () => {
const promise = serviceUnderTest().limit(limit, actor, -1);
await expect(promise).rejects.toThrow(/factor is zero or negative/);
});
it('should skip if minInterval is zero', async () => {
limit.minInterval = 0;
const info = await serviceUnderTest().limit(limit, actor);
expect(info.blocked).toBeFalsy();
expect(info.remaining).toBe(Number.MAX_SAFE_INTEGER);
});
it('should throw if minInterval is negative', async () => {
limit.minInterval = -1;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/minInterval is negative/);
});
});
describe('with legacy limit', () => {
@ -578,6 +691,34 @@ describe(SkRateLimiterService, () => {
expect(i1.blocked).toBeTruthy();
expect(i2.blocked).toBeFalsy();
});
it('should throw if factor is zero', async () => {
const promise = serviceUnderTest().limit(limit, actor, 0);
await expect(promise).rejects.toThrow(/factor is zero or negative/);
});
it('should throw if factor is negative', async () => {
const promise = serviceUnderTest().limit(limit, actor, -1);
await expect(promise).rejects.toThrow(/factor is zero or negative/);
});
it('should throw if max is zero', async () => {
limit.max = 0;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/size is less than 1/);
});
it('should throw if max is negative', async () => {
limit.max = -1;
const promise = serviceUnderTest().limit(limit, actor);
await expect(promise).rejects.toThrow(/size is less than 1/);
});
});
describe('with legacy limit and min interval', () => {