X-Git-Url: https://git.openstreetmap.org./rails.git/blobdiff_plain/a83030dab7512c4b2848e777f7a7dbff456774b3..74e1d7336ef926d4c3ef986abd8b899a7c4b6704:/app/controllers/amf_controller.rb diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 9f909ea10..fdad432a8 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -41,6 +41,11 @@ class AmfController < ApplicationController skip_before_action :verify_authenticity_token before_action :check_api_writable + # AMF Controller implements its own authentication and authorization checks + # completely independently of the rest of the codebase, so best just to let + # it keep doing its own thing. + skip_authorization_check + # Main AMF handlers: process the raw AMF string (using AMF library) and # calls each action (private method) accordingly. @@ -110,17 +115,17 @@ class AmfController < ApplicationController def amf_handle_error(call, rootobj, rootid) yield rescue OSM::APIAlreadyDeletedError => ex - return [-4, ex.object, ex.object_id] + [-4, ex.object, ex.object_id] rescue OSM::APIVersionMismatchError => ex - return [-3, [rootobj, rootid], [ex.type.downcase, ex.id, ex.latest]] + [-3, [rootobj, rootid], [ex.type.downcase, ex.id, ex.latest]] rescue OSM::APIUserChangesetMismatchError => ex - return [-2, ex.to_s] + [-2, ex.to_s] rescue OSM::APIBadBoundingBox => ex - return [-2, "Sorry - I can't get the map for that area. The server said: #{ex}"] + [-2, "Sorry - I can't get the map for that area. The server said: #{ex}"] rescue OSM::APIError => ex - return [-1, ex.to_s] + [-1, ex.to_s] rescue StandardError => ex - return [-2, "An unusual error happened (in #{call}). The server said: #{ex}"] + [-2, "An unusual error happened (in #{call}). The server said: #{ex}"] end def amf_handle_error_with_timeout(call, rootobj, rootid) @@ -143,6 +148,7 @@ class AmfController < ApplicationController if cstags return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(cstags) + cstags = strip_non_xml_chars cstags end @@ -437,9 +443,9 @@ class AmfController < ApplicationController revdates.collect! { |d| [(d + 1).strftime("%d %b %Y, %H:%M:%S")] + revusers[d.to_i] } revdates.uniq! - return ["way", wayid, revdates] + ["way", wayid, revdates] rescue ActiveRecord::RecordNotFound - return ["way", wayid, []] + ["way", wayid, []] end # Find history of a node. Returns 'node', id, and an array of previous versions as above. @@ -448,9 +454,9 @@ class AmfController < ApplicationController history = Node.find(nodeid).old_nodes.unredacted.reverse.collect do |old_node| [(old_node.timestamp + 1).strftime("%d %b %Y, %H:%M:%S")] + change_user(old_node) end - return ["node", nodeid, history] + ["node", nodeid, history] rescue ActiveRecord::RecordNotFound - return ["node", nodeid, []] + ["node", nodeid, []] end def change_user(obj) @@ -471,7 +477,7 @@ class AmfController < ApplicationController return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists? query = Trace.visible_to(user) - query = if searchterm.to_i > 0 + query = if searchterm.to_i.positive? query.where(:id => searchterm.to_i) else query.where("MATCH(name) AGAINST (?)", searchterm).limit(21) @@ -497,6 +503,7 @@ class AmfController < ApplicationController rel = Relation.where(:id => relid).first return [-4, "relation", relid] if rel.nil? || !rel.visible + [0, "", relid, rel.tags, rel.members, rel.version] end end @@ -506,9 +513,9 @@ class AmfController < ApplicationController def findrelations(searchterm) rels = [] - if searchterm.to_i > 0 + if searchterm.to_i.positive? rel = Relation.where(:id => searchterm.to_i).first - rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel && rel.visible + rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel&.visible else RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t| rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) if t.relation.visible @@ -533,6 +540,7 @@ class AmfController < ApplicationController return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(tags) + tags = strip_non_xml_chars tags relid = relid.to_i @@ -542,7 +550,7 @@ class AmfController < ApplicationController relation = nil Relation.transaction do # create a new relation, or find the existing one - relation = Relation.find(relid) if relid > 0 + relation = Relation.find(relid) if relid.positive? # We always need a new node, based on the data that has been sent to us new_relation = Relation.new @@ -550,7 +558,7 @@ class AmfController < ApplicationController typedmembers = [] members.each do |m| mid = m[1].to_i - if mid < 0 + if mid.negative? mid = renumberednodes[mid] if m[0] == "Node" mid = renumberedways[mid] if m[0] == "Way" end @@ -622,6 +630,7 @@ class AmfController < ApplicationController return -2, "Server error - way is only #{pointlist.length} points long." if pointlist.length < 2 return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(attributes) + attributes = strip_non_xml_chars attributes originalway = originalway.to_i @@ -651,6 +660,7 @@ class AmfController < ApplicationController # fixup node tags in a way as well return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(node.tags) + node.tags = strip_non_xml_chars node.tags node.tags.delete("created_by") @@ -672,7 +682,7 @@ class AmfController < ApplicationController # -- Save revised way pointlist.collect! do |a| - renumberednodes[a] ? renumberednodes[a] : a + renumberednodes[a] || a end new_way = Way.new new_way.tags = attributes @@ -728,6 +738,7 @@ class AmfController < ApplicationController return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(tags) + tags = strip_non_xml_chars tags id = id.to_i @@ -735,7 +746,7 @@ class AmfController < ApplicationController node = nil new_node = nil Node.transaction do - if id > 0 + if id.positive? begin node = Node.find(id) rescue ActiveRecord::RecordNotFound @@ -865,7 +876,7 @@ class AmfController < ApplicationController end def getlocales - @locales ||= Locale.list(Dir.glob(Rails.root.join("config", "potlatch", "locales", "*")).collect { |f| File.basename(f, ".yml") }) + @getlocales ||= Locale.list(Dir.glob(Rails.root.join("config", "potlatch", "locales", "*")).collect { |f| File.basename(f, ".yml") }) end ## @@ -883,12 +894,10 @@ class AmfController < ApplicationController # in the +tags+ hash. def strip_non_xml_chars(tags) new_tags = {} - unless tags.nil? - tags.each do |k, v| - new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015" - new_v = v.delete "\000-\037\ufffe\uffff", "^\011\012\015" - new_tags[new_k] = new_v - end + tags&.each do |k, v| + new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015" + new_v = v.delete "\000-\037\ufffe\uffff", "^\011\012\015" + new_tags[new_k] = new_v end new_tags end @@ -938,7 +947,7 @@ class AmfController < ApplicationController INNER JOIN current_relation_members crm ON crm.id=cr.id INNER JOIN current_nodes cn ON crm.member_id=cn.id AND crm.member_type='Node' WHERE #{OSM.sql_for_area(bbox, 'cn.')} - SQL + SQL unless way_ids.empty? sql += <<-SQL UNION @@ -947,7 +956,7 @@ class AmfController < ApplicationController INNER JOIN current_relation_members crm ON crm.id=cr.id WHERE crm.member_type='Way' AND crm.member_id IN (#{way_ids.join(',')}) - SQL + SQL end ActiveRecord::Base.connection.select_all(sql).collect { |a| [a["relid"].to_i, a["version"].to_i] } end