From 19b59d6fee478a1fdd1eb2002b99dd41f737e472 Mon Sep 17 00:00:00 2001 From: fabi Date: Wed, 12 Aug 2026 20:51:51 +0200 Subject: [PATCH] docs(upload): stop claiming a proxy bandwidth control that does not exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_original`'s comment said bandwidth abuse "belongs at the proxy, where per-connection limits still work", which reads as though the removed per-IP limiter had been replaced by something. It was not: the Caddyfile sets timeouts and no rate or concurrency directive, and the tower stack is TraceLayer alone. Removing the limiter was right — the venue is one NAT address, so that bucket throttled the whole party's feed — but the route is now unbounded, and the comment should say so rather than imply cover. Records the actual cost (no-store plus the derivative fallback plus the nonce'd retry, against a 15-slot pool that upload commits compete for) and the shape a real fix would take: a concurrency semaphore over media streaming, not a request-rate bucket. --- backend/src/handlers/upload.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/backend/src/handlers/upload.rs b/backend/src/handlers/upload.rs index 3034753..1728d0b 100644 --- a/backend/src/handlers/upload.rs +++ b/backend/src/handlers/upload.rs @@ -1477,9 +1477,21 @@ async fn stream_media_file( /// /// A per-IP bucket cannot separate "one scraper" from "the entire party" when they share an /// address, and these four media routes are unauthenticated by design (an `` cannot send a -/// bearer token), so there is no per-user key to move to. Bandwidth abuse belongs at the proxy, -/// where per-connection limits still work; the certain harm here outweighed the speculative -/// protection. +/// bearer token), so there is no per-user key to move to. The certain harm outweighed the +/// speculative protection. +/// +/// BE HONEST ABOUT WHAT REPLACED IT: nothing did. This used to say "bandwidth abuse belongs at the +/// proxy, where per-connection limits still work", which reads as though a control exists there. +/// It does not — the `Caddyfile` sets timeouts and no rate or concurrency directive, and the tower +/// stack is `TraceLayer` alone. So this route is unbounded, deliberately, and the cost is real +/// rather than theoretical: `no-store` below plus the feed's fallback to `/original` for any photo +/// whose derivatives are still compressing plus `VirtualFeed`'s nonce'd retry means a hundred open +/// feeds can re-fetch full-resolution originals off the same disk Postgres writes WAL to, each one +/// also holding a connection from a 15-slot pool that upload commits are competing for. +/// +/// If that needs bounding, the shape that fits is a concurrency semaphore over media streaming +/// (like `upload_admission`), NOT a request-rate bucket — the venue is one IP, which is what made +/// the previous attempt a self-inflicted outage. pub async fn get_original( State(state): State, headers: axum::http::HeaderMap,