Skip to content

Commit

Permalink
forces request query param values to be escaped
Browse files Browse the repository at this point in the history
udpates params parsing function and adds tests

Corrects error in merge

udpates specs
  • Loading branch information
Michael S. Kazmier committed Dec 18, 2017
1 parent 7491633 commit ddfa903
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
3 changes: 2 additions & 1 deletion api_auth.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ Gem::Specification.new do |s|
s.add_development_dependency 'actionpack', '< 6.0', '> 4.0'
s.add_development_dependency 'activeresource', '>= 4.0'
s.add_development_dependency 'activesupport', '< 6.0', '> 4.0'
s.add_development_dependency 'rails', '>= 4.0'
s.add_development_dependency 'curb', '~> 0.8.1'
s.add_development_dependency 'amatch'
s.add_development_dependency 'appraisal'
s.add_development_dependency 'curb', '~> 0.8'
s.add_development_dependency 'faraday', '>= 0.10'
s.add_development_dependency 'httpi'
s.add_development_dependency 'multipart-post', '~> 2.0'
Expand Down
24 changes: 22 additions & 2 deletions lib/api_auth/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,29 @@ def sign_header(header)
def parse_uri(uri)
parsed_uri = URI.parse(uri)

return parsed_uri.request_uri if parsed_uri.respond_to?(:request_uri)
uri_without_host = parsed_uri.respond_to?(:request_uri) ? parsed_uri.request_uri : uri
return '/' if uri_without_host.empty?
escape_params(uri_without_host)
end

uri.empty? ? '/' : uri
# Different versions of request parsers escape/unescape the param values
# Examples:
# Rails 5.1.3 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id%2Cfirst_name%2Clast_name,Thu, 14 Dec 2017 16:19:48 GMT'
# Rails 5.1.4 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id,first_name,last_name,Thu, 14 Dec 2017 16:20:57 GMT'
# This will force param values to escaped and fixes issue #123
def escape_params(uri)
unescaped_uri = CGI.unescape(uri)
uri_array = unescaped_uri.split('?')
return uri unless uri_array.length > 1
params = uri_array[1].split('&')
encoded_params = ""
params.each do |param|
next unless param.include?('=')
encoded_params += '&' if encoded_params.length.positive?
split_param = param.split('=')
encoded_params += split_param[0] + '=' + CGI.escape(split_param[1])
end
uri_array[0] + '?' + encoded_params
end
end
end
16 changes: 15 additions & 1 deletion spec/headers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,27 @@
let(:uri) { 'http://google.com/?redirect_to=https://www.example.com'.freeze }

it 'return /?redirect_to=https://www.example.com as canonical string path' do
expect(subject.canonical_string).to eq('GET,,,/?redirect_to=https://www.example.com,')
expect(subject.canonical_string).to eq('GET,,,/?redirect_to=https%3A%2F%2Fwww.example.com,')
end

it 'does not change request url (by removing host)' do
expect(request.url).to eq(uri)
end
end

context 'uri param values are not escaped' do
let(:uri) { 'http://google.com/search/advanced?redirect_to=https://www.example.com&account=a12dd334/3444\:23'.freeze }
it 'returns correct anonical string' do
expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,')
end
end

context 'uri param values are escaped' do
let(:uri) { 'http://google.com/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23'.freeze }
it 'returns correct anonical string' do
expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,')
end
end
end

context 'string construction' do
Expand Down

0 comments on commit ddfa903

Please sign in to comment.