From b47ed7b6991556a94cf720f7b985467eaa5157a1 Mon Sep 17 00:00:00 2001
From: sam <sam@vulpine.solutions>
Date: Mon, 2 Dec 2024 16:13:56 +0100
Subject: [PATCH] rate limit tweaks

the /users/{id} prefix contains most API routes so it's not a good idea
to put a single rate limit on *all* of them combined. the rate limiter
will now ignore the /users/{id} prefix *if* there's a second {id}
parameter in the URL.

also, X-RateLimit-Bucket is no longer hashed, so it can be directly
decoded by clients to get the actual bucket name. i'm not sure if this
will actually be useful, but it's nice to have the option.
---
 Foxnouns.Backend/Controllers/InternalController.cs | 14 ++++++++++++++
 rate/rate_limiter.go                               |  8 +-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/Foxnouns.Backend/Controllers/InternalController.cs b/Foxnouns.Backend/Controllers/InternalController.cs
index 0f857ef..c19d456 100644
--- a/Foxnouns.Backend/Controllers/InternalController.cs
+++ b/Foxnouns.Backend/Controllers/InternalController.cs
@@ -15,6 +15,9 @@ public partial class InternalController(DatabaseContext db) : ControllerBase
     [GeneratedRegex(@"(\{\w+\})")]
     private static partial Regex PathVarRegex();
 
+    [GeneratedRegex(@"\{id\}")]
+    private static partial Regex IdCountRegex();
+
     private static string GetCleanedTemplate(string template)
     {
         if (template.StartsWith("api/v2"))
@@ -22,8 +25,19 @@ public partial class InternalController(DatabaseContext db) : ControllerBase
         template = PathVarRegex()
             .Replace(template, "{id}") // Replace all path variables (almost always IDs) with `{id}`
             .Replace("@me", "{id}"); // Also replace hardcoded `@me` with `{id}`
+
+        // If there's at least one path parameter, we only return the *first* part of the path.
         if (template.Contains("{id}"))
+        {
+            // However, if the path starts with /users/{id} *and* there's another path parameter (such as a member ID)
+            // we ignore the leading /users/{id}. This is because a lot of routes are scoped by user, but should have
+            // separate rate limits from other user-scoped routes.
+            if (template.StartsWith("/users/{id}/") && IdCountRegex().Count(template) >= 2)
+                template = template["/users/{id}".Length..];
+
             return template.Split("{id}")[0] + "{id}";
+        }
+
         return template;
     }
 
diff --git a/rate/rate_limiter.go b/rate/rate_limiter.go
index d223197..6284243 100644
--- a/rate/rate_limiter.go
+++ b/rate/rate_limiter.go
@@ -1,7 +1,6 @@
 package main
 
 import (
-	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
 	"net/http"
@@ -91,12 +90,7 @@ func getReset(w http.ResponseWriter) int64 {
 }
 
 func requestBucket(method, template string) string {
-	hasher := sha256.New()
-	_, err := hasher.Write([]byte(method + "-" + template))
-	if err != nil {
-		panic(err)
-	}
-	return hex.EncodeToString(hasher.Sum(nil))
+	return hex.EncodeToString([]byte(method + "-" + template))
 }
 
 func (l *Limiter) globalLimiter(user string) *httprate.RateLimiter {