From 6c2093b29d74b145b85e61c4fff09b5f13afa0e5 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sat, 22 Sep 2018 17:12:29 +0100 Subject: [PATCH] Fix new rubocop warnings --- .rubocop.yml | 3 +++ app/controllers/amf_controller.rb | 6 ++++++ app/controllers/changeset_controller.rb | 1 + app/controllers/node_controller.rb | 2 ++ app/controllers/user_controller.rb | 1 + app/models/changeset.rb | 1 + app/models/client_application.rb | 1 + app/models/node.rb | 5 +++++ app/models/oauth_nonce.rb | 2 ++ app/models/relation.rb | 9 +++++++++ app/models/request_token.rb | 1 + app/models/trace.rb | 3 +++ app/models/way.rb | 6 ++++++ config/initializers/i18n.rb | 1 + config/initializers/oauth.rb | 1 + lib/bounding_box.rb | 2 ++ lib/classic_pagination/pagination.rb | 2 ++ lib/geo_record.rb | 1 + lib/potlatch.rb | 1 + lib/utf8.rb | 1 + script/gravatar | 1 + 21 files changed, 51 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index aacaf2d53..31c14773c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -30,6 +30,9 @@ Rails/ApplicationRecord: Rails/CreateTableWithTimestamps: Enabled: false +Rails/FindEach: + Enabled: false + Rails/HasManyOrHasOneDependent: Enabled: false diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 07e7669b1..8fbd8dea9 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -143,6 +143,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 @@ -497,6 +498,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 @@ -533,6 +535,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 @@ -622,6 +625,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 +655,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") @@ -728,6 +733,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 diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 79192754b..90b868129 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -481,6 +481,7 @@ class ChangesetController < ApplicationController u = if name.nil? # user input checking, we don't have any UIDs < 1 raise OSM::APIBadUserInput, "invalid user ID" if user.to_i < 1 + u = User.find(user.to_i) else u = User.find_by(:display_name => name) diff --git a/app/controllers/node_controller.rb b/app/controllers/node_controller.rb index 20baf6bb4..84b814a34 100644 --- a/app/controllers/node_controller.rb +++ b/app/controllers/node_controller.rb @@ -54,6 +54,7 @@ class NodeController < ApplicationController new_node = Node.from_xml(request.raw_post) raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id + node.delete_with_history!(new_node, current_user) render :plain => node.version.to_s end @@ -65,6 +66,7 @@ class NodeController < ApplicationController ids = params["nodes"].split(",").collect(&:to_i) raise OSM::APIBadUserInput, "No nodes were given to search for" if ids.empty? + doc = OSM::API.new.get_xml_doc Node.find(ids).each do |node| diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 5aaa9d72b..be261e8c8 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -826,6 +826,7 @@ class UserController < ApplicationController def gravatar_enable(user) # code from example https://en.gravatar.com/site/implement/images/ruby/ return false if user.image.present? + hash = Digest::MD5.hexdigest(user.email.downcase) url = "https://www.gravatar.com/avatar/#{hash}?d=404" # without d=404 we will always get an image back response = OSM.http_client.get(URI.parse(url)) diff --git a/app/models/changeset.rb b/app/models/changeset.rb index 1aa1e1fd3..4d305ea44 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -106,6 +106,7 @@ class Changeset < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing value") if tag["v"].nil? + cs.add_tag_keyval(tag["k"], tag["v"]) end diff --git a/app/models/client_application.rb b/app/models/client_application.rb index ef1a0110e..1d5922d8e 100644 --- a/app/models/client_application.rb +++ b/app/models/client_application.rb @@ -57,6 +57,7 @@ class ClientApplication < ActiveRecord::Base def self.verify_request(request, options = {}, &block) signature = OAuth::Signature.build(request, options, &block) return false unless OauthNonce.remember(signature.request.nonce, signature.request.timestamp) + value = signature.verify value rescue OAuth::Signature::UnknownSignatureMethod diff --git a/app/models/node.rb b/app/models/node.rb index c09fcbd67..989cdee5c 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -88,19 +88,23 @@ class Node < ActiveRecord::Base raise OSM::APIBadXMLError.new("node", pt, "lat missing") if pt["lat"].nil? raise OSM::APIBadXMLError.new("node", pt, "lon missing") if pt["lon"].nil? + node.lat = OSM.parse_float(pt["lat"], OSM::APIBadXMLError, "node", pt, "lat not a number") node.lon = OSM.parse_float(pt["lon"], OSM::APIBadXMLError, "node", pt, "lon not a number") raise OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt["changeset"].nil? + node.changeset_id = pt["changeset"].to_i raise OSM::APIBadUserInput, "The node is outside this world" unless node.in_world? # version must be present unless creating raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create || !pt["version"].nil? + node.version = create ? 0 : pt["version"].to_i unless create raise OSM::APIBadXMLError.new("node", pt, "ID is required when updating.") if pt["id"].nil? + node.id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -119,6 +123,7 @@ class Node < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("node", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("node", pt, "tag is missing value") if tag["v"].nil? + node.add_tag_key_val(tag["k"], tag["v"]) end diff --git a/app/models/oauth_nonce.rb b/app/models/oauth_nonce.rb index 0952f068e..9d2773e8f 100644 --- a/app/models/oauth_nonce.rb +++ b/app/models/oauth_nonce.rb @@ -22,8 +22,10 @@ class OauthNonce < ActiveRecord::Base # Remembers a nonce and it's associated timestamp. It returns false if it has already been used def self.remember(nonce, timestamp) return false if Time.now.to_i - timestamp.to_i > 86400 + oauth_nonce = OauthNonce.create(:nonce => nonce, :timestamp => timestamp.to_i) return false if oauth_nonce.new_record? + oauth_nonce end end diff --git a/app/models/relation.rb b/app/models/relation.rb index 2495830ee..b17a3b604 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -70,12 +70,15 @@ class Relation < ActiveRecord::Base relation = Relation.new raise OSM::APIBadXMLError.new("relation", pt, "Version is required when updating") unless create || !pt["version"].nil? + relation.version = pt["version"] raise OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt["changeset"].nil? + relation.changeset_id = pt["changeset"] unless create raise OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt["id"].nil? + relation.id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -94,6 +97,7 @@ class Relation < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag["v"].nil? + relation.add_tag_keyval(tag["k"], tag["v"]) end @@ -106,6 +110,7 @@ class Relation < ActiveRecord::Base pt.find("member").each do |member| # member_type = raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"] + # member_ref = member['ref'] # member_role member["role"] ||= "" # Allow the upload to not include this, in which case we default to an empty string. @@ -207,6 +212,7 @@ class Relation < ActiveRecord::Base lock! check_consistency(self, new_relation, user) raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid." unless new_relation.preconditions_ok?(members) + self.changeset_id = new_relation.changeset_id self.changeset = new_relation.changeset self.tags = new_relation.tags @@ -219,6 +225,7 @@ class Relation < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid." unless preconditions_ok? + self.version = 0 self.visible = true save_with_history! @@ -254,6 +261,7 @@ class Relation < ActiveRecord::Base # and check that it is OK to use. raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" unless element && element.visible? && element.preconditions_ok? + hash[m[1]] = true end @@ -270,6 +278,7 @@ class Relation < ActiveRecord::Base if old_id < 0 new_id = id_map[type.downcase.to_sym][old_id] raise OSM::APIBadUserInput, "Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}." if new_id.nil? + [type, new_id, role] else [type, id, role] diff --git a/app/models/request_token.rb b/app/models/request_token.rb index 335a735bc..ed0cc3ae4 100644 --- a/app/models/request_token.rb +++ b/app/models/request_token.rb @@ -40,6 +40,7 @@ class RequestToken < OauthToken def authorize!(user) return false if authorized? + self.user = user self.authorized_at = Time.now self.verifier = OAuth::Helper.generate_key(20)[0, 20] unless oauth10? diff --git a/app/models/trace.rb b/app/models/trace.rb index 214b0b647..91492404b 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -214,10 +214,12 @@ class Trace < ActiveRecord::Base def update_from_xml_node(pt, create = false) raise OSM::APIBadXMLError.new("trace", pt, "visibility missing") if pt["visibility"].nil? + self.visibility = pt["visibility"] unless create raise OSM::APIBadXMLError.new("trace", pt, "ID is required when updating.") if pt["id"].nil? + id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -232,6 +234,7 @@ class Trace < ActiveRecord::Base description = pt.find("description").first raise OSM::APIBadXMLError.new("trace", pt, "description missing") if description.nil? + self.description = description.content self.tags = pt.find("tag").collect do |tag| diff --git a/app/models/way.rb b/app/models/way.rb index e5b73ceaa..d6f0011d9 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -68,12 +68,15 @@ class Way < ActiveRecord::Base way = Way.new raise OSM::APIBadXMLError.new("way", pt, "Version is required when updating") unless create || !pt["version"].nil? + way.version = pt["version"] raise OSM::APIBadXMLError.new("way", pt, "Changeset id is missing") if pt["changeset"].nil? + way.changeset_id = pt["changeset"] unless create raise OSM::APIBadXMLError.new("way", pt, "ID is required when updating") if pt["id"].nil? + way.id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -92,6 +95,7 @@ class Way < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("way", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("way", pt, "tag is missing value") if tag["v"].nil? + way.add_tag_keyval(tag["k"], tag["v"]) end @@ -194,6 +198,7 @@ class Way < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid." unless preconditions_ok? + self.version = 0 self.visible = true save_with_history! @@ -252,6 +257,7 @@ class Way < ActiveRecord::Base if node_id < 0 new_id = id_map[:node][node_id] raise OSM::APIBadUserInput, "Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}" if new_id.nil? + new_id else node_id diff --git a/config/initializers/i18n.rb b/config/initializers/i18n.rb index 89ff677fb..b4399a9f8 100644 --- a/config/initializers/i18n.rb +++ b/config/initializers/i18n.rb @@ -5,6 +5,7 @@ module I18n super rescue InvalidPluralizationData => ex raise ex unless ex.entry.key?(:other) + ex.entry[:other] end end diff --git a/config/initializers/oauth.rb b/config/initializers/oauth.rb index 421ca2bf9..122c2e651 100644 --- a/config/initializers/oauth.rb +++ b/config/initializers/oauth.rb @@ -52,6 +52,7 @@ module OpenStreetMap def oauth1_verify(request, options = {}, &block) signature = OAuth::Signature.build(request, options, &block) return false unless OauthNonce.remember(signature.request.nonce, signature.request.timestamp) + value = signature.verify if request.ssl? && !value http_request = request.dup diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb index 1fc5dc696..6582a30c0 100644 --- a/lib/bounding_box.rb +++ b/lib/bounding_box.rb @@ -61,6 +61,7 @@ class BoundingBox # check the bbox is sane raise OSM::APIBadBoundingBox, "The minimum longitude must be less than the maximum longitude, but it wasn't" if min_lon > max_lon raise OSM::APIBadBoundingBox, "The minimum latitude must be less than the maximum latitude, but it wasn't" if min_lat > max_lat + if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT raise OSM::APIBadBoundingBox, "The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," \ " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}" @@ -157,6 +158,7 @@ class BoundingBox def from_bbox_array(bbox_array) raise OSM::APIBadUserInput, "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array + # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and # max_lat within their respective boundaries. min_lon = [[bbox_array[0].to_f, -LON_LIMIT].max, +LON_LIMIT].min diff --git a/lib/classic_pagination/pagination.rb b/lib/classic_pagination/pagination.rb index 328237d52..054b2bb34 100644 --- a/lib/classic_pagination/pagination.rb +++ b/lib/classic_pagination/pagination.rb @@ -316,6 +316,7 @@ module ActionController # same page number). def ==(other) return false if other.nil? + @paginator == other.paginator && @number == other.number end @@ -326,6 +327,7 @@ module ActionController # if the pages do not belong to the same Paginator object. def <=>(other) raise ArgumentError unless @paginator == other.paginator + @number <=> other.number end diff --git a/lib/geo_record.rb b/lib/geo_record.rb index e4a66f932..e02734ec9 100644 --- a/lib/geo_record.rb +++ b/lib/geo_record.rb @@ -29,6 +29,7 @@ module GeoRecord def in_world? return false if lat < -90 || lat > 90 return false if lon < -180 || lon > 180 + true end diff --git a/lib/potlatch.rb b/lib/potlatch.rb index 6fe0b0152..49e1e7a74 100644 --- a/lib/potlatch.rb +++ b/lib/potlatch.rb @@ -38,6 +38,7 @@ module Potlatch arr = {} while (key = getstring(s)) break if key == "" + arr[key] = getvalue(s) end s.getbyte # skip the 9 'end of object' value diff --git a/lib/utf8.rb b/lib/utf8.rb index 7865d6203..613e3005e 100644 --- a/lib/utf8.rb +++ b/lib/utf8.rb @@ -4,6 +4,7 @@ module UTF8 # using the iconv library, which is in the standard library. def self.valid?(str) return true if str.nil? + str.valid_encoding? end end diff --git a/script/gravatar b/script/gravatar index b86b79366..59ecff4f8 100755 --- a/script/gravatar +++ b/script/gravatar @@ -6,6 +6,7 @@ start = 0 User.where("image_use_gravatar AND id >=" + start.to_s).order("id").find_each do |user| p "checked up to id " + user.id.to_s if (user.id % 1000).zero? # just give a rough indication where we are for restarting next if user.image.present? + hash = Digest::MD5.hexdigest(user.email.downcase) url = "https://www.gravatar.com/avatar/#{hash}?d=404" # without d=404 we will always get an image back response = OSM.http_client.get(URI.parse(url)) -- 2.43.2