Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

escape colon in path segment #1237

Merged
merged 5 commits into from
Jan 18, 2021
Merged

escape colon in path segment #1237

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae627fc
escape colon in path segment
yarafan Jan 12, 2021
36a4ad7
Merge branch 'master' into fix/colon_in_path
iMacTia Jan 13, 2021
d29c563
fix rubocop
yarafan Jan 13, 2021
02dc8e9
Merge branch 'fix/colon_in_path' of github.com:yarafan/faraday into f…
yarafan Jan 13, 2021
abc7309
leave uri escaped
yarafan Jan 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/faraday/connection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,9 @@ def build_exclusive_url(url = nil, params = nil, params_encoder = nil)
base = base.dup
base.path = "#{base.path}/" # ensure trailing slash
end
url = url && URI.parse(url.to_s).opaque ? url.to_s.gsub(':', '%3A') : url
uri = url ? base + url : base
uri = URI.parse(CGI.unescape(uri.to_s))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether final uri should be unescaped

Copy link
Member

@iMacTia iMacTia Jan 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, this may cause issues in case the URL contains escaped characters like spaces? Escaped columns may also be part of the URL, so I'm not sure unescaping here is a good idea at all.
E.g. Faraday.get('https://service.com/path%20with%20spaces')

if params
uri.query = params.to_query(params_encoder || options.params_encoder)
end
Expand Down
23 changes: 23 additions & 0 deletions spec/faraday/connection_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,29 @@
expect(uri.to_s).to eq('http://sushi.com/sake/')
end
end

context 'with colon in path' do
let(:url) { 'http://service.com' }

it 'joins url to base when used absolute path' do
conn = Faraday.new(url: url)
uri = conn.build_exclusive_url('/service:search?limit=400')
expect(uri.to_s).to eq('http://service.com/service:search?limit=400')
end

it 'joins url to base when used relative path' do
conn = Faraday.new(url: url)
uri = conn.build_exclusive_url('service:search?limit=400')
expect(uri.to_s).to eq('http://service.com/service:search?limit=400')
end

it 'joins url to base when used with path prefix' do
conn = Faraday.new(url: url)
conn.path_prefix = '/api'
uri = conn.build_exclusive_url('service:search?limit=400')
expect(uri.to_s).to eq('http://service.com/api/service:search?limit=400')
end
end
end

describe '#build_url' do
Expand Down