PERF: rate limit search, and add anon cache for search results (#9969)

PERF: rate limit search, and add anon cache for search results (#9969)

Adds new hidden site settings for rate limits: 30 for logged in users, 15 for anon

Adds an anon cache for searching, caches results of searches for 1 minute

diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
index a2dd66e..6bfce11 100644
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -28,6 +28,10 @@ class SearchController < ApplicationController
       raise Discourse::InvalidParameters.new("string contains null byte")
     end
 
+    rate_limit_errors = rate_limit_search
+
+    discourse_expires_in 1.minute
+
     search_args = {
       type_filter: 'topic',
       guardian: guardian,
@@ -48,7 +52,10 @@ class SearchController < ApplicationController
     search_args[:ip_address] = request.remote_ip
     search_args[:user_id] = current_user.id if current_user.present?
 
-    if site_overloaded?
+    if rate_limit_errors
+      result = Search::GroupedSearchResults.new(search_args[:type_filter], @search_term, context, false, 0)
+      result.error = I18n.t("rate_limiter.slow_down")
+    elsif site_overloaded?
       result = Search::GroupedSearchResults.new(search_args[:type_filter], @search_term, context, false, 0)
       result.error = I18n.t("search.extreme_load_error")
     else
@@ -76,6 +83,10 @@ class SearchController < ApplicationController
       raise Discourse::InvalidParameters.new("string contains null byte")
     end
 
+    rate_limit_errors = rate_limit_search
+
+    discourse_expires_in 1.minute
+
     search_args = { guardian: guardian }
 
     search_args[:type_filter] = params[:type_filter]                 if params[:type_filter].present?
@@ -94,7 +105,10 @@ class SearchController < ApplicationController
     search_args[:user_id] = current_user.id if current_user.present?
     search_args[:restrict_to_archetype] = params[:restrict_to_archetype] if params[:restrict_to_archetype].present?
 
-    if site_overloaded?
+    if rate_limit_errors
+      result = Search::GroupedSearchResults.new(search_args[:type_filter], @search_term, context, false, 0)
+      result.error = I18n.t("rate_limiter.slow_down")
+    elsif site_overloaded?
       result = GroupedSearchResults.new(search_args["type_filter"], params[:term], context, false, 0)
     else
       search = Search.new(params[:term], search_args)
@@ -140,6 +154,19 @@ class SearchController < ApplicationController
       (queue_time > GlobalSetting.disable_search_queue_threshold)
   end
 
+  def rate_limit_search
+    begin
+      if current_user.present?
+        RateLimiter.new(current_user, "search-min", SiteSetting.rate_limit_search_user, 1.minute).performed!
+      else
+        RateLimiter.new(nil, "search-min-#{request.remote_ip}", SiteSetting.rate_limit_search_anon, 1.minute).performed!
+      end
+    rescue RateLimiter::LimitExceeded => e
+      return e
+    end
+    false
+  end
+
   def cancel_overloaded_search
     if site_overloaded?
       render_json_error I18n.t("search.extreme_load_error"), status: 409
diff --git a/config/site_settings.yml b/config/site_settings.yml
index c658ec7..382f2f6 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1549,6 +1549,12 @@ rate_limits:
   rate_limit_create_post: 5
   rate_limit_new_user_create_topic: 120
   rate_limit_new_user_create_post: 30
+  rate_limit_search_anon:
+    hidden: true
+    default: 15
+  rate_limit_search_user:
+    hidden: true
+    default: 30
   max_topics_per_day: 20
   max_personal_messages_per_day: 20
   max_likes_per_day: 50
diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb
index dac1169..044e741 100644
--- a/spec/requests/search_controller_spec.rb
+++ b/spec/requests/search_controller_spec.rb
@@ -179,6 +179,61 @@ describe SearchController do
       expect(response.status).to eq(200)
       expect(SearchLog.where(term: 'wookie')).to be_blank
     end
+
+    context 'rate limited' do
+      before do
+        SiteSetting.rate_limit_search_user = 3
+        SiteSetting.rate_limit_search_anon = 2
+      end
+
+      it 'rate limits searches' do
+        RateLimiter.enable
+        RateLimiter.clear_all!
+
+        2.times do
+          get "/search/query.json", params: {
+                term: 'wookie'
+              }
+
+          expect(response.status).to eq(200)
+          json = response.parsed_body
+          expect(json["grouped_search_result"]["error"]).to eq(nil)
+        end
+
+        get "/search/query.json", params: {
+              term: 'wookie'
+            }
+        expect(response.status).to eq(200)
+        json = response.parsed_body
+        expect(json["grouped_search_result"]["error"]).to eq(I18n.t("rate_limiter.slow_down"))
+      end
+
+      context "and a logged in user" do
+        before { sign_in(user) }
+
+        it 'rate limits logged in searches' do
+          RateLimiter.enable
+          RateLimiter.clear_all!
+
+          3.times do
+            get "/search/query.json", params: {
+                  term: 'wookie'
+                }
+
+            expect(response.status).to eq(200)
+            json = response.parsed_body
+            expect(json["grouped_search_result"]["error"]).to eq(nil)
+          end
+
+          get "/search/query.json", params: {
+                term: 'wookie'
+              }
+          expect(response.status).to eq(200)
+          json = response.parsed_body
+          expect(json["grouped_search_result"]["error"]).to eq(I18n.t("rate_limiter.slow_down"))
+        end
+      end
+    end
   end
 
   context "#show" do
@@ -217,6 +272,63 @@ describe SearchController do
       expect(response.status).to eq(200)
       expect(SearchLog.where(term: 'bantha')).to be_blank
     end
+
+    context 'rate limited' do
+
+      before do
+        SiteSetting.rate_limit_search_user = 3
+        SiteSetting.rate_limit_search_anon = 2
+      end
+
+      it 'rate limits searches' do
+        RateLimiter.enable
+        RateLimiter.clear_all!
+
+        2.times do
+          get "/search.json", params: {
+                q: 'bantha'
+              }
+
+          expect(response.status).to eq(200)
+          json = response.parsed_body
+          expect(json["grouped_search_result"]["error"]).to eq(nil)
+        end
+
+        get "/search.json", params: {
+              q: 'bantha'
+            }
+        expect(response.status).to eq(200)
+        json = response.parsed_body
+        expect(json["grouped_search_result"]["error"]).to eq(I18n.t("rate_limiter.slow_down"))
+
+      end
+
+      context "and a logged in user" do
+        before { sign_in(user) }
+
+        it 'rate limits searches' do
+          RateLimiter.enable
+          RateLimiter.clear_all!
+
+          3.times do
+            get "/search.json", params: {
+                  q: 'bantha'
+                }
+
+            expect(response.status).to eq(200)
+            json = response.parsed_body
+            expect(json["grouped_search_result"]["error"]).to eq(nil)
+          end
+
+          get "/search.json", params: {
+                q: 'bantha'
+              }
+          expect(response.status).to eq(200)
+          json = response.parsed_body
+          expect(json["grouped_search_result"]["error"]).to eq(I18n.t("rate_limiter.slow_down"))
+        end
+      end
+    end
   end
 
   context "search priority" do

GitHub sha: de29b4a5

1 Like

This commit appears in #9969 which was merged by featheredtoast.

@featheredtoast It looks like this variable isn’t defined.