Fixed problem where tag lengths were generating a 422 error with no message. They...

diff --git a/app/models/node.rb b/app/models/node.rb
index e37a10250d06e07ea4310bfa7c95d5a28997ccbc..742609c0e1d81e16cad88383e0f7757c21f194ad 100644 (file)
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -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
 

diff --git a/app/models/relation.rb b/app/models/relation.rb
index 7be027559883c1e0f42a8f2b9f7c5a0ee5d49225..24b139f0ddd33dbe34ebe85634fabf0904bc48b5 100644 (file)
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -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
 

diff --git a/app/models/way.rb b/app/models/way.rb
index 40a024b8bb2cd87853dee1ab2133c59328b40976..92d8f735a6e83cebb0561d50c7000955d7201c4f 100644 (file)
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -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
 

diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb
index bc1fdb835403bb001f4e16a626131810ad9726da..458a5adf17c15182f5031d6981197c9e275c0f24 100644 (file)
--- a/test/functional/changeset_controller_test.rb
+++ b/test/functional/changeset_controller_test.rb
@@ -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.

diff --git a/test/functional/node_controller_test.rb b/test/functional/node_controller_test.rb
index 396a667107ab2ce661792302b761fbf37aec5c95..3bebace8d692e1289c04cc4e0ead0afd8b701490 100644 (file)
--- a/test/functional/node_controller_test.rb
+++ b/test/functional/node_controller_test.rb
@@ -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

diff --git a/test/functional/way_controller_test.rb b/test/functional/way_controller_test.rb
index de23545f7d60b922e96a4ebff0986f3c00e6ed3a..862e700a68c85fa84eba541fc4b07aca1c076fac 100644 (file)
--- a/test/functional/way_controller_test.rb
+++ b/test/functional/way_controller_test.rb
@@ -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
 
   # -------------------------------------