From 21d647f423c69b165a751daeeab818b6f06ae622 Mon Sep 17 00:00:00 2001 From: alyssa Date: Fri, 27 Sep 2024 16:57:32 +0900 Subject: [PATCH] fix(api): do sliding window rate limiting correctly (hopefully) --- services/api/src/middleware/ratelimit.lua | 10 ++++------ services/api/src/middleware/ratelimit.rs | 14 +++++--------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/services/api/src/middleware/ratelimit.lua b/services/api/src/middleware/ratelimit.lua index 30beadcd..f272cb9f 100644 --- a/services/api/src/middleware/ratelimit.lua +++ b/services/api/src/middleware/ratelimit.lua @@ -2,13 +2,11 @@ -- redis.replicate_commands() local rate_limit_key = KEYS[1] -local burst = ARGV[1] -local rate = ARGV[2] -local period = ARGV[3] +local rate = ARGV[1] +local period = ARGV[2] +local cost = tonumber(ARGV[3]) --- we're only ever asking for 1 request at a time --- todo: this is no longer true -local cost = 1 --local cost = tonumber(ARGV[4]) +local burst = rate local emission_interval = period / rate local increment = emission_interval * cost diff --git a/services/api/src/middleware/ratelimit.rs b/services/api/src/middleware/ratelimit.rs index e7cd35b1..05e55815 100644 --- a/services/api/src/middleware/ratelimit.rs +++ b/services/api/src/middleware/ratelimit.rs @@ -137,24 +137,23 @@ pub async fn do_request_ratelimited( rlimit.key() ); - let burst = 5; let period = 1; // seconds + let cost = 1; // todo: update this for group member endpoints // local rate_limit_key = KEYS[1] - // local burst = ARGV[1] - // local rate = ARGV[2] - // local period = ARGV[3] + // local rate = ARGV[1] + // local period = ARGV[2] // return {remaining, tostring(retry_after), reset_after} let resp = redis .evalsha::<(i32, String, u64), String, Vec, Vec>( LUA_SCRIPT_SHA.to_string(), vec![rl_key.clone()], - vec![burst, rlimit.rate(), period], + vec![rlimit.rate(), period, cost], ) .await; match resp { - Ok((mut remaining, retry_after, reset_after)) => { + Ok((remaining, retry_after, reset_after)) => { // redis's lua doesn't support returning floats let retry_after: f64 = retry_after .parse() @@ -175,9 +174,6 @@ pub async fn do_request_ratelimited( ) }; - // the redis script puts burst in remaining for ??? some reason - remaining -= burst - rlimit.rate(); - let reset_time = SystemTime::now() .checked_add(Duration::from_secs(reset_after)) .expect("invalid timestamp")