Skip to content

Commit ddfa903

Browse files
author
Michael S. Kazmier
committed
forces request query param values to be escaped
udpates params parsing function and adds tests Corrects error in merge udpates specs
1 parent 7491633 commit ddfa903

File tree

3 files changed

+39
-4
lines changed

3 files changed

+39
-4
lines changed

api_auth.gemspec

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ Gem::Specification.new do |s|
1414
s.add_development_dependency 'actionpack', '< 6.0', '> 4.0'
1515
s.add_development_dependency 'activeresource', '>= 4.0'
1616
s.add_development_dependency 'activesupport', '< 6.0', '> 4.0'
17+
s.add_development_dependency 'rails', '>= 4.0'
18+
s.add_development_dependency 'curb', '~> 0.8.1'
1719
s.add_development_dependency 'amatch'
1820
s.add_development_dependency 'appraisal'
19-
s.add_development_dependency 'curb', '~> 0.8'
2021
s.add_development_dependency 'faraday', '>= 0.10'
2122
s.add_development_dependency 'httpi'
2223
s.add_development_dependency 'multipart-post', '~> 2.0'

lib/api_auth/headers.rb

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,29 @@ def sign_header(header)
9494
def parse_uri(uri)
9595
parsed_uri = URI.parse(uri)
9696

97-
return parsed_uri.request_uri if parsed_uri.respond_to?(:request_uri)
97+
uri_without_host = parsed_uri.respond_to?(:request_uri) ? parsed_uri.request_uri : uri
98+
return '/' if uri_without_host.empty?
99+
escape_params(uri_without_host)
100+
end
98101

99-
uri.empty? ? '/' : uri
102+
# Different versions of request parsers escape/unescape the param values
103+
# Examples:
104+
# 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'
105+
# 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'
106+
# This will force param values to escaped and fixes issue #123
107+
def escape_params(uri)
108+
unescaped_uri = CGI.unescape(uri)
109+
uri_array = unescaped_uri.split('?')
110+
return uri unless uri_array.length > 1
111+
params = uri_array[1].split('&')
112+
encoded_params = ""
113+
params.each do |param|
114+
next unless param.include?('=')
115+
encoded_params += '&' if encoded_params.length.positive?
116+
split_param = param.split('=')
117+
encoded_params += split_param[0] + '=' + CGI.escape(split_param[1])
118+
end
119+
uri_array[0] + '?' + encoded_params
100120
end
101121
end
102122
end

spec/headers_spec.rb

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,27 @@
3535
let(:uri) { 'http://google.com/?redirect_to=https://www.example.com'.freeze }
3636

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

4141
it 'does not change request url (by removing host)' do
4242
expect(request.url).to eq(uri)
4343
end
4444
end
45+
46+
context 'uri param values are not escaped' do
47+
let(:uri) { 'http://google.com/search/advanced?redirect_to=https://www.example.com&account=a12dd334/3444\:23'.freeze }
48+
it 'returns correct anonical string' do
49+
expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,')
50+
end
51+
end
52+
53+
context 'uri param values are escaped' do
54+
let(:uri) { 'http://google.com/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23'.freeze }
55+
it 'returns correct anonical string' do
56+
expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,')
57+
end
58+
end
4559
end
4660

4761
context 'string construction' do

0 commit comments

Comments
 (0)