From dcac09ed32e0162e04e0328c4a10c6e03e60f148 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Tue, 28 Jan 2025 15:12:52 -0500 Subject: [PATCH] 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). --- app/controllers/search_controller.rb | 7 ++++++- spec/requests/search_controller_spec.rb | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index c868db5f691..2794a92649b 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -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 diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index e4666f8d12a..7b8edb37c8d 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -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" }