Limit size of search query for non ES searches

When using non ES search without limits, we can easily get Stack level
too deep error. To fix it we implemented terms and character limits
for the search#show action.

app/controllers/search_controller.rb

@@ -5,6 +5,9 @@ class SearchController < ApplicationController
   include SearchHelper
   include RendersCommits
 
+  NON_ES_SEARCH_TERM_LIMIT = 64
+  NON_ES_SEARCH_CHAR_LIMIT = 4096
+
   around_action :allow_gitaly_ref_name_caching
 
   skip_before_action :authenticate_user!
@@ -21,6 +24,8 @@ class SearchController < ApplicationController
 
     return if params[:search].blank?
 
+    return unless search_term_valid?
+
     @search_term = params[:search]
 
     @scope = search_service.scope
@@ -62,6 +67,26 @@ class SearchController < ApplicationController
 
   private
 
+  def search_term_valid?
+    return true if Gitlab::CurrentSettings.elasticsearch_search?
+
+    chars_count = params[:search].length
+    if chars_count > NON_ES_SEARCH_CHAR_LIMIT
+      flash[:alert] = t('errors.messages.search_chars_too_long', count: NON_ES_SEARCH_CHAR_LIMIT)
+
+      return false
+    end
+
+    search_terms_count = params[:search].split.count { |word| word.length >= 3 }
+    if search_terms_count > NON_ES_SEARCH_TERM_LIMIT
+      flash[:alert] = t('errors.messages.search_terms_too_long', count: NON_ES_SEARCH_TERM_LIMIT)
+
+      return false
+    end
+
+    true
+  end
+
   def render_commits
     @search_objects = prepare_commits_for_rendering(@search_objects)
   end

config/locales/en.yml

@@ -195,6 +195,8 @@ en:
       wrong_length:
         one: is the wrong length (should be 1 character)
         other: is the wrong length (should be %{count} characters)
+      search_chars_too_long: Search query is too long (maximum is %{count} characters)
+      search_terms_too_long: Search query is too long (maximum is %{count} terms)
       other_than: must be other than %{count}
     template:
       body: 'There were problems with the following fields:'

doc/user/search/index.md

@@ -62,6 +62,7 @@ You can filter issues and merge requests by specific terms included in titles or
 - Limitation
   - For performance reasons, terms shorter than 3 chars are ignored. E.g.: searching
     issues for `included in titles` is same as `included titles`
+  - Search is limited to 4096 characters and 64 terms per query.
 
 ![filter issues by specific terms](img/issue_search_by_term.png)
 

ee/changelogs/unreleased/limit-size-of-search-query.yml

+---
+title: Limit size of search query for non ES searches
+merge_request: 22208
+author:
+type: other

spec/controllers/search_controller_spec.rb

@@ -92,6 +92,7 @@ describe SearchController do
     end
 
     context 'global search' do
+      using RSpec::Parameterized::TableSyntax
       render_views
 
       it 'omits pipeline status from load' do
@@ -102,6 +103,47 @@ describe SearchController do
 
         expect(assigns[:search_objects].first).to eq project
       end
+
+      context 'check search term length' do
+        let(:search_queries) do
+          char_limit = controller.class::NON_ES_SEARCH_CHAR_LIMIT
+          term_limit = controller.class::NON_ES_SEARCH_TERM_LIMIT
+          {
+            chars_under_limit: ('a' * (char_limit - 1)),
+            chars_over_limit: ('a' * (char_limit + 1)),
+            terms_under_limit: ('abc ' * (term_limit - 1)),
+            terms_over_limit: ('abc ' * (term_limit + 1))
+          }
+        end
+
+        where(:es_enabled, :string_name, :expectation) do
+          true  | :chars_under_limit | :not_to_set_flash
+          true  | :chars_over_limit  | :not_to_set_flash
+          true  | :terms_under_limit | :not_to_set_flash
+          true  | :terms_over_limit  | :not_to_set_flash
+          false | :chars_under_limit | :not_to_set_flash
+          false | :chars_over_limit  | :set_chars_flash
+          false | :terms_under_limit | :not_to_set_flash
+          false | :terms_over_limit  | :set_terms_flash
+        end
+
+        with_them do
+          it do
+            allow(Gitlab::CurrentSettings).to receive(:elasticsearch_search?).and_return(es_enabled)
+
+            get :show, params: { scope: 'projects', search: search_queries[string_name] }
+
+            case expectation
+            when :not_to_set_flash
+              expect(controller).not_to set_flash[:alert]
+            when :set_chars_flash
+              expect(controller).to set_flash[:alert].to(/characters/)
+            when :set_terms_flash
+              expect(controller).to set_flash[:alert].to(/terms/)
+            end
+          end
+        end
+      end
     end
 
     it 'finds issue comments' do