Encode the id as a URL path segment, not a form value

lib/active_resource.rb

@@ -23,6 +23,7 @@
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #++
 
+require "erb"
 require "uri"
 
 require "active_support"

lib/active_resource/base.rb

@@ -883,7 +883,7 @@ def element_path(id, prefix_options = {}, query_options = nil)
         check_prefix_options(prefix_options)
 
         prefix_options, query_options = split_options(prefix_options) if query_options.nil?
-        "#{prefix(prefix_options)}#{collection_name}/#{URI.encode_www_form_component(id.to_s)}#{format_extension}#{query_string(query_options)}"
+        "#{prefix(prefix_options)}#{collection_name}/#{ERB::Util.url_encode(id.to_s)}#{format_extension}#{query_string(query_options)}"
       end
 
       # Gets the element url for the given ID in +id+. If the +query_options+ parameter is omitted, Rails

lib/active_resource/custom_methods.rb

@@ -140,7 +140,7 @@ def delete(method_name, options = {})
 
     private
       def custom_method_element_url(method_name, options = {})
-        "#{self.class.prefix(prefix_options)}#{self.class.collection_name}/#{URI.encode_www_form_component(id.to_s)}/#{method_name}#{self.class.format_extension}#{self.class.__send__(:query_string, options)}"
+        "#{self.class.prefix(prefix_options)}#{self.class.collection_name}/#{ERB::Util.url_encode(id.to_s)}/#{method_name}#{self.class.format_extension}#{self.class.__send__(:query_string, options)}"
       end
 
       def custom_method_new_element_url(method_name, options = {})

test/cases/base/custom_methods_test.rb

@@ -212,6 +212,21 @@ def test_custom_element_method_identifier_encoding
 
     assert_equal ActiveResource::Response.new(luis, 204), Person.find("luís").put(:deactivate)
   end
+
+  def test_custom_element_method_identifier_is_encoded_for_a_path_not_a_form
+    ann = { person: { id: "ann mary", name: "Ann Mary" } }.to_json
+    ann_plus = { person: { id: "ann+mary", name: "Ann Plus Mary" } }.to_json
+
+    ActiveResource::HttpMock.respond_to do |mock|
+      mock.get "/people/ann%20mary.json", {}, ann
+      mock.put "/people/ann%20mary/deactivate.json", {}, ann, 204
+      mock.get "/people/ann%2Bmary.json", {}, ann_plus
+      mock.put "/people/ann%2Bmary/deactivate.json", {}, ann_plus, 204
+    end
+
+    assert_equal ActiveResource::Response.new(ann, 204), Person.find("ann mary").put(:deactivate)
+    assert_equal ActiveResource::Response.new(ann_plus, 204), Person.find("ann+mary").put(:deactivate)
+  end
 end
 
 class SingletonCustomMethodsTest < ActiveSupport::TestCase

test/cases/base_test.rb

@@ -854,7 +854,12 @@ def test_custom_element_path
     assert_equal "/people/1/addresses/1.json", StreetAddress.element_path(1, person_id: 1)
     assert_equal "/people/1/addresses/1.json", StreetAddress.element_path(1, "person_id" => 1)
     assert_equal "/people/Greg/addresses/1.json", StreetAddress.element_path(1, "person_id" => "Greg")
-    assert_equal "/people/ann%20mary/addresses/ann+mary.json", StreetAddress.element_path(:'ann mary', "person_id" => "ann mary")
+    assert_equal "/people/ann%20mary/addresses/ann%20mary.json", StreetAddress.element_path(:'ann mary', "person_id" => "ann mary")
+  end
+
+  def test_id_is_encoded_for_a_path_not_a_form
+    assert_equal "/people/ann%20mary.json", Person.element_path("ann mary")
+    assert_equal "/people/ann%2Bmary.json", Person.element_path("ann+mary")
   end
 
   def test_custom_element_path_without_required_prefix_param

test/cases/finder_test.rb

@@ -352,6 +352,14 @@ def test_find_identifier_encoding
     assert_equal "David", david.name
   end
 
+  def test_find_identifier_encoding_for_space
+    ActiveResource::HttpMock.respond_to { |m| m.get "/people/ann%20mary.json", {}, @david }
+
+    david = Person.find("ann mary")
+
+    assert_equal "David", david.name
+  end
+
   def test_find_identifier_encoding_for_path_traversal
     ActiveResource::HttpMock.respond_to { |m| m.get "/people/..%2F.json", {}, @david }