From 71209a069e76d768f2eaec819bd338b2ab06cc89 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 23 Sep 2026 15:43:59 +0100 Subject: [PATCH] Validate the bbox parameter when listing changesets An invalid bbox raised OSM::APIBadUserInput, which isn't handled by web controllers and so resulted in a 500 error. Validate it with param! instead so that it gets a bad request response. --- app/controllers/changesets_controller.rb | 5 +++-- test/controllers/changesets_controller_test.rb | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb index a42ad78bf..69b39ce0d 100644 --- a/app/controllers/changesets_controller.rb +++ b/app/controllers/changesets_controller.rb @@ -24,6 +24,9 @@ class ChangesetsController < ApplicationController def index param! :before, Integer, :min => 1 param! :after, Integer, :min => 1 + param! :bbox, String, :custom => lambda { |bbox| + raise RailsParam::InvalidParameterError, "bbox must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox.count(",") == 3 + } @params = params.permit(:display_name, :bbox, :friends, :nearby, :before, :after, :list) @@ -59,8 +62,6 @@ class ChangesetsController < ApplicationController end elsif @params[:bbox] bbox_array = @params[:bbox].split(",").map(&:to_f) - raise OSM::APIBadUserInput, "The parameter bbox must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array.count == 4 - changesets = conditions_bbox(changesets, *bbox_array) elsif @params[:friends] && current_user changesets = changesets.where(:user => current_user.followings.identifiable) diff --git a/test/controllers/changesets_controller_test.rb b/test/controllers/changesets_controller_test.rb index 41c605391..bb03b69b2 100644 --- a/test/controllers/changesets_controller_test.rb +++ b/test/controllers/changesets_controller_test.rb @@ -119,6 +119,17 @@ class ChangesetsControllerTest < ActionDispatch::IntegrationTest check_index_result(changesets) end + def test_index_bbox_invalid + get history_path(:format => "html", :bbox => "4.5,4.5,5.5", :list => "1"), :xhr => true + assert_redirected_to :controller => :errors, :action => :bad_request + + get history_feed_path(:bbox => "4.5,4.5,5.5") + assert_response :bad_request + + get history_feed_path(:bbox => "") + assert_response :bad_request + end + def test_index_bbox_across_antimeridian_with_changesets_close_to_antimeridian west_of_antimeridian_changeset = create(:changeset, :num_changes => 1, :bbox => [176, 0, 178, 1]) east_of_antimeridian_changeset = create(:changeset, :num_changes => 1, :bbox => [-178, 0, -176, 1]) -- 2.47.3