From 4826032d183932312d635193c9d3acef6107f280 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Mon, 27 Jul 2009 16:54:00 +0000 Subject: [PATCH 1/1] Added methods to strip those non-XML control characters from tags in AMF controller and give an error if there's invalid UTF-8. --- app/controllers/amf_controller.rb | 31 ++++++++++++++ lib/utf8.rb | 14 ++++++ lib/validators.rb | 14 +----- test/functional/amf_controller_test.rb | 59 ++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 lib/utf8.rb diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 2aee84d0c..20053644d 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -526,6 +526,8 @@ class AmfController < ApplicationController amf_handle_error("'putrelation' #{relid}") do user = getuser(usertoken) if !user then return -1,"You are not logged in, so the relation could not be saved." end + if !tags_ok(tags) then return -1,"One of the tags is invalid. Please pester Adobe to fix Flash on Linux." end + tags = strip_non_xml_chars tags relid = relid.to_i visible = (visible.to_i != 0) @@ -612,6 +614,8 @@ class AmfController < ApplicationController user = getuser(usertoken) if !user then return -1,"You are not logged in, so the way could not be saved." end if pointlist.length < 2 then return -2,"Server error - way is only #{points.length} points long." end + if !tags_ok(tags) then return -1,"One of the tags is invalid. Please pester Adobe to fix Flash on Linux." end + tags = strip_non_xml_chars tags originalway = originalway.to_i pointlist.collect! {|a| a.to_i } @@ -708,6 +712,8 @@ class AmfController < ApplicationController amf_handle_error("'putpoi' #{id}") do user = getuser(usertoken) if !user then return -1,"You are not logged in, so the point could not be saved." end + if !tags_ok(tags) then return -1,"One of the tags is invalid. Please pester Adobe to fix Flash on Linux." end + tags = strip_non_xml_chars tags id = id.to_i visible = (visible.to_i == 1) @@ -862,6 +868,31 @@ class AmfController < ApplicationController def getlocales Dir.glob("#{RAILS_ROOT}/config/potlatch/localised/*").collect { |f| File.basename(f) } end + + ## + # check that all key-value pairs are valid UTF-8. + def tags_ok(tags) + tags.each do |k, v| + return false unless UTF8.valid? k + return false unless UTF8.valid? v + end + return true + end + + ## + # strip characters which are invalid in XML documents from the strings + # in the +tags+ hash. + def strip_non_xml_chars(tags) + new_tags = Hash.new + unless tags.nil? + tags.each do |k, v| + new_k = k.delete "\000-\037", "^\011\012\015" + new_v = v.delete "\000-\037", "^\011\012\015" + new_tags[new_k] = new_v + end + end + return new_tags + end # ==================================================================== # Alternative SQL queries for getway/whichways diff --git a/lib/utf8.rb b/lib/utf8.rb new file mode 100644 index 000000000..5f0d219ba --- /dev/null +++ b/lib/utf8.rb @@ -0,0 +1,14 @@ +module UTF8 + ## + # Checks that a string is valid UTF-8 by trying to convert it to UTF-8 + # using the iconv library, which is in the standard library. + def self.valid?(str) + return true if str.nil? + Iconv.conv("UTF-8", "UTF-8", str) + return true + + rescue + return false + end +end + diff --git a/lib/validators.rb b/lib/validators.rb index 095fb7af9..640a49564 100644 --- a/lib/validators.rb +++ b/lib/validators.rb @@ -11,22 +11,10 @@ module ActiveRecord # is a valid UTF-8 format string. def validates_as_utf8(*attrs) validates_each(attrs) do |record, attr, value| - record.errors.add(attr, @@invalid_utf8_message) unless valid_utf8? value + record.errors.add(attr, @@invalid_utf8_message) unless UTF8.valid? value end end - - ## - # Checks that a string is valid UTF-8 by trying to convert it to UTF-8 - # using the iconv library, which is in the standard library. - def valid_utf8?(str) - return true if str.nil? - Iconv.conv("UTF-8", "UTF-8", str) - return true - rescue - return false - end - end end end diff --git a/test/functional/amf_controller_test.rb b/test/functional/amf_controller_test.rb index ab0f5d5ad..9193cb4d2 100644 --- a/test/functional/amf_controller_test.rb +++ b/test/functional/amf_controller_test.rb @@ -454,6 +454,65 @@ class AmfControllerTest < ActionController::TestCase assert_equal result[4], first_historic_node.version, "The version returned, is different to the one returned by the amf" end + # try creating a POI with rubbish in the tags + def test_putpoi_create_with_control_chars + # This node has no tags + nd = Node.new + # create a node with random lat/lon + lat = rand(100)-50 + rand + lon = rand(100)-50 + rand + # normal user has a changeset open + changeset = changesets(:public_user_first_change) + + mostly_invalid = 32.times.to_a.map {|i| i.chr}.join + tags = { "something" => "foo#{mostly_invalid}bar" } + + amf_content "putpoi", "/1", ["test@example.com:test", changeset.id, nil, nil, lon, lat, tags, nil] + post :amf_write + assert_response :success + amf_parse_response + result = amf_result("/1") + + # check the array returned by the amf + assert_equal 5, result.size + assert_equal 0, result[0], "Expected to get the status ok in the amf" + assert_equal 0, result[2], "The old id should be 0" + assert result[3] > 0, "The new id should be greater than 0" + assert_equal 1, result[4], "The new version should be 1" + + # Finally check that the node that was saved has saved the data correctly + # in both the current and history tables + # First check the current table + current_node = Node.find(result[3]) + assert_equal 1, current_node.tags.size, "There seems to be a tag that has been added to the node" + assert_equal({ "something" => "foo\t\n\rbar" }, current_node.tags, "tags were not fixed correctly") + assert_equal result[4], current_node.version, "The version returned, is different to the one returned by the amf" + end + + # try creating a POI with rubbish in the tags + def test_putpoi_create_with_invalid_utf8 + # This node has no tags + nd = Node.new + # create a node with random lat/lon + lat = rand(100)-50 + rand + lon = rand(100)-50 + rand + # normal user has a changeset open + changeset = changesets(:public_user_first_change) + + invalid = "\xc0\xc0" + tags = { "something" => "foo#{invalid}bar" } + + amf_content "putpoi", "/1", ["test@example.com:test", changeset.id, nil, nil, lon, lat, tags, nil] + post :amf_write + assert_response :success + amf_parse_response + result = amf_result("/1") + + assert_equal 2, result.size + assert_equal -1, result[0], "Expected to get the status FAIL in the amf" + assert_equal "One of the tags is invalid. Please pester Adobe to fix Flash on Linux.", result[1] + end + def test_putpoi_delete_valid end -- 2.39.5