Fixed problem where tag lengths were generating a 422 error with no message. They...
authorMatt Amos <zerebubuth@gmail.com>
Sun, 10 May 2009 00:33:55 +0000 (00:33 +0000)
committerMatt Amos <zerebubuth@gmail.com>
Sun, 10 May 2009 00:33:55 +0000 (00:33 +0000)
app/models/node.rb
app/models/relation.rb
app/models/way.rb
test/functional/changeset_controller_test.rb
test/functional/node_controller_test.rb
test/functional/way_controller_test.rb

index e37a102..742609c 100644 (file)
@@ -254,6 +254,11 @@ class Node < ActiveRecord::Base
     # in the hash to be overwritten.
     raise OSM::APIDuplicateTagsError.new("node", self.id, k) if @tags.include? k
 
+    # check tag size here, as we don't create a NodeTag object until
+    # just before we save...
+    raise OSM::APIBadUserInput.new("Node #{self.id} has a tag with too long a key, '#{k}'.") if k.length > 255
+    raise OSM::APIBadUserInput.new("Node #{self.id} has a tag with too long a value, '#{k}'='#{v}'.") if v.length > 255
+
     @tags[k] = v
   end
 
index 7be0275..24b139f 100644 (file)
@@ -218,6 +218,11 @@ class Relation < ActiveRecord::Base
     # in the hash to be overwritten.
     raise OSM::APIDuplicateTagsError.new("relation", self.id, k) if @tags.include? k
 
+    # check tag size here, as we don't create a RelationTag object until
+    # just before we save...
+    raise OSM::APIBadUserInput.new("Relation #{self.id} has a tag with too long a key, '#{k}'.") if k.length > 255
+    raise OSM::APIBadUserInput.new("Relation #{self.id} has a tag with too long a value, '#{k}'='#{v}'.") if v.length > 255
+
     @tags[k] = v
   end
 
index 40a024b..92d8f73 100644 (file)
@@ -191,6 +191,11 @@ class Way < ActiveRecord::Base
     # in the hash to be overwritten.
     raise OSM::APIDuplicateTagsError.new("way", self.id, k) if @tags.include? k
 
+    # check tag size here, as we don't create a WayTag object until
+    # just before we save...
+    raise OSM::APIBadUserInput.new("Way #{self.id} has a tag with too long a key, '#{k}'.") if k.length > 255
+    raise OSM::APIBadUserInput.new("Way #{self.id} has a tag with too long a value, '#{k}'='#{v}'.") if v.length > 255
+
     @tags[k] = v
   end
 
index bc1fdb8..458a5ad 100644 (file)
@@ -82,7 +82,7 @@ class ChangesetControllerTest < ActionController::TestCase
     post :create
     assert_response :method_not_allowed
   end
-    
+
   ##
   # check that the changeset can be read and returns the correct
   # document structure.
@@ -446,6 +446,31 @@ EOF
     assert_equal true, Relation.find(current_relations(:visible_relation).id).visible
   end
 
+  ##
+  # upload an element with a really long tag value
+  def test_upload_invalid_too_long_tag
+    basic_authorization users(:public_user).email, "test"
+    cs_id = changesets(:public_user_first_change).id
+
+    # simple diff to create a node way and relation using placeholders
+    diff = <<EOF
+<osmChange>
+ <create>
+  <node id='-1' lon='0' lat='0' changeset='#{cs_id}'>
+   <tag k='foo' v='#{"x"*256}'/>
+  </node>
+ </create>
+</osmChange>
+EOF
+
+    # upload it
+    content diff
+    post :upload, :id => cs_id
+    assert_response :bad_request, 
+      "shoudln't be able to upload too long a tag to changeset: #{@response.body}"
+
+  end
+    
   ##
   # upload something which creates new objects and inserts them into
   # existing containers using placeholders.
index 396a667..3bebace 100644 (file)
@@ -91,6 +91,12 @@ class NodeControllerTest < ActionController::TestCase
     assert_response :bad_request, "node upload did not return bad_request status"
     assert_equal "Cannot parse valid node from xml string <node lat=\"3.434\" changeset=\"#{changeset.id}\"/>. lon missing", @response.body
 
+    # test that the upload is rejected when we have a tag which is too long
+    content("<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'><tag k='foo' v='#{'x'*256}'/></node></osm>")
+    put :create
+    assert_response :bad_request, "node upload did not return bad_request status"
+    assert_equal "Node  has a tag with too long a value, 'foo'='#{'x'*256}'.", @response.body
+
   end
 
   def test_read
index de23545..862e700 100644 (file)
@@ -182,6 +182,16 @@ class WayControllerTest < ActionController::TestCase
     # expect failure
     assert_response :conflict, 
         "way upload to closed changeset did not return 'conflict'"    
+
+    # create a way with a tag which is too long
+    content "<osm><way changeset='#{open_changeset_id}'>" +
+      "<nd ref='#{nid1}'/>" +
+      "<tag k='foo' v='#{'x'*256}'/>" +
+      "</way></osm>"
+    put :create
+    # expect failure
+    assert_response :bad_request, 
+        "way upload to with too long tag did not return 'bad_request'"
   end
 
   # -------------------------------------