]> git.openstreetmap.org Git - rails.git/commitdiff
Introduce relation member limit
authormmd-osm <mmd.osm@gmail.com>
Sat, 29 Jan 2022 14:52:21 +0000 (15:52 +0100)
committermmd-osm <mmd.osm@gmail.com>
Wed, 2 Feb 2022 12:15:40 +0000 (13:15 +0100)
Adds a new parameter `max_number_of_relation_members` in settings.yml

app/models/relation.rb
app/views/api/capabilities/show.builder
config/initializers/config.rb
config/settings.yml
lib/osm.rb
test/controllers/api/capabilities_controller_test.rb
test/models/relation_test.rb

index a231feddb7fe45427ab5039a1b453e6adce393f6..4200a08dd7c709da827e405759c7d5df968a5672 100644 (file)
@@ -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
index 682373898be4398a2402a7668b620e12a946aa0a..b6a38723db2d37870c702d6d0119d5313df94c40 100644 (file)
@@ -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,
index d0f8c26fc31b7f5aef6877959006d3ea9b3c9125..e51feececc645c56ed37019298d90aecde7d6b8b 100644 (file)
@@ -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)
index 801e8f2d1328045a0629870cd385aaf4afbc526a..42a911057c9f864691ec9ee719987a64575d4db0 100644 (file)
@@ -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
index 005d3ebb848fcd5c109fac687872b2a6d704ba76..ee0b8d9030ee719c769cdb0039edef17d8631e01 100644 (file)
@@ -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
index 1cae8f02cf134fa1cb3452ac8c86a80c533ae22e..9d6de4a6fafbc7ec6eb35a1adbd0700ecabf2b34 100644 (file)
@@ -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
index 2aaaaed8aedd22ddb512e11e52956eaea51630cd..193126df14408e69a0ec3666eab10b8c2d4b5165 100644 (file)
@@ -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