From b7f306a437f1b0d6960cdafb348a5c15366ec53e Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Sun, 10 May 2009 00:33:55 +0000
Subject: [PATCH 1/1] Fixed problem where tag lengths were generating a 422
 error with no message. They now generate a 400 error with a meaningful
 message.

---
 app/models/node.rb                           |  5 ++++
 app/models/relation.rb                       |  5 ++++
 app/models/way.rb                            |  5 ++++
 test/functional/changeset_controller_test.rb | 27 +++++++++++++++++++-
 test/functional/node_controller_test.rb      |  6 +++++
 test/functional/way_controller_test.rb       | 10 ++++++++
 6 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/app/models/node.rb b/app/models/node.rb
index e37a10250..742609c0e 100644
--- 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 7be027559..24b139f0d 100644
--- 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 40a024b8b..92d8f735a 100644
--- 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 bc1fdb835..458a5adf1 100644
--- 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 396a66710..3bebace8d 100644
--- 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 de23545f7..862e700a6 100644
--- 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
 
   # -------------------------------------
-- 
2.43.2