From 2efd73c672dae8f6956024638b4b090961e74781 Mon Sep 17 00:00:00 2001 From: mmd-osm Date: Sat, 29 Jan 2022 15:52:21 +0100 Subject: [PATCH] Introduce relation member limit Adds a new parameter `max_number_of_relation_members` in settings.yml --- app/models/relation.rb | 2 ++ app/views/api/capabilities/show.builder | 1 + config/initializers/config.rb | 1 + config/settings.yml | 2 ++ lib/osm.rb | 18 +++++++++++++++ .../api/capabilities_controller_test.rb | 1 + test/models/relation_test.rb | 23 +++++++++++++++++++ 7 files changed, 48 insertions(+) diff --git a/app/models/relation.rb b/app/models/relation.rb index a231feddb..4200a08dd 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -206,6 +206,8 @@ class Relation < ApplicationRecord end def preconditions_ok?(good_members = []) + raise OSM::APITooManyRelationMembersError.new(id, members.length, Settings.max_number_of_relation_members) if members.length > Settings.max_number_of_relation_members + # These are hastables that store an id in the index of all # the nodes/way/relations that have already been added. # If the member is valid and visible then we add it to the diff --git a/app/views/api/capabilities/show.builder b/app/views/api/capabilities/show.builder index 682373898..b6a38723d 100644 --- a/app/views/api/capabilities/show.builder +++ b/app/views/api/capabilities/show.builder @@ -6,6 +6,7 @@ xml.osm(OSM::API.new.xml_root_attributes) do |osm| api.note_area(:maximum => Settings.max_note_request_area) api.tracepoints(:per_page => Settings.tracepoints_per_page) api.waynodes(:maximum => Settings.max_number_of_way_nodes) + api.relationmembers(:maximum => Settings.max_number_of_relation_members) api.changesets(:maximum_elements => Changeset::MAX_ELEMENTS) api.timeout(:seconds => Settings.api_timeout) api.status(:database => @database_status, diff --git a/config/initializers/config.rb b/config/initializers/config.rb index d0f8c26fc..e51feecec 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -74,6 +74,7 @@ Config.setup do |config| required(:max_note_request_area).filled(:number?) required(:tracepoints_per_page).filled(:int?) required(:max_number_of_way_nodes).filled(:int?) + required(:max_number_of_relation_members).filled(:int?) required(:api_timeout).filled(:int?) required(:imagery_blacklist).maybe(:array?) required(:status).filled(:str?, :included_in? => ALLOWED_STATUS) diff --git a/config/settings.yml b/config/settings.yml index 801e8f2d1..42a911057 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -31,6 +31,8 @@ tracepoints_per_page: 5000 max_number_of_nodes: 50000 # Maximum number of nodes that can be in a way (checked on save) max_number_of_way_nodes: 2000 +# Maximum number of members that can be in a relation (checked on save) +max_number_of_relation_members: 32000 # The maximum area you're allowed to request notes from, in square degrees max_note_request_area: 25 # Zoom level to use for postcode results from the geocoder diff --git a/lib/osm.rb b/lib/osm.rb index 005d3ebb8..ee0b8d903 100644 --- a/lib/osm.rb +++ b/lib/osm.rb @@ -237,6 +237,24 @@ module OSM end end + # Raised when a relation has more than the configured number of relation members. + # This prevents relations from being too complex and difficult to work with + class APITooManyRelationMembersError < APIError + def initialize(id, provided, max) + super "You tried to add #{provided} members to relation #{id}, however only #{max} are allowed" + + @id = id + @provided = provided + @max = max + end + + attr_reader :id, :provided, :max + + def status + :bad_request + end + end + ## # raised when user input couldn't be parsed class APIBadUserInput < APIError diff --git a/test/controllers/api/capabilities_controller_test.rb b/test/controllers/api/capabilities_controller_test.rb index 1cae8f02c..9d6de4a6f 100644 --- a/test/controllers/api/capabilities_controller_test.rb +++ b/test/controllers/api/capabilities_controller_test.rb @@ -25,6 +25,7 @@ module Api assert_select "note_area[maximum='#{Settings.max_note_request_area}']", :count => 1 assert_select "tracepoints[per_page='#{Settings.tracepoints_per_page}']", :count => 1 assert_select "changesets[maximum_elements='#{Changeset::MAX_ELEMENTS}']", :count => 1 + assert_select "relationmembers[maximum='#{Settings.max_number_of_relation_members}']", :count => 1 assert_select "status[database='online']", :count => 1 assert_select "status[api='online']", :count => 1 assert_select "status[gpx='online']", :count => 1 diff --git a/test/models/relation_test.rb b/test/models/relation_test.rb index 2aaaaed8a..193126df1 100644 --- a/test/models/relation_test.rb +++ b/test/models/relation_test.rb @@ -230,4 +230,27 @@ class RelationTest < ActiveSupport::TestCase assert_equal 39, changeset.min_lat assert_equal 116, changeset.max_lat end + + # Check that the preconditions fail when you are over the defined limit of + # the maximum number of members in a relation. + def test_max_members_per_relation_limit + # Speed up unit test by using a small relation member limit + default_limit = Settings.max_number_of_relation_members + Settings.max_number_of_relation_members = 20 + + user = create(:user) + changeset = create(:changeset, :user => user) + relation = create(:relation, :changeset => changeset) + node = create(:node, :longitude => 116, :latitude => 39) + # Create relation which exceeds the relation member limit by one + 0.upto(Settings.max_number_of_relation_members) do |i| + create(:relation_member, :relation => relation, :member_type => "Node", :member_id => node.id, :sequence_id => i) + end + + assert_raise OSM::APITooManyRelationMembersError do + relation.create_with_history user + end + + Settings.max_number_of_relation_members = default_limit + end end -- 2.39.5