Merge pull request #58 from koshigoe/bugfix/fix-wrong-escaping-parameter

Fix wrong path escape
koshigoe · Nov 10, 2017 · b5ebee4 · b5ebee4
2 parents 1cbf9bf + 1f450eb
commit b5ebee4
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,9 @@ Changelog
 
 ### Fixed Bugs:
 
+- [#58](https://github.com/koshigoe/brick_ftp/pull/58) Fix wrong path escape
+    - reported by [terencedignon](https://github.com/terencedignon)
+
 
 [0.4.0](https://github.com/koshigoe/brick_ftp/compare/v0.3.8...v0.4.0)
 ----

diff --git a/lib/brick_ftp/api_component.rb b/lib/brick_ftp/api_component.rb
@@ -1,3 +1,5 @@
+require 'erb'
+
 module BrickFTP
   class APIComponent
     attr_reader :path_template, :query_keys
@@ -12,7 +14,7 @@ def path(params)
 
       path_params = build_path_params_from(params)
       escaped_path_params = path_params.each_with_object({}) do |(k, v), res|
-        res[k] = CGI.escape(v.to_s)
+        res[k] = ERB::Util.url_encode(v.to_s)
       end
 
       path = path_template.call(params) % escaped_path_params

diff --git a/spec/brick_ftp/api/file_spec.rb b/spec/brick_ftp/api/file_spec.rb
@@ -25,7 +25,7 @@
     context 'exists' do
       context 'without query action=stat' do
         before do
-          stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering+Candidates%2FJohn+Smith.docx')
+          stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering%20Candidates%2FJohn%20Smith.docx')
             .with(basic_auth: ['xxxxxxxx', 'x'])
             .to_return(body: file.to_json)
         end
@@ -54,7 +54,7 @@
 
         before do
           file.delete('download_uri')
-          stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering+Candidates%2FJohn+Smith.docx?action=stat')
+          stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering%20Candidates%2FJohn%20Smith.docx?action=stat')
             .with(basic_auth: ['xxxxxxxx', 'x'])
             .to_return(body: file.to_json)
         end
@@ -81,7 +81,7 @@
 
     context 'not exists' do
       before do
-        stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering+Candidates%2FJohn+Smith.docx')
+        stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering%20Candidates%2FJohn%20Smith.docx')
           .with(basic_auth: ['xxxxxxxx', 'x'])
           .to_return(body: '[]')
       end
@@ -99,7 +99,7 @@
       let(:file) { described_class.new(path: 'Engineering Candidates/John Smith.docx') }
 
       before do
-        stub_request(:delete, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering+Candidates%2FJohn+Smith.docx')
+        stub_request(:delete, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering%20Candidates%2FJohn%20Smith.docx')
           .with(basic_auth: ['xxxxxxxx', 'x'])
           .to_return(body: '[]')
       end
@@ -115,7 +115,7 @@
       let(:file) { described_class.new(path: 'Engineering Candidates') }
 
       before do
-        stub_request(:delete, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering+Candidates')
+        stub_request(:delete, 'https://koshigoe.brickftp.com/api/rest/v1/files/Engineering%20Candidates')
           .with(basic_auth: ['xxxxxxxx', 'x'], headers: { 'Depth' => 'infinity' })
           .to_return(body: '[]')
       end

diff --git a/spec/brick_ftp/api/folder_spec.rb b/spec/brick_ftp/api/folder_spec.rb
@@ -57,7 +57,7 @@
     end
 
     before do
-      stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/folders/Engineering+Candidates%2FR%C3%A9sum%C3%A9s?page=1&per_page=10&search=Engineering%20Candidates%2F&sort_by%5Bmodified_at_datetime%5D=asc&sort_by%5Bpath%5D=asc&sort_by%5Bsize%5D=asc')
+      stub_request(:get, 'https://koshigoe.brickftp.com/api/rest/v1/folders/Engineering%20Candidates%2FR%C3%A9sum%C3%A9s?page=1&per_page=10&search=Engineering%20Candidates%2F&sort_by%5Bmodified_at_datetime%5D=asc&sort_by%5Bpath%5D=asc&sort_by%5Bsize%5D=asc')
         .with(basic_auth: ['xxxxxxxx', 'x'])
         .to_return(body: folders.to_json)
     end

diff --git a/spec/brick_ftp/api_component_spec.rb b/spec/brick_ftp/api_component_spec.rb
@@ -12,7 +12,7 @@
 
   let(:params) do
     {
-      path: 'a/b/c.txt',
+      path: 'a/b/c include spaces.txt',
       page: 1,
       per_page: 1,
       search: 'a/',
@@ -26,7 +26,7 @@
     subject { api_component.path(params) }
 
     context 'params is a Hash' do
-      it { is_expected.to eq '/api/rest/v1/folders/a%2Fb%2Fc.txt?page=1&per_page=1&search=a%2F&sort_by%5Bpath%5D=asc&sort_by%5Bsize%5D=asc&sort_by%5Bmodified_at_datetime%5D=asc' }
+      it { is_expected.to eq '/api/rest/v1/folders/a%2Fb%2Fc%20include%20spaces.txt?page=1&per_page=1&search=a%2F&sort_by%5Bpath%5D=asc&sort_by%5Bsize%5D=asc&sort_by%5Bmodified_at_datetime%5D=asc' }
     end
 
     context 'params is not a Hash' do
@@ -56,7 +56,7 @@
 
     context 'params is a Hash' do
       it do
-        is_expected.to eq(path: 'a/b/c.txt')
+        is_expected.to eq(path: 'a/b/c include spaces.txt')
       end
     end