Use CGI.escape on ID's passed into the api client

lib/restforce/concerns/api.rb

@@ -147,7 +147,7 @@ def describe(sobject = nil)
       def describe_layouts(sobject, layout_id = nil)
         version_guard(28.0) do
           if layout_id
-            api_get("sobjects/#{sobject}/describe/layouts/#{layout_id}").body
+            api_get("sobjects/#{sobject}/describe/layouts/#{CGI.escape(layout_id)}").body
           else
             api_get("sobjects/#{sobject}/describe/layouts").body
           end
@@ -319,7 +319,7 @@ def update!(sobject, attrs)
         id = attrs.fetch(attrs.keys.find { |k, v| k.to_s.casecmp('id').zero? }, nil)
         raise ArgumentError, 'ID field missing from provided attributes' unless id
         attrs_without_id = attrs.reject { |k, v| k.to_s.casecmp("id").zero? }
-        api_patch "sobjects/#{sobject}/#{id}", attrs_without_id
+        api_patch "sobjects/#{sobject}/#{CGI.escape(id)}", attrs_without_id
         true
       end
 
@@ -375,7 +375,7 @@ def upsert!(sobject, field, attrs)
               api_post "sobjects/#{sobject}/#{field}", attrs
             end
           else
-            api_patch "sobjects/#{sobject}/#{field}/#{URI.encode(external_id)}", attrs
+            api_patch "sobjects/#{sobject}/#{field}/#{CGI.escape(external_id)}", attrs
           end
 
         response.body && response.body['id'] ? response.body['id'] : true
@@ -412,7 +412,7 @@ def destroy(*args)
       # Returns true of the sobject was successfully deleted.
       # Raises an exception if an error is returned from Salesforce.
       def destroy!(sobject, id)
-        api_delete "sobjects/#{sobject}/#{id}"
+        api_delete "sobjects/#{sobject}/#{CGI.escape(id)}"
         true
       end
 
@@ -426,9 +426,9 @@ def destroy!(sobject, id)
       # Returns the Restforce::SObject sobject record.
       def find(sobject, id, field = nil)
         url = if field
-                "sobjects/#{sobject}/#{field}/#{URI.encode(id)}"
+                "sobjects/#{sobject}/#{field}/#{CGI.escape(id)}"
               else
-                "sobjects/#{sobject}/#{id}"
+                "sobjects/#{sobject}/#{CGI.escape(id)}"
               end
         api_get(url).body
       end
@@ -444,9 +444,9 @@ def find(sobject, id, field = nil)
       #
       def select(sobject, id, select, field = nil)
         path = if field
-                 "sobjects/#{sobject}/#{field}/#{URI.encode(id)}"
+                 "sobjects/#{sobject}/#{field}/#{CGI.escape(id)}"
                else
-                 "sobjects/#{sobject}/#{id}"
+                 "sobjects/#{sobject}/#{CGI.escape(id)}"
                end
         path << "?fields=#{select.join(',')}" if select && select.any?
 

spec/unit/concerns/api_spec.rb

@@ -316,6 +316,16 @@
         end
       end
 
+      context 'when the id field contains special characters' do
+        let(:attrs) { { id: '1234/?abc', StageName: "Call Scheduled" } }
+
+        it 'sends an HTTP PATCH, and encodes the ID' do
+          client.should_receive(:api_patch).
+            with('sobjects/Whizbang/1234%2F%3Fabc', StageName: "Call Scheduled")
+          expect(result).to be_true
+        end
+      end
+
       context 'when the id field is missing from the attrs' do
         it "raises an error" do
           expect { client.update!(sobject, attrs) }.
@@ -442,6 +452,16 @@
         with('sobjects/Whizbang/1234')
       expect(result).to be_true
     end
+
+    context 'when the id field contains special characters' do
+      let(:id) { '1234/?abc' }
+
+      it 'sends an HTTP delete, and encodes the ID' do
+        client.should_receive(:api_delete).
+          with('sobjects/Whizbang/1234%2F%3Fabc')
+        expect(result).to be_true
+      end
+    end
   end
 
   describe '.find' do
@@ -480,6 +500,16 @@
         expect(result).to eq response.body
       end
     end
+
+    context 'when an internal ID which contains special characters is specified' do
+      let(:id)    { "1234/?abc" }
+      it 'returns the full representation of the object' do
+        client.should_receive(:api_get).
+          with('sobjects/Whizbang/1234%2F%3Fabc').
+          and_return(response)
+        expect(result).to eq response.body
+      end
+    end
   end
 
   describe '.select' do
@@ -551,6 +581,16 @@
         end
       end
     end
+
+    context 'when an internal ID which contains special characters is specified' do
+      let(:id)    { "1234/?abc" }
+      it 'returns the full representation of the object' do
+        client.should_receive(:api_get).
+          with('sobjects/Whizbang/1234%2F%3Fabc').
+          and_return(response)
+        expect(result).to eq response.body
+      end
+    end
   end
 
   describe "#recent" do