Skip to content

Commit

Permalink
Merge pull request #13964 from eduardoj/feature/introduce_search_with…
Browse files Browse the repository at this point in the history
…_configured_limit

Introduce limiting results for /search/... endpoints
  • Loading branch information
eduardoj committed Apr 3, 2023
2 parents c1bc9c7 + b0569a3 commit 6daa0b5
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 0 deletions.
25 changes: 25 additions & 0 deletions src/api/app/controllers/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,19 @@ def search(what, render_all)
items = find_items(what, predicate)

matches = items.size
if render_all && search_results_exceed_configured_limit?(matches)
render_error status: 403, errorcode: 'search_results_exceed_configured_limit', message: <<~MESSAGE.chomp
The number of results returned by the performed search exceeds the configured limit.
You can:
- retrieve only the ids by using an '/search/.../id' API endpoint, or
- reduce the number of matches of your search:
- paginating your results, through the 'limit' and 'offset' parameters, or
- adjusting your `match` expression.
MESSAGE

return
end

if params[:offset] || params[:limit]
# Add some pagination. Limiting the ids we have
Expand Down Expand Up @@ -275,4 +288,16 @@ def find_items(what, predicate)
raise IllegalXpathError, "Error found searching elements '#{what}' with xpath predicate: '#{predicate}'.\n\n" \
"Detailed error message from parser: #{e.message}"
end

def search_results_exceed_configured_limit?(matches)
config_limit = CONFIG['limit_for_search_results']
return false if config_limit.blank?

params_limit = params[:limit].present? && params[:limit] =~ /\A\d+\z/ ? params[:limit].to_i : nil

returned_results = params_limit.present? && params_limit < matches ? params_limit : matches
return false if returned_results <= config_limit

true
end
end
7 changes: 7 additions & 0 deletions src/api/config/options.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,13 @@ default: &default
# Former and default value is `false`: the user is added as a maintainer.
prevent_adding_maintainer_in_project_creation_with_api: false

# Limit results returned by this API endpoint:
# GET /search/...
# Return a 403 error if both:
# - number of matched results surpass the limit
# - the endpoint doesn't end in .../id
# limit_for_search_results: 10000

production:
<<: *default

Expand Down
19 changes: 19 additions & 0 deletions src/api/public/apidocs-new/paths/search_request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,25 @@ get:
summary: unable to evaluate 'foo' for 'requests'
'401':
$ref: '../components/responses/unauthorized.yaml'
'403':
description: |
Forbidden.
XML Schema used for body validation: [status.xsd](../schema/status.xsd).
content:
application/xml; charset=utf-8:
schema:
$ref: '../components/schemas/api_response.yaml'
example:
code: search_results_exceed_configured_limit
summary: |
The number of results returned by the performed search exceeds the configured limit.
You can:
- retrieve only the ids by using an '/search/.../id' API endpoint, or
- reduce the number of matches of your search:
- paginating your results, through the 'limit' and 'offset' parameters, or
- adjusting your `match` expression.
tags:
- Search

Expand Down
39 changes: 39 additions & 0 deletions src/api/spec/controllers/search_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,43 @@
end
end
end

describe 'search limited to 2 results', vcr: false do
render_views false

before do
stub_const('CONFIG', CONFIG.merge('limit_for_search_results' => 2))
end

let!(:package1) { create(:package, name: 'package_1') }
let!(:package2) { create(:package, name: 'package_2') }

describe 'same number of results than the limit' do
it 'returns results' do
get :package, params: { match: "starts_with(@name,'package_')" }, format: :xml

expect(response).to have_http_status(:success)
end
end

describe 'more number of results than the limit' do
let!(:package3) { create(:package, name: 'package_3') }

describe 'search without ids' do
it 'fails' do
get :package, params: { match: "starts_with(@name,'package_')" }, format: :xml

expect(response).to have_http_status(:forbidden)
end
end

describe 'search with ids' do
it 'returns results' do
get :package_id, params: { match: "starts_with(@name,'package_')" }, format: :xml

expect(response).to have_http_status(:success)
end
end
end
end
end

0 comments on commit 6daa0b5

Please sign in to comment.