From 6f72f4ccecdcb985248cc72ad34587f1d6a266b5 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Wed, 17 Jan 2024 20:34:12 -0300 Subject: [PATCH] chore: RateLimiter to async (#31480) --- apps/meteor/app/api/server/api.ts | 2 +- apps/meteor/app/lib/server/lib/RateLimiter.js | 55 +++++++++++++++++-- .../app/lib/server/startup/rateLimiter.js | 4 +- .../externals/meteor/rate-limit.d.ts | 8 ++- 4 files changed, 61 insertions(+), 8 deletions(-) diff --git a/apps/meteor/app/api/server/api.ts b/apps/meteor/app/api/server/api.ts index d4e8377a4dd7..b74163903604 100644 --- a/apps/meteor/app/api/server/api.ts +++ b/apps/meteor/app/api/server/api.ts @@ -322,7 +322,7 @@ export class APIClass extends Restivus { } rateLimiterDictionary[objectForRateLimitMatch.route].rateLimiter.increment(objectForRateLimitMatch); - const attemptResult = rateLimiterDictionary[objectForRateLimitMatch.route].rateLimiter.check(objectForRateLimitMatch); + const attemptResult = await rateLimiterDictionary[objectForRateLimitMatch.route].rateLimiter.check(objectForRateLimitMatch); const timeToResetAttempsInSeconds = Math.ceil(attemptResult.timeToReset / 1000); response.setHeader('X-RateLimit-Limit', rateLimiterDictionary[objectForRateLimitMatch.route].options.numRequestsAllowed); response.setHeader('X-RateLimit-Remaining', attemptResult.numInvocationsLeft); diff --git a/apps/meteor/app/lib/server/lib/RateLimiter.js b/apps/meteor/app/lib/server/lib/RateLimiter.js index b8c069ed290e..126b236426da 100644 --- a/apps/meteor/app/lib/server/lib/RateLimiter.js +++ b/apps/meteor/app/lib/server/lib/RateLimiter.js @@ -7,13 +7,60 @@ export const RateLimiterClass = new (class { if (process.env.TEST_MODE === 'true') { return fn; } - const rateLimiter = new RateLimiter(); + const rateLimiter = new (class extends RateLimiter { + async check(input) { + const reply = { + allowed: true, + timeToReset: 0, + numInvocationsLeft: Infinity, + }; + + const matchedRules = this._findAllMatchingRules(input); + + for await (const rule of matchedRules) { + const ruleResult = await rule.apply(input); + let numInvocations = rule.counters[ruleResult.key]; + + if (ruleResult.timeToNextReset < 0) { + // Reset all the counters since the rule has reset + await rule.resetCounter(); + ruleResult.timeSinceLastReset = new Date().getTime() - rule._lastResetTime; + ruleResult.timeToNextReset = rule.options.intervalTime; + numInvocations = 0; + } + + if (numInvocations > rule.options.numRequestsAllowed) { + // Only update timeToReset if the new time would be longer than the + // previously set time. This is to ensure that if this input triggers + // multiple rules, we return the longest period of time until they can + // successfully make another call + if (reply.timeToReset < ruleResult.timeToNextReset) { + reply.timeToReset = ruleResult.timeToNextReset; + } + reply.allowed = false; + reply.numInvocationsLeft = 0; + reply.ruleId = rule.id; + await rule._executeCallback(reply, input); + } else { + // If this is an allowed attempt and we haven't failed on any of the + // other rules that match, update the reply field. + if (rule.options.numRequestsAllowed - numInvocations < reply.numInvocationsLeft && reply.allowed) { + reply.timeToReset = ruleResult.timeToNextReset; + reply.numInvocationsLeft = rule.options.numRequestsAllowed - numInvocations; + } + reply.ruleId = rule.id; + await rule._executeCallback(reply, input); + } + } + return reply; + } + })(); Object.entries(matchers).forEach(([key, matcher]) => { - matchers[key] = (...args) => Promise.await(matcher(...args)); + matchers[key] = matcher; }); rateLimiter.addRule(matchers, numRequests, timeInterval); - return function (...args) { + return async function (...args) { const match = {}; Object.keys(matchers).forEach((key) => { @@ -21,7 +68,7 @@ export const RateLimiterClass = new (class { }); rateLimiter.increment(match); - const rateLimitResult = rateLimiter.check(match); + const rateLimitResult = await rateLimiter.check(match); if (rateLimitResult.allowed) { return fn.apply(null, args); } diff --git a/apps/meteor/app/lib/server/startup/rateLimiter.js b/apps/meteor/app/lib/server/startup/rateLimiter.js index a1ddfe87886c..5a312f4520d4 100644 --- a/apps/meteor/app/lib/server/startup/rateLimiter.js +++ b/apps/meteor/app/lib/server/startup/rateLimiter.js @@ -123,7 +123,7 @@ const checkNameForStream = (name) => name && !names.has(name) && name.startsWith const ruleIds = {}; -const callback = (msg, name) => (reply, input) => { +const callback = (msg, name) => async (reply, input) => { if (reply.allowed === false) { rateLimiterLog({ msg, reply, input }); metrics.ddpRateLimitExceeded.inc({ @@ -136,7 +136,7 @@ const callback = (msg, name) => (reply, input) => { }); // sleep before sending the error to slow down next requests if (slowDownRate > 0 && reply.numInvocationsExceeded) { - Promise.await(sleep(slowDownRate * reply.numInvocationsExceeded)); + await sleep(slowDownRate * reply.numInvocationsExceeded); } // } else { // console.log('DDP RATE LIMIT:', message); diff --git a/apps/meteor/definition/externals/meteor/rate-limit.d.ts b/apps/meteor/definition/externals/meteor/rate-limit.d.ts index 7f1e6cc8dde4..9d01911952ef 100644 --- a/apps/meteor/definition/externals/meteor/rate-limit.d.ts +++ b/apps/meteor/definition/externals/meteor/rate-limit.d.ts @@ -4,8 +4,14 @@ declare module 'meteor/rate-limit' { route: string; }; + type RateLimiterCheckResult = { + allowed: boolean; + timeToReset: number; + numInvocationsLeft: number; + }; + class RateLimiter { - public check(input: RateLimiterOptionsToCheck); + public check(input: RateLimiterOptionsToCheck): Promise; public increment(input: RateLimiterOptionsToCheck);