From ded84a4b585ad2691e581324ddd9375641452f3d Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 23 Apr 2018 11:54:58 +1000
Subject: [PATCH] PERF: improve performance once logged in rate limiter hits

If "logged in" is being forced anonymous on certain routes, trigger
the protection for any requests that spend 50ms queueing

This means that ...

1. You need to trip it by having 3 requests take longer than 1 second in 10 second interval
2. Once tripped, if your route is still spending 50m queueuing it will continue to be protected

This means that site will continue to function with almost no delays while it is scaling up to handle the new load
---
 config/discourse_defaults.conf                |  2 +-
 lib/middleware/anonymous_cache.rb             | 19 +++++++++----
 .../middleware/anonymous_cache_spec.rb        | 27 ++++++++++++++++---
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf
index 9c3cf3c38a7..7715cb29273 100644
--- a/config/discourse_defaults.conf
+++ b/config/discourse_defaults.conf
@@ -198,7 +198,7 @@ max_reqs_rate_limit_on_private = false
 # logged in DoS protection
 
 # protection will only trigger for requests that queue longer than this amount
-force_anonymous_min_queue_seconds = 2
+force_anonymous_min_queue_seconds = 1
 # only trigger anon if we see more than N requests for this path in last 10 seconds
 force_anonymous_min_per_10_seconds = 3
 
diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb
index 217f294a85e..17e3309d021 100644
--- a/lib/middleware/anonymous_cache.rb
+++ b/lib/middleware/anonymous_cache.rb
@@ -104,20 +104,29 @@ module Middleware
         request.delete_param('api_key')
       end
 
-      def check_logged_in_rate_limit!
-        limiter = RateLimiter.new(
+      def logged_in_anon_limiter
+        @logged_in_anon_limiter ||= RateLimiter.new(
           nil,
           "logged_in_anon_cache_#{@env["HOST"]}/#{@env["REQUEST_URI"]}",
           GlobalSetting.force_anonymous_min_per_10_seconds,
           10
         )
-        !limiter.performed!(raise_error: false)
       end
 
+      def check_logged_in_rate_limit!
+        !logged_in_anon_limiter.performed!(raise_error: false)
+      end
+
+      MIN_TIME_TO_CHECK = 0.05
+
       def should_force_anonymous?
-        if queue_time = @env['REQUEST_QUEUE_SECONDS']
-          if queue_time > GlobalSetting.force_anonymous_min_queue_seconds && get?
+        if (queue_time = @env['REQUEST_QUEUE_SECONDS']) && get?
+          if queue_time > GlobalSetting.force_anonymous_min_queue_seconds
             return check_logged_in_rate_limit!
+          elsif queue_time >= MIN_TIME_TO_CHECK
+            if !logged_in_anon_limiter.can_perform?
+              return check_logged_in_rate_limit!
+            end
           end
         end
 
diff --git a/spec/components/middleware/anonymous_cache_spec.rb b/spec/components/middleware/anonymous_cache_spec.rb
index 800f4d825ad..65ef9f63f65 100644
--- a/spec/components/middleware/anonymous_cache_spec.rb
+++ b/spec/components/middleware/anonymous_cache_spec.rb
@@ -79,18 +79,37 @@ describe Middleware::AnonymousCache::Helper do
         "rack.input" => StringIO.new
       }
 
-      app.call(env)
+      is_anon = false
+      app.call(env.dup)
       expect(is_anon).to eq(false)
 
-      app.call(env)
+      is_anon = false
+      app.call(env.dup)
       expect(is_anon).to eq(false)
 
-      app.call(env)
+      is_anon = false
+      app.call(env.dup)
       expect(is_anon).to eq(true)
 
-      _status, headers, _body = app.call(env)
+      is_anon = false
+      _status, headers, _body = app.call(env.dup)
       expect(is_anon).to eq(true)
       expect(headers['Set-Cookie']).to eq('dosp=1')
+
+      # tricky change, a 50ms delay still will trigger protection
+      # once it is tripped
+
+      env["REQUEST_QUEUE_SECONDS"] = 0.05
+      is_anon = false
+
+      app.call(env.dup)
+      expect(is_anon).to eq(true)
+
+      is_anon = false
+      env["REQUEST_QUEUE_SECONDS"] = 0.01
+
+      app.call(env.dup)
+      expect(is_anon).to eq(false)
     end
   end