From ac6a872a48c2663524fb2884724524f11ab671c9 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Mon, 16 Dec 2019 21:23:09 +0000 Subject: [PATCH] Avoid errors when /edit is called on an invalid object --- .rubocop_todo.yml | 1 + app/controllers/site_controller.rb | 44 ++++++----- test/controllers/site_controller_test.rb | 94 +++++++++++++++++++++++- 3 files changed, 117 insertions(+), 22 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 054e10cda..f768dddc6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -31,6 +31,7 @@ Lint/SuppressedException: Exclude: - 'app/controllers/api/amf_controller.rb' - 'app/controllers/users_controller.rb' + - 'app/controllers/site_controller.rb' # Offense count: 701 Metrics/AbcSize: diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index 57ac07501..3a69eed12 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -77,26 +77,30 @@ class SiteController < ApplicationController ) end - if params[:node] - bbox = Node.find(params[:node]).bbox.to_unscaled - @lat = bbox.centre_lat - @lon = bbox.centre_lon - @zoom = 18 - elsif params[:way] - bbox = Way.find(params[:way]).bbox.to_unscaled - @lat = bbox.centre_lat - @lon = bbox.centre_lon - @zoom = 17 - elsif params[:note] - note = Note.find(params[:note]) - @lat = note.lat - @lon = note.lon - @zoom = 17 - elsif params[:gpx] && current_user - trace = Trace.visible_to(current_user).find(params[:gpx]) - @lat = trace.latitude - @lon = trace.longitude - @zoom = 16 + begin + if params[:node] + bbox = Node.visible.find(params[:node]).bbox.to_unscaled + @lat = bbox.centre_lat + @lon = bbox.centre_lon + @zoom = 18 + elsif params[:way] + bbox = Way.visible.find(params[:way]).bbox.to_unscaled + @lat = bbox.centre_lat + @lon = bbox.centre_lon + @zoom = 17 + elsif params[:note] + note = Note.visible.find(params[:note]) + @lat = note.lat + @lon = note.lon + @zoom = 17 + elsif params[:gpx] && current_user + trace = Trace.visible_to(current_user).find(params[:gpx]) + @lat = trace.latitude + @lon = trace.longitude + @zoom = 16 + end + rescue ActiveRecord::RecordNotFound + # don't try and derive a location from a missing/deleted object end end diff --git a/test/controllers/site_controller_test.rb b/test/controllers/site_controller_test.rb index 91fb83b67..a798013db 100644 --- a/test/controllers/site_controller_test.rb +++ b/test/controllers/site_controller_test.rb @@ -259,11 +259,31 @@ class SiteControllerTest < ActionController::TestCase assert_equal 18, assigns(:zoom) end + # Test editing inaccessible nodes + def test_edit_with_inaccessible_nodes + user = create(:user) + deleted_node = create(:node, :lat => 1.0, :lon => 1.0, :visible => false) + + get :edit, :params => { :node => 99999 }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + + get :edit, :params => { :node => deleted_node.id }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + end + # Test editing a specific way def test_edit_with_way user = create(:user) node = create(:node, :lat => 3, :lon => 3) - way = create(:way) + way = create(:way) create(:way_node, :node => node, :way => way) get :edit, :params => { :way => way.id }, :session => { :user => user } @@ -274,6 +294,26 @@ class SiteControllerTest < ActionController::TestCase assert_equal 17, assigns(:zoom) end + # Test editing inaccessible ways + def test_edit_with_inaccessible_ways + user = create(:user) + deleted_way = create(:way, :visible => false) + + get :edit, :params => { :way => 99999 }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + + get :edit, :params => { :way => deleted_way.id }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + end + # Test editing a specific note def test_edit_with_note user = create(:user) @@ -289,10 +329,32 @@ class SiteControllerTest < ActionController::TestCase assert_equal 17, assigns(:zoom) end + # Test editing inaccessible notes + def test_edit_with_inaccessible_notes + user = create(:user) + deleted_note = create(:note, :status => "hidden") do |n| + n.comments.create(:author_id => user.id) + end + + get :edit, :params => { :note => 99999 }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + + get :edit, :params => { :note => deleted_note.id }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + end + # Test editing a specific GPX trace def test_edit_with_gpx user = create(:user) - gpx = create(:trace, :latitude => 1, :longitude => 1) + gpx = create(:trace, :latitude => 1, :longitude => 1) get :edit, :params => { :gpx => gpx.id }, :session => { :user => user } assert_response :success @@ -302,6 +364,34 @@ class SiteControllerTest < ActionController::TestCase assert_equal 16, assigns(:zoom) end + # Test editing inaccessible GPX traces + def test_edit_with_inaccessible_gpxes + user = create(:user) + deleted_gpx = create(:trace, :deleted, :latitude => 1, :longitude => 1) + private_gpx = create(:trace, :latitude => 1, :longitude => 1, :visibility => "private") + + get :edit, :params => { :gpx => 99999 }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + + get :edit, :params => { :gpx => deleted_gpx.id }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + + get :edit, :params => { :gpx => private_gpx.id }, :session => { :user => user } + assert_response :success + assert_template "edit" + assert_nil assigns(:lat) + assert_nil assigns(:lon) + assert_nil assigns(:zoom) + end + # Test the edit page redirects def test_edit_redirect get :edit, :params => { :lat => 4, :lon => 5 } -- 2.43.2