Skip to content

Commit 4e9d226

Browse files
ccouzensautopp
authored andcommitted
Ruby: Avoid double escaping path items (#3093)
`URI.encode` is obsolete. `CGI.escape`, `URI.encode_www_form` or `URI.encode_www_form_component` are recommended instead. https://ruby-doc.org/stdlib-2.6/libdoc/uri/rdoc/URI/Escape.html#method-i-escape URI.encode has different behaviour to CGI.escape: ```ruby URI.encode('hello/world?test%string') => "hello/world?test%25string" CGI.escape('hello/world?test%string') => "hello%2Fworld%3Ftest%25string" ``` I recently raised pull request #3039 201cbdc That pull request escapes path items at insertion. Before either pull request, the path item 'hello?world' would go into the URL as 'hello?world'. That behaviour was insecure as if an attacker could control the path item value, they could change the URL the application connected to. After #3039 'hello?world' would go in as 'hello%253Fworld'. This was safer than before, but it's still not correct. If I'd realised at the time, I would have made it correct at the time. What this pull request does is make it go in as 'hello%35world', which is correct. ApiClient::build_request_url was URI.encoding the whole path. This wasn't protecting against all undesirable characters in the path items, but was escaping % characters a 2nd time which was unhelpful. I have additionally removed URI.encode from Configuration::base_url as I can't see any benefit it could be bringing. There is no justification for it in the commit where it was originally added: 47c8597
1 parent 66bf0dc commit 4e9d226

20 files changed

+6
-32
lines changed

modules/openapi-generator/src/main/resources/ruby-client/api.mustache

-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
{{> api_info}}
33
=end
44

5-
require 'uri'
65
require 'cgi'
76

87
module {{moduleName}}

modules/openapi-generator/src/main/resources/ruby-client/api_client.mustache

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ require 'json'
77
require 'logger'
88
require 'tempfile'
99
require 'typhoeus'
10-
require 'uri'
1110

1211
module {{moduleName}}
1312
class ApiClient
@@ -256,7 +255,7 @@ module {{moduleName}}
256255
def build_request_url(path)
257256
# Add leading and trailing slashes to path
258257
path = "/#{path}".gsub(/\/+/, '/')
259-
URI.encode(@config.base_url + path)
258+
@config.base_url + path
260259
end
261260

262261
# Builds the HTTP request body

modules/openapi-generator/src/main/resources/ruby-client/configuration.mustache

+1-4
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
{{> api_info}}
33
=end
44

5-
require 'uri'
6-
75
module {{moduleName}}
86
class Configuration
97
# Defines url scheme
@@ -166,8 +164,7 @@ module {{moduleName}}
166164
end
167165

168166
def base_url
169-
url = "#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '')
170-
URI.encode(url)
167+
"#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '')
171168
end
172169

173170
# Gets API key (with prefix if set).

samples/client/petstore/ruby/lib/petstore/api/another_fake_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/client/petstore/ruby/lib/petstore/api/fake_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/client/petstore/ruby/lib/petstore/api/fake_classname_tags123_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/client/petstore/ruby/lib/petstore/api/pet_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/client/petstore/ruby/lib/petstore/api/store_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/client/petstore/ruby/lib/petstore/api/user_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/client/petstore/ruby/lib/petstore/api_client.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
require 'logger'
1616
require 'tempfile'
1717
require 'typhoeus'
18-
require 'uri'
1918

2019
module Petstore
2120
class ApiClient
@@ -262,7 +261,7 @@ def sanitize_filename(filename)
262261
def build_request_url(path)
263262
# Add leading and trailing slashes to path
264263
path = "/#{path}".gsub(/\/+/, '/')
265-
URI.encode(@config.base_url + path)
264+
@config.base_url + path
266265
end
267266

268267
# Builds the HTTP request body

samples/client/petstore/ruby/lib/petstore/configuration.rb

+1-4
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
14-
1513
module Petstore
1614
class Configuration
1715
# Defines url scheme
@@ -174,8 +172,7 @@ def base_path=(base_path)
174172
end
175173

176174
def base_url
177-
url = "#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '')
178-
URI.encode(url)
175+
"#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '')
179176
end
180177

181178
# Gets API key (with prefix if set).

samples/openapi3/client/petstore/ruby/lib/petstore/api/another_fake_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/openapi3/client/petstore/ruby/lib/petstore/api/default_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/openapi3/client/petstore/ruby/lib/petstore/api/fake_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/openapi3/client/petstore/ruby/lib/petstore/api/fake_classname_tags123_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/openapi3/client/petstore/ruby/lib/petstore/api/pet_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/openapi3/client/petstore/ruby/lib/petstore/api/store_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/openapi3/client/petstore/ruby/lib/petstore/api/user_api.rb

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
1413
require 'cgi'
1514

1615
module Petstore

samples/openapi3/client/petstore/ruby/lib/petstore/api_client.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
require 'logger'
1616
require 'tempfile'
1717
require 'typhoeus'
18-
require 'uri'
1918

2019
module Petstore
2120
class ApiClient
@@ -262,7 +261,7 @@ def sanitize_filename(filename)
262261
def build_request_url(path)
263262
# Add leading and trailing slashes to path
264263
path = "/#{path}".gsub(/\/+/, '/')
265-
URI.encode(@config.base_url + path)
264+
@config.base_url + path
266265
end
267266

268267
# Builds the HTTP request body

samples/openapi3/client/petstore/ruby/lib/petstore/configuration.rb

+1-4
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
1111
=end
1212

13-
require 'uri'
14-
1513
module Petstore
1614
class Configuration
1715
# Defines url scheme
@@ -174,8 +172,7 @@ def base_path=(base_path)
174172
end
175173

176174
def base_url
177-
url = "#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '')
178-
URI.encode(url)
175+
"#{scheme}://#{[host, base_path].join('/').gsub(/\/+/, '/')}".sub(/\/+\z/, '')
179176
end
180177

181178
# Gets API key (with prefix if set).

0 commit comments

Comments
 (0)