DEV: Add proper error response when searching with an invalid page param (#31026)

Previously, for a search query with `page=11` or higher, we were quietly
returning the page 10 results. The frontend app isn't affected because
it sets its own limit to 10 pages, but still, this response from the
search endpoint does not make sense.

This change switches to returning a 400 error when the `page` parameter
is above the allowed limit (a max of 10).
This commit is contained in:
Penar Musaraj 2025-01-28 15:12:52 -05:00 committed by GitHub
parent 7334cfb191
commit dcac09ed32
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 11 additions and 1 deletions

View File

@ -5,6 +5,8 @@ class SearchController < ApplicationController
skip_before_action :check_xhr, only: :show
after_action :add_noindex_header
PAGE_LIMIT = 10
def self.valid_context_types
%w[user topic category private_messages tag]
end
@ -28,6 +30,9 @@ class SearchController < ApplicationController
page = permitted_params[:page]
# check for a malformed page parameter
raise Discourse::InvalidParameters if page && (!page.is_a?(String) || page.to_i.to_s != page)
if page && page.to_i > PAGE_LIMIT
raise Discourse::InvalidParameters.new("page parameter must not be greater than 10")
end
discourse_expires_in 1.minute
@ -35,7 +40,7 @@ class SearchController < ApplicationController
type_filter: "topic",
guardian: guardian,
blurb_length: 300,
page: ([page.to_i, 1].max if page.to_i <= 10),
page: [page.to_i, 1].max,
}
context, type = lookup_search_context

View File

@ -348,6 +348,11 @@ RSpec.describe SearchController do
expect(response.status).to eq(400)
end
it "returns a 400 error if the page parameter is above the limit" do
get "/search.json", params: { q: "kittens", page: 11 }
expect(response.status).to eq(400)
end
it "logs the search term" do
SiteSetting.log_search_queries = true
get "/search.json", params: { q: "bantha" }