Add validation for before/after parameters to pagination concern

diff --git a/app/controllers/concerns/pagination_methods.rb b/app/controllers/concerns/pagination_methods.rb
index 3dc9f52aad3057726c69435227ec381a3fe808f0..79ab10bfb1a6c9bb8c47937406b2d29952df9c70 100644 (file)
--- a/app/controllers/concerns/pagination_methods.rb
+++ b/app/controllers/concerns/pagination_methods.rb
@@ -6,6 +6,9 @@ module PaginationMethods
   ##
   # limit selected items to one page, get ids of first item before/after the page
   def get_page_items(items, includes: [], limit: 20)
   ##
   # limit selected items to one page, get ids of first item before/after the page
   def get_page_items(items, includes: [], limit: 20)

+    param! :before, Integer, :min => 1
+    param! :after, Integer, :min => 1
+

     id_column = "#{items.table_name}.id"
     page_items = if params[:before]
                    items.where("#{id_column} < ?", params[:before]).order(:id => :desc)
     id_column = "#{items.table_name}.id"
     page_items = if params[:before]
                    items.where("#{id_column} < ?", params[:before]).order(:id => :desc)

diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb
index 2b10402fa04cd14d159e3d3c76749cb42f758aab..d13a50163ea8bc59e15af03885e25385379ac71a 100644 (file)
--- a/test/controllers/diary_entries_controller_test.rb
+++ b/test/controllers/diary_entries_controller_test.rb
@@ -590,6 +590,17 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
     assert_select "li.page-item.disabled span.page-link", :text => "Newer Entries", :count => 1
   end
 
     assert_select "li.page-item.disabled span.page-link", :text => "Newer Entries", :count => 1
   end
 

+  def test_index_invalid_paged
+    # Try some invalid paged accesses
+    %w[-1 0 fred].each do |id|
+      get diary_entries_path(:before => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+
+      get diary_entries_path(:after => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+    end
+  end
+

   def test_rss
     create(:language, :code => "de")
     create(:diary_entry, :language_code => "en")
   def test_rss
     create(:language, :code => "de")
     create(:diary_entry, :language_code => "en")


@@ -899,6 +910,18 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
     assert_response :not_found
   end
 
     assert_response :not_found
   end
 

+  def test_comments_invalid_paged
+    user = create(:user)
+
+    %w[-1 0 fred].each do |id|
+      get diary_comments_path(:display_name => user.display_name, :before => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+
+      get diary_comments_path(:display_name => user.display_name, :after => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+    end
+  end
+

   def test_subscribe_page
     user = create(:user)
     other_user = create(:user)
   def test_subscribe_page
     user = create(:user)
     other_user = create(:user)

diff --git a/test/controllers/traces_controller_test.rb b/test/controllers/traces_controller_test.rb
index 73966641eb8641f346c25429bba0cd233f5421c0..972cbb3c32be43e42978e19f27c74c633464ac97 100644 (file)
--- a/test/controllers/traces_controller_test.rb
+++ b/test/controllers/traces_controller_test.rb
@@ -322,6 +322,17 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
     assert_select "li.page-item a.page-link", :text => "Older Traces", :count => 2
   end
 
     assert_select "li.page-item a.page-link", :text => "Older Traces", :count => 2
   end
 

+  def test_index_invalid_paged
+    # Try some invalid paged accesses
+    %w[-1 0 fred].each do |id|
+      get traces_path(:before => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+
+      get traces_path(:after => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+    end
+  end
+

   # Check the RSS feed
   def test_rss
     user = create(:user)
   # Check the RSS feed
   def test_rss
     user = create(:user)

diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb
index a7ab02c75fb9afa5011f8a1b275e743df1d3d677..97f5171335d4910fd27e53fda34bc6ea8dc8e2f5 100644 (file)
--- a/test/controllers/user_blocks_controller_test.rb
+++ b/test/controllers/user_blocks_controller_test.rb
@@ -115,6 +115,18 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
     check_no_page_link "Older Blocks"
   end
 
     check_no_page_link "Older Blocks"
   end
 

+  ##
+  # test the index action with invalid pages
+  def test_index_invalid_paged
+    %w[-1 0 fred].each do |id|
+      get user_blocks_path(:before => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+
+      get user_blocks_path(:after => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+    end
+  end
+

   ##
   # test the show action
   def test_show
   ##
   # test the show action
   def test_show


@@ -560,6 +572,20 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
     check_no_page_link "Older Blocks"
   end
 
     check_no_page_link "Older Blocks"
   end
 

+  ##
+  # test the blocks_on action with invalid pages
+  def test_blocks_on_invalid_paged
+    user = create(:user)
+
+    %w[-1 0 fred].each do |id|
+      get user_blocks_on_path(user, :before => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+
+      get user_blocks_on_path(user, :after => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+    end
+  end
+

   ##
   # test the blocks_by action
   def test_blocks_by
   ##
   # test the blocks_by action
   def test_blocks_by


@@ -628,6 +654,20 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
     check_no_page_link "Older Blocks"
   end
 
     check_no_page_link "Older Blocks"
   end
 

+  ##
+  # test the blocks_by action with invalid pages
+  def test_blocks_by_invalid_paged
+    user = create(:moderator_user)
+
+    %w[-1 0 fred].each do |id|
+      get user_blocks_by_path(user, :before => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+
+      get user_blocks_by_path(user, :after => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+    end
+  end
+

   private
 
   def check_user_blocks_table(user_blocks)
   private
 
   def check_user_blocks_table(user_blocks)

diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index 62bb34279eafa50c69a465ff65d5a2442768e693..02cc2eb582cbeea9abc91c7f15ac62e41f617e83 100644 (file)
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -668,6 +668,18 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     check_no_page_link "Older Users"
   end
 
     check_no_page_link "Older Users"
   end
 

+  def test_index_get_invalid_paginated
+    session_for(create(:administrator_user))
+
+    %w[-1 0 fred].each do |id|
+      get users_path(:before => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+
+      get users_path(:after => id)
+      assert_redirected_to :controller => :errors, :action => :bad_request
+    end
+  end
+

   private
 
   def check_no_page_link(name)
   private
 
   def check_no_page_link(name)