From c28976d89e79ff536efe5a4e82f43463f7e9c196 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Tue, 19 Dec 2023 05:55:39 -0500
Subject: [PATCH] Handle negative offset param in `api/v2/search` (#28282)

---
 app/controllers/api/base_controller.rb      |  8 ++++++++
 app/controllers/api/v2/search_controller.rb |  1 +
 spec/requests/api/v2/search_spec.rb         | 22 ++++++++++++++++++++-
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb
index b3c60fcaf4..98fa1897ef 100644
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@@ -108,6 +108,10 @@ class Api::BaseController < ApplicationController
     render json: { error: 'Your login is currently disabled' }, status: 403 if current_user&.account&.unavailable?
   end
 
+  def require_valid_pagination_options!
+    render json: { error: 'Pagination values for `offset` and `limit` must be positive' }, status: 400 if pagination_options_invalid?
+  end
+
   def require_user!
     if !current_user
       render json: { error: 'This method requires an authenticated user' }, status: 422
@@ -136,6 +140,10 @@ class Api::BaseController < ApplicationController
 
   private
 
+  def pagination_options_invalid?
+    params.slice(:limit, :offset).values.map(&:to_i).any?(&:negative?)
+  end
+
   def respond_with_error(code)
     render json: { error: Rack::Utils::HTTP_STATUS_CODES[code] }, status: code
   end
diff --git a/app/controllers/api/v2/search_controller.rb b/app/controllers/api/v2/search_controller.rb
index 4339bee21e..3cfc6e7919 100644
--- a/app/controllers/api/v2/search_controller.rb
+++ b/app/controllers/api/v2/search_controller.rb
@@ -12,6 +12,7 @@ class Api::V2::SearchController < Api::BaseController
     before_action :query_pagination_error, if: :pagination_requested?
     before_action :remote_resolve_error, if: :remote_resolve_requested?
   end
+  before_action :require_valid_pagination_options!
 
   def index
     @search = Search.new(search_results)
diff --git a/spec/requests/api/v2/search_spec.rb b/spec/requests/api/v2/search_spec.rb
index d0778cba4d..13bcf17984 100644
--- a/spec/requests/api/v2/search_spec.rb
+++ b/spec/requests/api/v2/search_spec.rb
@@ -40,7 +40,7 @@ describe 'Search API' do
           end
         end
 
-        context 'with `offset`' do
+        context 'with valid `offset` value' do
           let(:params) { { q: 'test1', offset: 1 } }
 
           it 'returns http unauthorized' do
@@ -50,6 +50,26 @@ describe 'Search API' do
           end
         end
 
+        context 'with negative `offset` value' do
+          let(:params) { { q: 'test1', offset: '-100', type: 'accounts' } }
+
+          it 'returns http bad_request' do
+            get '/api/v2/search', headers: headers, params: params
+
+            expect(response).to have_http_status(400)
+          end
+        end
+
+        context 'with negative `limit` value' do
+          let(:params) { { q: 'test1', limit: '-100', type: 'accounts' } }
+
+          it 'returns http bad_request' do
+            get '/api/v2/search', headers: headers, params: params
+
+            expect(response).to have_http_status(400)
+          end
+        end
+
         context 'with following=true' do
           let(:params) { { q: 'test', type: 'accounts', following: 'true' } }