Fix more parameter sanitisation issues and add tests

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 2be52f7d691a0a3b04094d1ebd3a1e8236a50bc9..e89927c9d83dc00c472d55aad17c8843e1ed8489 100644 (file)
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -64,7 +64,7 @@ Metrics/BlockNesting:
 # Offense count: 62
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 1783
+  Max: 1790
 
 # Offense count: 69
 Metrics/CyclomaticComplexity:

diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index f4667bef5f47beaad0ff4da13a95f99e03beeb15..20894c4e83dbe1a0e426eee4a329d66d0945a249 100644 (file)
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -279,6 +279,7 @@ class NotesController < ApplicationController
   def mine
     if params[:display_name]
       if @this_user = User.active.find_by(:display_name => params[:display_name])
+        @params = params.permit(:display_name)
         @title = t "note.mine.title", :user => @this_user.display_name
         @heading = t "note.mine.heading", :user => @this_user.display_name
         @description = t "note.mine.subheading", :user => render_to_string(:partial => "user", :object => @this_user)

diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb
index 467ca4c3c13ec02f4a0baaa15c45e7df2e6c9b0f..ea5cdab10353987042340320a840b2fa1488f69e 100644 (file)
--- a/app/controllers/user_blocks_controller.rb
+++ b/app/controllers/user_blocks_controller.rb
@@ -12,6 +12,7 @@ class UserBlocksController < ApplicationController
   before_action :check_database_writable, :only => [:create, :update, :revoke]
 
   def index
+    @params = params.permit
     @user_blocks_pages, @user_blocks = paginate(:user_blocks,
                                                 :include => [:user, :creator, :revoker],
                                                 :order => "user_blocks.ends_at DESC",
@@ -88,6 +89,7 @@ class UserBlocksController < ApplicationController
   ##
   # shows a list of all the blocks on the given user
   def blocks_on
+    @params = params.permit(:display_name)
     @user_blocks_pages, @user_blocks = paginate(:user_blocks,
                                                 :include => [:user, :creator, :revoker],
                                                 :conditions => { :user_id => @this_user.id },
@@ -98,6 +100,7 @@ class UserBlocksController < ApplicationController
   ##
   # shows a list of all the blocks by the given user.
   def blocks_by
+    @params = params.permit(:display_name)
     @user_blocks_pages, @user_blocks = paginate(:user_blocks,
                                                 :include => [:user, :creator, :revoker],
                                                 :conditions => { :creator_id => @this_user.id },

diff --git a/app/views/notes/_notes_paging_nav.html.erb b/app/views/notes/_notes_paging_nav.html.erb
index 108cbb3d233747d75f94140c2529fc1737c63a72..862eae17c2ae257cc8c1a251f2ae27fc132ee32b 100644 (file)
--- a/app/views/notes/_notes_paging_nav.html.erb
+++ b/app/views/notes/_notes_paging_nav.html.erb
@@ -1,7 +1,7 @@
 <p>
 
 <% if @page > 1 %>
-<%= link_to t('changeset.changeset_paging_nav.previous'), params.merge({ :page => @page - 1 }) %>
+<%= link_to t('changeset.changeset_paging_nav.previous'), @params.merge({ :page => @page - 1 }) %>
 <% else %>
 <%= t('changeset.changeset_paging_nav.previous') %>
 <% end %>
@@ -11,7 +11,7 @@
 <% if @notes.size < @page_size %>
 <%= t('changeset.changeset_paging_nav.next') %>
 <% else %>
-<%= link_to t('changeset.changeset_paging_nav.next'), params.merge({ :page => @page + 1 }) %>
+<%= link_to t('changeset.changeset_paging_nav.next'), @params.merge({ :page => @page + 1 }) %>
 <% end %>
 
 </p>

diff --git a/app/views/user_blocks/_blocks.html.erb b/app/views/user_blocks/_blocks.html.erb
index d3908dab3e84caa102d9bdb568a9691eb9f9c317..e9dfc7185dd8d8fa110ac99b6bb4950f4483737d 100644 (file)
--- a/app/views/user_blocks/_blocks.html.erb
+++ b/app/views/user_blocks/_blocks.html.erb
@@ -20,7 +20,7 @@
 
 <ul class='secondary-actions'>
   <% if @user_blocks_pages.current_page.number > 1 -%>
-    <li><%= link_to t('user_block.partial.previous'), params.merge({ :page => @user_blocks_pages.current_page.number - 1 }) %></li>
+    <li><%= link_to t('user_block.partial.previous'), @params.merge({ :page => @user_blocks_pages.current_page.number - 1 }) %></li>
   <% else -%>
     <li><%= t('user_block.partial.previous') %></li>
   <% end -%>
@@ -28,7 +28,7 @@
   <li><%= t('user_block.partial.showing_page', :page => @user_blocks_pages.current_page.number) %></li>
 
   <% if @user_blocks_pages.current_page.number < @user_blocks_pages.page_count -%>
-    <li><%= link_to t('user_block.partial.next'), params.merge({ :page => @user_blocks_pages.current_page.number + 1 }) %></li>
+    <li><%= link_to t('user_block.partial.next'), @params.merge({ :page => @user_blocks_pages.current_page.number + 1 }) %></li>
   <% else -%>
     <li><%= t('user_block.partial.next') %></li>
   <% end -%>

diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb
index f1b029b39d5b2abb902f4771e9a7b90ee78611b3..654682ac083ad34722af114f206a9e191ffbd53f 100644 (file)
--- a/test/controllers/changeset_controller_test.rb
+++ b/test/controllers/changeset_controller_test.rb
@@ -2044,6 +2044,18 @@ EOF
     check_list_result(Changeset.where("id <= 4"))
   end
 
+  ##
+  # Check that a list with a next page link works
+  def test_list_more
+    create_list(:changeset, 50)
+
+    get :list, :params => { :format => "html" }
+    assert_response :success
+
+    get :list, :params => { :format => "html" }, :xhr => true
+    assert_response :success
+  end
+
   ##
   # This should display the last 20 non-empty changesets
   def test_feed

diff --git a/test/controllers/diary_entry_controller_test.rb b/test/controllers/diary_entry_controller_test.rb
index 58fffb8d3f39f1b908e2a4e8a6b926db7c508e17..fcd93eb2ab179ddc4548d8959880d7b63dde0086 100644 (file)
--- a/test/controllers/diary_entry_controller_test.rb
+++ b/test/controllers/diary_entry_controller_test.rb
@@ -570,6 +570,21 @@ class DiaryEntryControllerTest < ActionController::TestCase
     check_diary_list
   end
 
+  def test_list_paged
+    # Create several pages worth of diary entries
+    create_list(:diary_entry, 50)
+
+    # Try and get the list
+    get :list
+    assert_response :success
+    assert_select "div.diary_post", :count => 20
+
+    # Try and get the second page
+    get :list, :params => { :page => 2 }
+    assert_response :success
+    assert_select "div.diary_post", :count => 20
+  end
+
   def test_rss
     create(:language, :code => "de")
     create(:diary_entry, :language_code => "en")

diff --git a/test/controllers/notes_controller_test.rb b/test/controllers/notes_controller_test.rb
index c6d89f1ca339d6ccc6c9b966b0771947915e0091..e19810851bef03039c75bd6fa16b94b0964ab65a 100644 (file)
--- a/test/controllers/notes_controller_test.rb
+++ b/test/controllers/notes_controller_test.rb
@@ -999,4 +999,20 @@ class NotesControllerTest < ActionController::TestCase
     get :mine, :params => { :display_name => "non-existent" }
     assert_response :not_found
   end
+
+  def test_mine_paged
+    user = create(:user)
+
+    create_list(:note, 50) do |note|
+      create(:note_comment, :note => note, :author => user)
+    end
+
+    get :mine, :params => { :display_name => user.display_name }
+    assert_response :success
+    assert_select "table.note_list tr", :count => 11
+
+    get :mine, :params => { :display_name => user.display_name, :page => 2 }
+    assert_response :success
+    assert_select "table.note_list tr", :count => 11
+  end
 end

diff --git a/test/controllers/trace_controller_test.rb b/test/controllers/trace_controller_test.rb
index 1e7642e3629b73b730675dd1bd23ccbe69bc5c0b..e53809840b0aa53c8b9c4bdfe27fe0ff8fc952d6 100644 (file)
--- a/test/controllers/trace_controller_test.rb
+++ b/test/controllers/trace_controller_test.rb
@@ -257,6 +257,26 @@ class TraceControllerTest < ActionController::TestCase
     assert_template "user/no_such_user"
   end
 
+  # Check a multi-page list
+  def test_list_paged
+    # Create several pages worth of traces
+    create_list(:trace, 50)
+
+    # Try and get the list
+    get :list
+    assert_response :success
+    assert_select "table#trace_list tbody", :count => 1 do
+      assert_select "tr", :count => 20
+    end
+
+    # Try and get the second page
+    get :list, :params => { :page => 2 }
+    assert_response :success
+    assert_select "table#trace_list tbody", :count => 1 do
+      assert_select "tr", :count => 20
+    end
+  end
+
   # Check that the rss loads
   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 7be619c2ae73548cfbccfdd068eba2c309f42bd3..e5b0bdb3208468a769eaa0f21a5fe213fdbfaf19 100644 (file)
--- a/test/controllers/user_blocks_controller_test.rb
+++ b/test/controllers/user_blocks_controller_test.rb
@@ -73,6 +73,24 @@ class UserBlocksControllerTest < ActionController::TestCase
     end
   end
 
+  ##
+  # test the index action with multiple pages
+  def test_index_paged
+    create_list(:user_block, 50)
+
+    get :index
+    assert_response :success
+    assert_select "table#block_list", :count => 1 do
+      assert_select "tr", :count => 21
+    end
+
+    get :index, :params => { :page => 2 }
+    assert_response :success
+    assert_select "table#block_list", :count => 1 do
+      assert_select "tr", :count => 21
+    end
+  end
+
   ##
   # test the show action
   def test_show
@@ -421,6 +439,25 @@ class UserBlocksControllerTest < ActionController::TestCase
     end
   end
 
+  ##
+  # test the blocks_on action with multiple pages
+  def test_blocks_on_paged
+    user = create(:user)
+    create_list(:user_block, 50, :user => user)
+
+    get :blocks_on, :params => { :display_name => user.display_name }
+    assert_response :success
+    assert_select "table#block_list", :count => 1 do
+      assert_select "tr", :count => 21
+    end
+
+    get :blocks_on, :params => { :display_name => user.display_name, :page => 2 }
+    assert_response :success
+    assert_select "table#block_list", :count => 1 do
+      assert_select "tr", :count => 21
+    end
+  end
+
   ##
   # test the blocks_by action
   def test_blocks_by
@@ -465,4 +502,23 @@ class UserBlocksControllerTest < ActionController::TestCase
     assert_select "table#block_list", false
     assert_select "p", "#{normal_user.display_name} has not made any blocks yet."
   end
+
+  ##
+  # test the blocks_by action with multiple pages
+  def test_blocks_by_paged
+    user = create(:moderator_user)
+    create_list(:user_block, 50, :creator => user)
+
+    get :blocks_by, :params => { :display_name => user.display_name }
+    assert_response :success
+    assert_select "table#block_list", :count => 1 do
+      assert_select "tr", :count => 21
+    end
+
+    get :blocks_by, :params => { :display_name => user.display_name, :page => 2 }
+    assert_response :success
+    assert_select "table#block_list", :count => 1 do
+      assert_select "tr", :count => 21
+    end
+  end
 end