end
end
- # Delete a node. Doesn't actually delete it, but retains its history in a wiki-like way.
- # FIXME remove all the fricking SQL
+ # Delete a node. Doesn't actually delete it, but retains its history
+ # in a wiki-like way. We therefore treat it like an update, so the delete
+ # method returns the new version number.
def delete
begin
node = Node.find(params[:id])
new_node = Node.from_xml(request.raw_post)
- # FIXME we no longer care about the user, (or maybe we want to check
- # that the user of the changeset is the same user as is making this
- # little change?) we really care about the
- # changeset which must be open, and that the version that we have been
- # given is the one that is currently stored in the database
if new_node and new_node.id == node.id
- node.delete_with_history(new_node, @user)
- render :nothing => true
+ node.delete_with_history!(new_node, @user)
+ render :text => node.version.to_s, :content_type => "text/plain"
else
render :nothing => true, :status => :bad_request
end
relation = Relation.find(params[:id])
new_relation = Relation.from_xml(request.raw_post)
if new_relation and new_relation.id == relation.id
- relation.delete_with_history(new_relation, @user)
- render :nothing => true
+ relation.delete_with_history!(new_relation, @user)
+ render :text => relation.version.to_s, :content_type => "text/plain"
else
render :nothing => true, :status => :bad_request
end
begin
way = Way.find(params[:id])
new_way = Way.from_xml(request.raw_post)
- if new_way and new_way.id == way.id
- way.delete_with_history(new_way, @user)
- # if we get here, all is fine, otherwise something will catch below.
- render :nothing => true
+ if new_way and new_way.id == way.id
+ way.delete_with_history!(new_way, @user)
+ render :text => way.version.to_s, :content_type => "text/plain"
else
render :nothing => true, :status => :bad_request
end
end
# Should probably be renamed delete_from to come in line with update
- def delete_with_history(new_node, user)
+ def delete_with_history!(new_node, user)
if self.visible
check_consistency(self, new_node, user)
if WayNode.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_nodes.id", :conditions => [ "current_ways.visible = 1 AND current_way_nodes.node_id = ?", self.id ])
end
end
- def delete_with_history(new_relation, user)
+ def delete_with_history!(new_relation, user)
if self.visible
check_consistency(self, new_relation, user)
- if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = 1 AND member_type='relation' and member_id=?", self.id ])
+ if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = 1 AND member_type='relation' and member_id=? ", self.id ])
raise OSM::APIPreconditionFailedError.new
else
- #self.user_id = user.id
- # FIXME we need to deal with changeset here, which is probably already dealt with
self.changeset_id = new_relation.changeset_id
self.tags = []
self.members = []
if !new_relation.preconditions_ok?
raise OSM::APIPreconditionFailedError.new
end
- # FIXME need to deal with changeset etc
- #self.user_id = user.id
self.changeset_id = new_relation.changeset_id
self.tags = new_relation.tags
self.members = new_relation.members
return true
end
- def delete_with_history(new_way, user)
+ def delete_with_history!(new_way, user)
check_consistency(self, new_way, user)
if self.visible
- # FIXME
- # this should actually delete the relations,
- # not just throw a PreconditionFailed if it's a member of a relation!!
- # WHY?? The editor should decide whether the node is in the relation or not!
-
- # FIXME: this should probably renamed to delete_with_history
if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id",
- :conditions => [ "visible = 1 AND member_type='way' and member_id=?", self.id])
+ :conditions => [ "visible = 1 AND member_type='way' and member_id=? ", self.id])
raise OSM::APIPreconditionFailedError
- # end FIXME
else
self.changeset_id = new_way.changeset_id
self.tags = []
end
end
+ # raised when a two tags have a duplicate key string in an element.
+ # this is now forbidden by the API.
+ class APIDuplicateTagsError < APIError
+ def initialize(type, id, tag_key)
+ @type, @id, @tag_key = type, id, tag_key
+ end
+
+ attr_reader :type, :id, :tag_key
+
+ def render_opts
+ { :text => "Element #{@type}/#{@id} has duplicate tags with key #{@tag_key}.",
+ :status => :bad_request }
+ end
+ end
+
# Helper methods for going to/from mercator and lat/lng.
class Mercator
include Math
delete :delete, :id => current_nodes(:visible_node).id
assert_response :success
+ # valid delete should return the new version number, which should
+ # be greater than the old version number
+ assert @response.body.to_i > current_nodes(:visible_node).version,
+ "delete request should return a new version number for node"
+
# this won't work since the node is already deleted
content(nodes(:invisible_node).to_xml)
delete :delete, :id => current_nodes(:invisible_node).id
# in a way...
content(nodes(:used_node_1).to_xml)
delete :delete, :id => current_nodes(:used_node_1).id
- assert_response :precondition_failed
+ assert_response :precondition_failed,
+ "shouldn't be able to delete a node used in a way (#{@response.body})"
# in a relation...
content(nodes(:node_used_by_relationship).to_xml)
delete :delete, :id => current_nodes(:node_used_by_relationship).id
- assert_response :precondition_failed
+ assert_response :precondition_failed,
+ "shouldn't be able to delete a node used in a relation (#{@response.body})"
end
##
put :update, :id => current_nodes(:visible_node).id
assert_response :bad_request,
"adding duplicate tags to a node should fail with 'bad request'"
- end
+ end
+
+ # test whether string injection is possible
+ def test_string_injection
+ basic_authorization(users(:normal_user).email, "test")
+ changeset_id = changesets(:normal_user_first_change).id
+
+ # try and put something into a string that the API might
+ # use unquoted and therefore allow code injection...
+ content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" +
+ '<tag k="#{@user.inspect}" v="0"/>' +
+ '</node></osm>'
+ put :create
+ assert_response :success
+ nodeid = @response.body
+
+ # find the node in the database
+ checknode = Node.find(nodeid)
+ assert_not_nil checknode, "node not found in data base after upload"
+
+ # and grab it using the api
+ get :read, :id => nodeid
+ assert_response :success
+ apinode = Node.from_xml(@response.body)
+ assert_not_nil apinode, "downloaded node is nil, but shouldn't be"
+
+ # check the tags are not corrupted
+ assert_equal checknode.tags, apinode.tags
+ assert apinode.tags.include?('#{@user.inspect}')
+ end
def basic_authorization(user, pass)
@request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
delete :delete, :id => current_relations(:visible_relation).id
assert_response :bad_request
+ # this won't work because the relation is in-use by another relation
+ content(relations(:used_relation).to_xml)
+ delete :delete, :id => current_relations(:used_relation).id
+ assert_response :precondition_failed,
+ "shouldn't be able to delete a relation used in a relation (#{@response.body})"
+
# this should work when we provide the appropriate payload...
content(relations(:visible_relation).to_xml)
delete :delete, :id => current_relations(:visible_relation).id
assert_response :success
+ # valid delete should return the new version number, which should
+ # be greater than the old version number
+ assert @response.body.to_i > current_relations(:visible_relation).version,
+ "delete request should return a new version number for relation"
+
# this won't work since the relation is already deleted
content(relations(:invisible_relation).to_xml)
delete :delete, :id => current_relations(:invisible_relation).id
assert_response :gone
+ # this works now because the relation which was using this one
+ # has been deleted.
+ content(relations(:used_relation).to_xml)
+ delete :delete, :id => current_relations(:used_relation).id
+ assert_response :success,
+ "should be able to delete a relation used in an old relation (#{@response.body})"
+
# this won't work since the relation never existed
delete :delete, :id => 0
assert_response :not_found
delete :delete, :id => current_ways(:visible_way).id
assert_response :bad_request
- # Now try and get a changeset
- changeset_id = changesets(:normal_user_first_change).id
+ # Now try with a valid changeset
content current_ways(:visible_way).to_xml
delete :delete, :id => current_ways(:visible_way).id
assert_response :success
+ # check the returned value - should be the new version number
+ # valid delete should return the new version number, which should
+ # be greater than the old version number
+ assert @response.body.to_i > current_ways(:visible_way).version,
+ "delete request should return a new version number for way"
+
# this won't work since the way is already deleted
content current_ways(:invisible_way).to_xml
delete :delete, :id => current_ways(:invisible_way).id
assert_response :gone
+ # this shouldn't work as the way is used in a relation
+ content current_ways(:used_way).to_xml
+ delete :delete, :id => current_ways(:used_way).id
+ assert_response :precondition_failed,
+ "shouldn't be able to delete a way used in a relation (#{@response.body})"
+
# this won't work since the way never existed
delete :delete, :id => 0
assert_response :not_found
projects / rails.git / commitdiff