From 82aef7af004110695ff667e9f1de9591ad5efe43 Mon Sep 17 00:00:00 2001 From: jlenon7 Date: Wed, 6 May 2026 22:47:32 -0300 Subject: [PATCH] fix: verify number of buckets --- package-lock.json | 4 +- package.json | 2 +- src/ratelimiter/RateLimitStore.ts | 18 +- tests/unit/ratelimiter/RateLimitStoreTest.ts | 170 +++++++++++++++++++ 4 files changed, 187 insertions(+), 7 deletions(-) create mode 100644 tests/unit/ratelimiter/RateLimitStoreTest.ts diff --git a/package-lock.json b/package-lock.json index 4ac4a28..39b01d1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@athenna/ratelimiter", - "version": "5.13.0", + "version": "5.14.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@athenna/ratelimiter", - "version": "5.13.0", + "version": "5.14.0", "license": "MIT", "devDependencies": { "@athenna/cache": "^5.2.0", diff --git a/package.json b/package.json index 225afed..05230ae 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@athenna/ratelimiter", - "version": "5.13.0", + "version": "5.14.0", "description": "Respect the rate limit rules of API's you need to consume.", "license": "MIT", "author": "João Lenon ", diff --git a/src/ratelimiter/RateLimitStore.ts b/src/ratelimiter/RateLimitStore.ts index 3344b42..c9e7450 100644 --- a/src/ratelimiter/RateLimitStore.ts +++ b/src/ratelimiter/RateLimitStore.ts @@ -10,7 +10,7 @@ import { debug } from '#src/debug' import { Cache } from '@athenna/cache' import { WINDOW_MS } from '#src/constants/window' -import { Uuid, Sleep, Macroable } from '@athenna/common' +import { Uuid, Sleep, Macroable, Is } from '@athenna/common' import type { Reserve, RateLimitRule, RateLimitStoreOptions } from '#src/types' export class RateLimitStore extends Macroable { @@ -48,10 +48,20 @@ export class RateLimitStore extends Macroable { return JSON.parse(initialized) as number[][] } - const parsed = JSON.parse(buckets) as number[][] + const parsed = JSON.parse(buckets) - if (parsed.length !== rules.length) { - const reconciled = rules.map((_, i) => parsed[i] ?? []) + const isValid = Is.Array(parsed) && + parsed.length === rules.length && + parsed.every(entry => Is.Array(entry)) + + if (!isValid) { + const src = Is.Array(parsed) ? parsed : [] + + const reconciled = rules.map((_, i) => { + const entry = src[i] + + return Is.Array(entry) ? entry : [] + }) await cache.set(key, JSON.stringify(reconciled)) diff --git a/tests/unit/ratelimiter/RateLimitStoreTest.ts b/tests/unit/ratelimiter/RateLimitStoreTest.ts new file mode 100644 index 0000000..491f0fb --- /dev/null +++ b/tests/unit/ratelimiter/RateLimitStoreTest.ts @@ -0,0 +1,170 @@ +/** + * @athenna/ratelimiter + * + * (c) João Lenon + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +import { Path } from '@athenna/common' +import type { RateLimitRule } from '#src/types' +import { RateLimiter, RateLimitStore } from '#src' +import { Cache, CacheProvider } from '@athenna/cache' +import { AfterEach, BeforeEach, Test, type Context } from '@athenna/test' + +export class RateLimitStoreTest { + private store: RateLimitStore + + @BeforeEach() + public async beforeEach() { + await Config.loadAll(Path.fixtures('config')) + + new CacheProvider().register() + + this.store = new RateLimitStore({ store: 'memory' }) + } + + @AfterEach() + public async afterEach() { + await Cache.store('memory').truncate() + + Config.clear() + ioc.reconstruct() + } + + @Test() + public async shouldReturnFreshBucketsWhenCacheHasNonArrayObject({ assert }: Context) { + const key = 'test:corrupted-object' + const rules: RateLimitRule[] = [ + { type: 'minute', limit: 5 }, + { type: 'hour', limit: 10 } + ] + + // Inject a plain object whose .length matches rules.length — the exact shape + // that previously caused `buckets[i]` to return undefined and crash. + await Cache.store('memory').set( + key, + JSON.stringify({ '0': [], length: 2 }) + ) + + const buckets = await this.store.getOrInit(key, rules) + + assert.isTrue(Array.isArray(buckets)) + assert.equal(buckets.length, rules.length) + buckets.forEach(bucket => assert.isTrue(Array.isArray(bucket))) + } + + @Test() + public async shouldReturnFreshBucketsWhenCacheHasNullEntries({ assert }: Context) { + const key = 'test:null-entries' + const rules: RateLimitRule[] = [ + { type: 'minute', limit: 5 }, + { type: 'hour', limit: 10 } + ] + + // null is valid JSON but not a valid bucket — previously bypassed the + // parsed.length check and reached tryReserve as undefined. + await Cache.store('memory').set(key, JSON.stringify([null, null])) + + const buckets = await this.store.getOrInit(key, rules) + + assert.isTrue(Array.isArray(buckets)) + assert.equal(buckets.length, rules.length) + buckets.forEach(bucket => assert.isTrue(Array.isArray(bucket))) + } + + @Test() + public async shouldReturnFreshBucketsWhenCacheHasScalarEntries({ assert }: Context) { + const key = 'test:scalar-entries' + const rules: RateLimitRule[] = [{ type: 'minute', limit: 5 }] + + // A flat array of timestamps (not nested arrays) — length matches but entries + // are numbers, not arrays. + await Cache.store('memory').set(key, JSON.stringify([Date.now()])) + + const buckets = await this.store.getOrInit(key, rules) + + assert.isTrue(Array.isArray(buckets)) + assert.equal(buckets.length, rules.length) + buckets.forEach(bucket => assert.isTrue(Array.isArray(bucket))) + } + + @Test() + public async shouldReturnFreshBucketsWhenCacheHasNonArrayPrimitive({ assert }: Context) { + const key = 'test:primitive' + const rules: RateLimitRule[] = [{ type: 'second', limit: 3 }] + + // A JSON number — .length is undefined so reconcile is triggered + await Cache.store('memory').set(key, JSON.stringify(42)) + + const buckets = await this.store.getOrInit(key, rules) + + assert.isTrue(Array.isArray(buckets)) + assert.equal(buckets.length, rules.length) + buckets.forEach(bucket => assert.isTrue(Array.isArray(bucket))) + } + + @Test() + public async shouldPreserveValidTimestampsWhenOnlySomeEntriesAreInvalid({ assert }: Context) { + const key = 'test:mixed-entries' + const now = Date.now() + const rules: RateLimitRule[] = [ + { type: 'minute', limit: 5 }, + { type: 'hour', limit: 10 } + ] + + // First bucket has valid timestamps, second is null — valid entries are kept. + await Cache.store('memory').set(key, JSON.stringify([[now], null])) + + const buckets = await this.store.getOrInit(key, rules) + + assert.isTrue(Array.isArray(buckets)) + assert.equal(buckets.length, 2) + assert.deepEqual(buckets[0], [now]) + assert.deepEqual(buckets[1], []) + } + + @Test() + public async shouldNotCrashInTryReserveWhenCacheHasCorruptedNonArrayObject({ assert }: Context) { + const key = 'test:reserve-corrupted' + const rules: RateLimitRule[] = [ + { type: 'minute', limit: 5 }, + { type: 'hour', limit: 10 } + ] + + // Pre-populate the cache with the exact shape that caused the production crash: + // a plain object with a `length` property equal to rules.length but missing + // numeric-keyed entries, so property access returns undefined. + await Cache.store('memory').set( + key, + JSON.stringify({ '0': [], length: 2 }) + ) + + // tryReserve must not throw "Cannot read properties of undefined (reading 'length')" + const result = await this.store.tryReserve(key, rules) + + assert.isObject(result) + assert.isDefined(result.allowed) + assert.isDefined(result.waitMs) + } + + @Test() + public async shouldNotCrashScheduleWhenCacheHasCorruptedData({ assert }: Context) { + const limiter = RateLimiter.build() + .key('test:schedule-corrupted') + .store('memory', { windowMs: { minute: 500, hour: 1000 } }) + .addRule({ type: 'minute', limit: 5 }) + .addRule({ type: 'hour', limit: 10 }) + + // Inject corrupted data directly to simulate what was seen in production. + await Cache.store('memory').set( + 'test:schedule-corrupted', + JSON.stringify({ '0': [], length: 2 }) + ) + + const result = await limiter.schedule(() => 'ok') + + assert.equal(result, 'ok') + } +}