From cbc53de8237fdfeee7c81effa819030962fa4b51 Mon Sep 17 00:00:00 2001
From: anatawa12 <anatawa12@icloud.com>
Date: Sun, 11 May 2025 15:37:46 +0900
Subject: fix: RateLimiterService (#13997)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix rate limit check never ends

* fix: long term / short term limitがないときでもそれぞれ用のnew Limiterとlimiter.getが呼ばれる問題

* refactor: wrap ratelimiter with promise

* refactor: reimplement max/min with async

* refactor: reimplement limit with async

* refactor: do not check long term limit inside min

* refactor: check if there is rate limit inside min/max function

* refactor: remove unnecessary return in min/max function

* refactor: remove unnecessary max/min function

* refactor: return rate limit instead of throwing an object

* fix: レートリミットのfactorが二回適用されて二乗の効果がある問題を修正

* fix lint error

---------

Co-authored-by: Kisaragi <48310258+KisaragiEffective@users.noreply.github.com>
Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com>
Co-authored-by: Sayamame-beans <61457993+Sayamame-beans@users.noreply.github.com>
---
 .../backend/src/server/api/RateLimiterService.ts   | 110 +++++++++------------
 1 file changed, 49 insertions(+), 61 deletions(-)

(limited to 'packages/backend/src/server/api/RateLimiterService.ts')

diff --git a/packages/backend/src/server/api/RateLimiterService.ts b/packages/backend/src/server/api/RateLimiterService.ts
index 52d73baa0a..58fd6c7d8b 100644
--- a/packages/backend/src/server/api/RateLimiterService.ts
+++ b/packages/backend/src/server/api/RateLimiterService.ts
@@ -12,6 +12,14 @@ import { LoggerService } from '@/core/LoggerService.js';
 import { bindThis } from '@/decorators.js';
 import type { IEndpointMeta } from './endpoints.js';
 
+type RateLimitInfo = {
+	code: 'BRIEF_REQUEST_INTERVAL',
+	info: Limiter.LimiterInfo,
+} | {
+	code: 'RATE_LIMIT_EXCEEDED',
+	info: Limiter.LimiterInfo,
+}
+
 @Injectable()
 export class RateLimiterService {
 	private logger: Logger;
@@ -31,77 +39,57 @@ export class RateLimiterService {
 	}
 
 	@bindThis
-	public limit(limitation: IEndpointMeta['limit'] & { key: NonNullable<string> }, actor: string, factor = 1) {
-		{
-			if (this.disabled) {
-				return Promise.resolve();
-			}
-
-			// Short-term limit
-			const min = new Promise<void>((ok, reject) => {
-				const minIntervalLimiter = new Limiter({
-					id: `${actor}:${limitation.key}:min`,
-					duration: limitation.minInterval! * factor,
-					max: 1,
-					db: this.redisClient,
-				});
-
-				minIntervalLimiter.get((err, info) => {
-					if (err) {
-						return reject({ code: 'ERR', info });
-					}
+	private checkLimiter(options: Limiter.LimiterOption): Promise<Limiter.LimiterInfo> {
+		return new Promise<Limiter.LimiterInfo>((resolve, reject) => {
+			new Limiter(options).get((err, info) => {
+				if (err) {
+					return reject(err);
+				}
+				resolve(info);
+			});
+		});
+	}
 
-					this.logger.debug(`${actor} ${limitation.key} min remaining: ${info.remaining}`);
+	@bindThis
+	public async limit(limitation: IEndpointMeta['limit'] & { key: NonNullable<string> }, actor: string, factor = 1): Promise<RateLimitInfo | null> {
+		if (this.disabled) {
+			return null;
+		}
 
-					if (info.remaining === 0) {
-						return reject({ code: 'BRIEF_REQUEST_INTERVAL', info });
-					} else {
-						if (hasLongTermLimit) {
-							return max.then(ok, reject);
-						} else {
-							return ok();
-						}
-					}
-				});
+		// Short-term limit
+		if (limitation.minInterval != null) {
+			const info = await this.checkLimiter({
+				id: `${actor}:${limitation.key}:min`,
+				duration: limitation.minInterval * factor,
+				max: 1,
+				db: this.redisClient,
 			});
 
-			// Long term limit
-			const max = new Promise<void>((ok, reject) => {
-				const limiter = new Limiter({
-					id: `${actor}:${limitation.key}`,
-					duration: limitation.duration! * factor,
-					max: limitation.max! / factor,
-					db: this.redisClient,
-				});
+			this.logger.debug(`${actor} ${limitation.key} min remaining: ${info.remaining}`);
 
-				limiter.get((err, info) => {
-					if (err) {
-						return reject({ code: 'ERR', info });
-					}
-
-					this.logger.debug(`${actor} ${limitation.key} max remaining: ${info.remaining}`);
+			if (info.remaining === 0) {
+				// eslint-disable-next-line no-throw-literal
+				return { code: 'BRIEF_REQUEST_INTERVAL', info };
+			}
+		}
 
-					if (info.remaining === 0) {
-						return reject({ code: 'RATE_LIMIT_EXCEEDED', info });
-					} else {
-						return ok();
-					}
-				});
+		// Long term limit
+		if (limitation.duration != null && limitation.max != null) {
+			const info = await this.checkLimiter({
+				id: `${actor}:${limitation.key}`,
+				duration: limitation.duration,
+				max: limitation.max / factor,
+				db: this.redisClient,
 			});
 
-			const hasShortTermLimit = typeof limitation.minInterval === 'number';
-
-			const hasLongTermLimit =
-				typeof limitation.duration === 'number' &&
-				typeof limitation.max === 'number';
+			this.logger.debug(`${actor} ${limitation.key} max remaining: ${info.remaining}`);
 
-			if (hasShortTermLimit) {
-				return min;
-			} else if (hasLongTermLimit) {
-				return max;
-			} else {
-				return Promise.resolve();
+			if (info.remaining === 0) {
+				// eslint-disable-next-line no-throw-literal
+				return { code: 'RATE_LIMIT_EXCEEDED', info };
 			}
 		}
+
+		return null;
 	}
 }
-- 
cgit v1.2.3-freya