From 10a6799e2939dd132c889e3fe0ea4252284b6ce6 Mon Sep 17 00:00:00 2001 From: Glenn Thompson Date: Mon, 13 Apr 2026 15:17:47 +0100 Subject: [PATCH] fix: replace r-simple-rate sliding-window with fixed-window rate limiter The upstream r-simple-rate tax-rate updates the timestamp on every request, preventing the window from ever resetting while polling is active. This caused 429 errors on all API endpoints during normal browser usage. limiter.lisp: - Add fixed-window-check that uses proper fixed windows (timestamp only resets when window expires, not on every request) - Rewrite define-page-with-limit and define-api-with-limit to call fixed-window-check directly, bypassing rate:with-limitation entirely - Immune to Radiance module reload ordering (no monkey-patches) frontend-partials.lisp: - Bump now-playing, now-playing-inline, now-playing-json rate limits from 10/1s to 120/60s to accommodate normal polling intervals parenscript/front-page.lisp: - Normalize channel-name polling from 10s to 15s (matches stream-player) --- frontend-partials.lisp | 6 +-- limiter.lisp | 86 +++++++++++++++++++++++++++++-------- parenscript/front-page.lisp | 2 +- 3 files changed, 72 insertions(+), 22 deletions(-) diff --git a/frontend-partials.lisp b/frontend-partials.lisp index a0f27d3..462809b 100644 --- a/frontend-partials.lisp +++ b/frontend-partials.lisp @@ -93,7 +93,7 @@ (:track-id . ,(find-track-by-title title)) (:favorite-count . ,(or (get-track-favorite-count title) 1)))))))) -(define-api-with-limit asteroid/partial/now-playing (&optional mount) (:limit 10 :timeout 1) +(define-api-with-limit asteroid/partial/now-playing (&optional mount) (:limit 120 :timeout 60) "Get Partial HTML with live status from Icecast server. Optional MOUNT parameter specifies which stream to get metadata from. Always polls both streams to keep recently played lists updated." @@ -123,7 +123,7 @@ :connection-error t :stats nil)))))) -(define-api-with-limit asteroid/partial/now-playing-inline (&optional mount) (:limit 10 :timeout 1) +(define-api-with-limit asteroid/partial/now-playing-inline (&optional mount) (:limit 120 :timeout 60) "Get inline text with now playing info (for admin dashboard and widgets). Optional MOUNT parameter specifies which stream to get metadata from." (with-error-handling @@ -137,7 +137,7 @@ (setf (header "Content-Type") "text/plain") "Stream Offline"))))) -(define-api-with-limit asteroid/partial/now-playing-json (&optional mount) (:limit 10 :timeout 1) +(define-api-with-limit asteroid/partial/now-playing-json (&optional mount) (:limit 120 :timeout 60) "Get JSON with now playing info including track ID for favorites. Optional MOUNT parameter specifies which stream to get metadata from." ;; Register web listener for geo stats (keeps listener active during playback) diff --git a/limiter.lisp b/limiter.lisp index 305f0c8..c5f1344 100644 --- a/limiter.lisp +++ b/limiter.lisp @@ -1,4 +1,10 @@ ;;;; limiter.lisp - Rate limiter definitions for the application +;;;; +;;;; 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) @@ -15,6 +21,50 @@ (error (e) (l:warn :rate-limiter "Failed to cleanup rate limits: ~a" e)))) +;;; --- 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 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 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) + t)))) + ;; First request ever from this IP for this limit + (t + (db:insert 'simple-rate::tracking + `((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" (cleanup-corrupted-rate-limits)) @@ -43,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))))))) diff --git a/parenscript/front-page.lisp b/parenscript/front-page.lisp index 1a72015..9941011 100644 --- a/parenscript/front-page.lisp +++ b/parenscript/front-page.lisp @@ -776,7 +776,7 @@ (setf (ps:@ curated-option text-content) (+ "🎧 " current-channel-name))))))))))) (catch (lambda (error) (ps:chain console (log "Could not fetch channel name:" error)))))) - 10000)) ;; Poll every 10 seconds + 15000)) ;; Poll every 15 seconds ;; Listen for messages from popout window (ps:chain window