From 2946f85592022f5aaa490d3f40d6e3068957d33f Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 8 Dec 2024 13:07:55 -0500 Subject: [PATCH] fix type errors from new rate limit definitions --- packages/backend/src/misc/rate-limit-utils.ts | 5 +++-- .../backend/src/server/FileServerService.ts | 8 ++++---- .../backend/src/server/api/ApiCallService.ts | 4 ++-- .../src/server/api/RateLimiterService.ts | 3 ++- .../src/server/api/SkRateLimiterService.ts | 16 ++++++++-------- packages/backend/src/server/api/endpoints.ts | 2 +- .../server/api/SkRateLimiterServiceTests.ts | 18 +++++------------- 7 files changed, 25 insertions(+), 31 deletions(-) diff --git a/packages/backend/src/misc/rate-limit-utils.ts b/packages/backend/src/misc/rate-limit-utils.ts index 6336f29cb7..9909bb97fa 100644 --- a/packages/backend/src/misc/rate-limit-utils.ts +++ b/packages/backend/src/misc/rate-limit-utils.ts @@ -6,6 +6,7 @@ import { FastifyReply } from 'fastify'; export type RateLimit = BucketRateLimit | LegacyRateLimit; +export type Keyed = T & { key: string }; /** * Rate limit based on "leaky bucket" logic. @@ -16,7 +17,7 @@ export interface BucketRateLimit { /** * Unique key identifying the particular resource (or resource group) being limited. */ - key: string; + key?: string; /** * Constant value identifying the type of rate limit. @@ -50,7 +51,7 @@ export interface LegacyRateLimit { /** * Unique key identifying the particular resource (or resource group) being limited. */ - key: string; + key?: string; /** * Constant value identifying the type of rate limit. diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts index 3a03cd8c00..5293d529ad 100644 --- a/packages/backend/src/server/FileServerService.ts +++ b/packages/backend/src/server/FileServerService.ts @@ -32,7 +32,7 @@ import { getIpHash } from '@/misc/get-ip-hash.js'; import { AuthenticateService } from '@/server/api/AuthenticateService.js'; import { RoleService } from '@/core/RoleService.js'; import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; -import { RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; +import { Keyed, RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify'; const _filename = fileURLToPath(import.meta.url); @@ -633,7 +633,7 @@ export class FileServerService { } private async checkResourceLimit(reply: FastifyReply, actor: string, group: string, resource: string, factor = 1): Promise { - const limit: RateLimit = { + const limit: Keyed = { // Group by resource key: `${group}${resource}`, type: 'bucket', @@ -647,7 +647,7 @@ export class FileServerService { } private async checkSharedLimit(reply: FastifyReply, actor: string, group: string, factor = 1): Promise { - const limit: RateLimit = { + const limit: Keyed = { key: group, type: 'bucket', @@ -658,7 +658,7 @@ export class FileServerService { return await this.checkLimit(reply, actor, limit, factor); } - private async checkLimit(reply: FastifyReply, actor: string, limit: RateLimit, factor = 1): Promise { + private async checkLimit(reply: FastifyReply, actor: string, limit: Keyed, factor = 1): Promise { const info = await this.rateLimiterService.limit(limit, actor, factor); sendRateLimitHeaders(reply, info); diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 6ad4bc8cb5..c6c33f7303 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -18,7 +18,7 @@ import { createTemp } from '@/misc/create-temp.js'; import { bindThis } from '@/decorators.js'; import { RoleService } from '@/core/RoleService.js'; import type { Config } from '@/config.js'; -import { RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; +import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { ApiError } from './error.js'; import { ApiLoggerService } from './ApiLoggerService.js'; @@ -327,7 +327,7 @@ export class ApiCallService implements OnApplicationShutdown { const limit = { key: ep.name, ...endpointLimit, - } as RateLimit; + }; // Rate limit const info = await this.rateLimiterService.limit(limit, limitActor, factor); diff --git a/packages/backend/src/server/api/RateLimiterService.ts b/packages/backend/src/server/api/RateLimiterService.ts index 305dcc1d6d..879529090f 100644 --- a/packages/backend/src/server/api/RateLimiterService.ts +++ b/packages/backend/src/server/api/RateLimiterService.ts @@ -10,6 +10,7 @@ import { DI } from '@/di-symbols.js'; import type Logger from '@/logger.js'; import { LoggerService } from '@/core/LoggerService.js'; import { bindThis } from '@/decorators.js'; +import { LegacyRateLimit } from '@/misc/rate-limit-utils.js'; import type { IEndpointMeta } from './endpoints.js'; /** @deprecated Use SkRateLimiterService instead */ @@ -32,7 +33,7 @@ export class RateLimiterService { } @bindThis - public limit(limitation: IEndpointMeta['limit'] & { key: NonNullable }, actor: string, factor = 1) { + public limit(limitation: LegacyRateLimit & { key: NonNullable }, actor: string, factor = 1) { return new Promise((ok, reject) => { if (this.disabled) ok(); diff --git a/packages/backend/src/server/api/SkRateLimiterService.ts b/packages/backend/src/server/api/SkRateLimiterService.ts index 1aee8aa799..6415ee905c 100644 --- a/packages/backend/src/server/api/SkRateLimiterService.ts +++ b/packages/backend/src/server/api/SkRateLimiterService.ts @@ -10,7 +10,7 @@ import { TimeService } from '@/core/TimeService.js'; import { EnvService } from '@/core/EnvService.js'; import { DI } from '@/di-symbols.js'; import type Logger from '@/logger.js'; -import { BucketRateLimit, LegacyRateLimit, LimitInfo, RateLimit, hasMinLimit, isLegacyRateLimit } from '@/misc/rate-limit-utils.js'; +import { BucketRateLimit, LegacyRateLimit, LimitInfo, RateLimit, hasMinLimit, isLegacyRateLimit, Keyed } from '@/misc/rate-limit-utils.js'; @Injectable() export class SkRateLimiterService { @@ -34,7 +34,7 @@ export class SkRateLimiterService { this.disabled = envService.env.NODE_ENV !== 'production'; // TODO disable in TEST *only* } - public async limit(limit: RateLimit, actor: string, factor = 1): Promise { + public async limit(limit: Keyed, actor: string, factor = 1): Promise { if (this.disabled || factor === 0) { return { blocked: false, @@ -57,7 +57,7 @@ export class SkRateLimiterService { } } - private async limitLegacy(limit: LegacyRateLimit, actor: string, factor: number): Promise { + private async limitLegacy(limit: Keyed, actor: string, factor: number): Promise { const promises: Promise[] = []; // The "min" limit - if present - is handled directly. @@ -90,7 +90,7 @@ export class SkRateLimiterService { }; } - private async limitMin(limit: LegacyRateLimit & { minInterval: number }, actor: string, factor: number): Promise { + private async limitMin(limit: Keyed & { minInterval: number }, actor: string, factor: number): Promise { if (limit.minInterval === 0) return null; if (limit.minInterval < 0) throw new Error(`Invalid rate limit ${limit.key}: minInterval is negative (${limit.minInterval})`); @@ -126,7 +126,7 @@ export class SkRateLimiterService { return limitInfo; } - private async limitBucket(limit: BucketRateLimit, actor: string, factor: number): Promise { + private async limitBucket(limit: Keyed, actor: string, factor: number): Promise { 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})`); @@ -166,7 +166,7 @@ export class SkRateLimiterService { return limitInfo; } - private async getLimitCounter(limit: RateLimit, actor: string, subject: string): Promise { + private async getLimitCounter(limit: Keyed, actor: string, subject: string): Promise { const key = createLimitKey(limit, actor, subject); const value = await this.redisClient.get(key); @@ -177,7 +177,7 @@ export class SkRateLimiterService { return JSON.parse(value); } - private async setLimitCounter(limit: RateLimit, actor: string, counter: LimitCounter, expiration: number, subject: string): Promise { + private async setLimitCounter(limit: Keyed, actor: string, counter: LimitCounter, expiration: number, subject: string): Promise { const key = createLimitKey(limit, actor, subject); const value = JSON.stringify(counter); const expirationSec = Math.max(expiration, 1); @@ -185,7 +185,7 @@ export class SkRateLimiterService { } } -function createLimitKey(limit: RateLimit, actor: string, subject: string): string { +function createLimitKey(limit: Keyed, actor: string, subject: string): string { return `rl_${actor}_${limit.key}_${subject}`; } diff --git a/packages/backend/src/server/api/endpoints.ts b/packages/backend/src/server/api/endpoints.ts index a4193a64ec..7eb18fbfe2 100644 --- a/packages/backend/src/server/api/endpoints.ts +++ b/packages/backend/src/server/api/endpoints.ts @@ -856,7 +856,7 @@ interface IEndpointMetaBase { * エンドポイントのリミテーションに関するやつ * 省略した場合はリミテーションは無いものとして解釈されます。 */ - readonly limit?: Readonly>; + readonly limit?: Readonly; /** * ファイルの添付を必要とするか否か diff --git a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts index 910a3e5582..dbf7795fc6 100644 --- a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts +++ b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts @@ -8,7 +8,7 @@ import { jest } from '@jest/globals'; import type Redis from 'ioredis'; import { LimitCounter, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { LoggerService } from '@/core/LoggerService.js'; -import { BucketRateLimit, LegacyRateLimit } from '@/misc/rate-limit-utils.js'; +import { BucketRateLimit, Keyed, LegacyRateLimit } from '@/misc/rate-limit-utils.js'; /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable @typescript-eslint/no-unnecessary-condition */ @@ -142,7 +142,7 @@ describe(SkRateLimiterService, () => { }); describe('with bucket limit', () => { - let limit: BucketRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -387,7 +387,7 @@ describe(SkRateLimiterService, () => { }); describe('with min interval', () => { - let limit: MutableLegacyRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -570,7 +570,7 @@ describe(SkRateLimiterService, () => { }); describe('with legacy limit', () => { - let limit: MutableLegacyRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -726,7 +726,7 @@ describe(SkRateLimiterService, () => { }); describe('with legacy limit and min interval', () => { - let limit: MutableLegacyRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -855,11 +855,3 @@ describe(SkRateLimiterService, () => { }); }); }); - -// The same thing, but mutable -interface MutableLegacyRateLimit extends LegacyRateLimit { - key: string; - duration?: number; - max?: number; - minInterval?: number; -}