Replace r-simple-rate monkey-patches with fixed-window-check
The previous approach overrode simple-rate::tax-rate and rate:left directly, but Radiance reloads r-simple-rate at startup and clobbers the overrides. New approach: define-page-with-limit and define-api-with-limit now call fixed-window-check directly, bypassing rate:with-limitation entirely. The fixed-window logic is self-contained and immune to module reload ordering.
This commit is contained in:
Glenn Thompson
parent
91686cd0cc
commit
76d331248b
128
limiter.lisp
128
limiter.lisp
|
|
@ -1,10 +1,10 @@
|
|||
;;;; limiter.lisp - Rate limiter definitions for the application
|
||||
;;;;
|
||||
;;;; Includes monkey-patches for r-simple-rate's sliding-window bug:
|
||||
;;;; upstream tax-rate updates the timestamp on EVERY request, which
|
||||
;;;; prevents the window from ever resetting while polling is active.
|
||||
;;;; Our overrides use a proper fixed window — the timestamp is only
|
||||
;;;; updated when the window expires and the counter resets.
|
||||
;;;; Replaces r-simple-rate's with-limitation with a fixed-window
|
||||
;;;; implementation. The upstream tax-rate updates the timestamp on
|
||||
;;;; EVERY request, preventing the window from ever resetting while
|
||||
;;;; polling is active. Our define-*-with-limit macros bypass
|
||||
;;;; rate:with-limitation entirely and call fixed-window-check instead.
|
||||
|
||||
(in-package :asteroid)
|
||||
|
||||
|
|
@ -21,55 +21,49 @@
|
|||
(error (e)
|
||||
(l:warn :rate-limiter "Failed to cleanup rate limits: ~a" e))))
|
||||
|
||||
;;; ——— r-simple-rate fixed-window overrides ———
|
||||
;;; ——— Fixed-window rate limiter ———
|
||||
;;;
|
||||
;;; r-simple-rate has a sliding-window bug: tax-rate updates the timestamp
|
||||
;;; on every request, so the window never resets while polling is active.
|
||||
;;; Rather than monkey-patching (Radiance may reload the module and clobber
|
||||
;;; our overrides), we implement our own fixed-window logic directly.
|
||||
|
||||
(defun simple-rate::tax-rate (limit &key (ip (remote *request*)))
|
||||
"Fixed-window version of tax-rate.
|
||||
Only updates the timestamp when the window resets, not on every request.
|
||||
This prevents the sliding-window bug where continuous polling starves
|
||||
the counter because the reset condition never triggers."
|
||||
(let* ((limit (simple-rate::limit limit))
|
||||
(defun fixed-window-check (limit-name max-requests timeout-seconds)
|
||||
"Check and tax a fixed-window rate limit. Returns T if the request
|
||||
is allowed, or (VALUES NIL seconds-remaining) if rate-limited.
|
||||
LIMIT-NAME is a string key, MAX-REQUESTS and TIMEOUT-SECONDS are integers.
|
||||
Uses the SIMPLE-RATE/TRACKING table for storage."
|
||||
(let* ((ip (remote *request*))
|
||||
(tracking (dm:get-one 'simple-rate::tracking
|
||||
(db:query (:and (:= 'limit (simple-rate::name limit))
|
||||
(:= 'ip ip))))))
|
||||
(cond (tracking
|
||||
;; If the window has expired, reset counter AND timestamp
|
||||
(when (<= (+ (dm:field tracking "time")
|
||||
(simple-rate::timeout limit))
|
||||
(get-universal-time))
|
||||
(setf (dm:field tracking "amount") (simple-rate::amount limit))
|
||||
(setf (dm:field tracking "time") (get-universal-time)))
|
||||
;; Tax it (do NOT touch timestamp here — fixed window)
|
||||
(db:query (:and (:= 'limit limit-name)
|
||||
(:= 'ip ip)))))
|
||||
(now (get-universal-time)))
|
||||
(cond
|
||||
;; Existing entry
|
||||
(tracking
|
||||
(let ((window-end (+ (dm:field tracking "time") timeout-seconds)))
|
||||
(when (<= window-end now)
|
||||
;; Window expired — reset counter and start new window
|
||||
(setf (dm:field tracking "amount") max-requests)
|
||||
(setf (dm:field tracking "time") now)
|
||||
(setf window-end (+ now timeout-seconds)))
|
||||
;; Check budget
|
||||
(if (<= (dm:field tracking "amount") 0)
|
||||
;; Exhausted — report time remaining
|
||||
(values nil (- window-end now))
|
||||
;; Allowed — decrement and save
|
||||
(progn
|
||||
(decf (dm:field tracking "amount"))
|
||||
(dm:save tracking))
|
||||
(dm:save tracking)
|
||||
t))))
|
||||
;; First request ever from this IP for this limit
|
||||
(t
|
||||
(db:insert 'simple-rate::tracking
|
||||
`((limit . ,(simple-rate::name limit))
|
||||
(time . ,(get-universal-time))
|
||||
(amount . ,(simple-rate::amount limit))
|
||||
(ip . ,ip)))))))
|
||||
|
||||
(defun rate:left (limit &key (ip (remote *request*)))
|
||||
"Fixed-window version of rate:left.
|
||||
Returns correct remaining amount even for expired windows, so that
|
||||
with-limitation does not block on stale tracking entries."
|
||||
(let* ((limit (simple-rate::limit limit))
|
||||
(tracking (dm:get-one 'simple-rate::tracking
|
||||
(db:query (:and (:= 'limit (simple-rate::name limit))
|
||||
(:= 'ip ip))))))
|
||||
(if tracking
|
||||
(let ((window-end (+ (dm:field tracking "time")
|
||||
(simple-rate::timeout limit)))
|
||||
(now (get-universal-time)))
|
||||
(if (<= window-end now)
|
||||
;; Window expired — report full budget
|
||||
(values (simple-rate::amount limit) 0)
|
||||
;; Window still active
|
||||
(values (dm:field tracking "amount")
|
||||
(- window-end now))))
|
||||
;; No tracking entry yet — full budget
|
||||
(values (simple-rate::amount limit)
|
||||
(simple-rate::timeout limit)))))
|
||||
`((limit . ,limit-name)
|
||||
(time . ,now)
|
||||
(amount . ,(1- max-requests))
|
||||
(ip . ,ip)))
|
||||
t))))
|
||||
|
||||
(define-trigger db:connected ()
|
||||
"Clean up any corrupted rate limit entries on startup"
|
||||
|
|
@ -99,31 +93,31 @@
|
|||
|
||||
|
||||
(defmacro define-page-with-limit (name uri options &body body)
|
||||
"Rate limit for a page route. Defaults to 30 requests per minute."
|
||||
"Rate limit for a page route. Defaults to 30 requests per minute.
|
||||
Uses fixed-window rate limiting (not r-simple-rate's sliding window)."
|
||||
(multiple-value-bind (limit timeout group rest) (extract-limit-options options)
|
||||
(let* ((limit-name (string-upcase (format nil "~a-route-limit" (or group name))))
|
||||
(limit-sym (intern limit-name))
|
||||
(limit (or limit 30))
|
||||
(timeout (or timeout 60)))
|
||||
`(eval-when (:compile-toplevel :load-toplevel :execute)
|
||||
(rate:define-limit ,limit-sym (time-left :limit ,limit :timeout ,timeout)
|
||||
;; (format t "Route limit '~a' hit. Wait ~a seconds and retry.~%" ,(string name) time-left)
|
||||
(render-rate-limit-error-page))
|
||||
(define-page ,name ,uri ,rest
|
||||
(rate:with-limitation (,limit-sym)
|
||||
,@body))))))
|
||||
`(define-page ,name ,uri ,rest
|
||||
(multiple-value-bind (allowed time-left)
|
||||
(fixed-window-check ,limit-name ,limit ,timeout)
|
||||
(declare (ignorable time-left))
|
||||
(if allowed
|
||||
(progn ,@body)
|
||||
(render-rate-limit-error-page)))))))
|
||||
|
||||
(defmacro define-api-with-limit (name args options &body body)
|
||||
"Rate limit for api routes. Defaults to 60 requests per minute."
|
||||
"Rate limit for api routes. Defaults to 60 requests per minute.
|
||||
Uses fixed-window rate limiting (not r-simple-rate's sliding window)."
|
||||
(multiple-value-bind (limit timeout group rest) (extract-limit-options options)
|
||||
(let* ((limit-name (string-upcase (format nil "~a-api-limit" (or group name))))
|
||||
(limit-sym (intern limit-name))
|
||||
(limit (or limit 60))
|
||||
(timeout (or timeout 60)))
|
||||
`(eval-when (:compile-toplevel :load-toplevel :execute)
|
||||
(rate:define-limit ,limit-sym (time-left :limit ,limit :timeout ,timeout)
|
||||
;; (format t "API Rate limit '~a' hit. Wait ~a seconds and retry.~%" ,(string name) time-left)
|
||||
(api-limit-error-output))
|
||||
(define-api ,name ,args ,rest
|
||||
(rate:with-limitation (,limit-sym)
|
||||
,@body))))))
|
||||
`(define-api ,name ,args ,rest
|
||||
(multiple-value-bind (allowed time-left)
|
||||
(fixed-window-check ,limit-name ,limit ,timeout)
|
||||
(declare (ignorable time-left))
|
||||
(if allowed
|
||||
(progn ,@body)
|
||||
(api-limit-error-output)))))))
|
||||
|
|
|
|||
Loading…
Reference in New Issue