Skip to content

Commit e57d4f3

Browse files
leipertjeremyevans
authored andcommitted
Decode user and password from env configured proxy
If someone sets an env variable defining a http_proxy, containing a username / password with percent-encoded characters, then the resulting base64 encoded auth header will be wrong. For example, suppose a username is `Y\X` and the password is `R%S] ?X`. Properly URL encoded the proxy url would be: http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000 The resulting proxy auth header should be: `WVxYOlIlU10gP1g=`, but the getters defined by ruby StdLib `URI` return a username `Y%5CX` and password `R%25S%5D%20%3FX`, resulting in `WSU1Q1g6UiUyNVMlNUQlMjAlM0ZY`. As a result the proxy will deny the request. Please note that this is my first contribution to the ruby ecosystem, to standard lib especially and I am not a ruby developer. References: - https://gitlab.com/gitlab-org/gitlab/-/issues/289836 - https://bugs.ruby-lang.org/projects/ruby-master/repository/trunk/revisions/58461 - https://bugs.ruby-lang.org/issues/17542
1 parent 69b2bbc commit e57d4f3

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

Diff for: lib/net/http.rb

+9-2
Original file line numberDiff line numberDiff line change
@@ -1183,7 +1183,8 @@ def proxy_port
11831183
# The username of the proxy server, if one is configured.
11841184
def proxy_user
11851185
if ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE && @proxy_from_env
1186-
proxy_uri&.user
1186+
user = proxy_uri&.user
1187+
unescape(user) if user
11871188
else
11881189
@proxy_user
11891190
end
@@ -1192,7 +1193,8 @@ def proxy_user
11921193
# The password of the proxy server, if one is configured.
11931194
def proxy_pass
11941195
if ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE && @proxy_from_env
1195-
proxy_uri&.password
1196+
pass = proxy_uri&.password
1197+
unescape(pass) if pass
11961198
else
11971199
@proxy_pass
11981200
end
@@ -1203,6 +1205,11 @@ def proxy_pass
12031205

12041206
private
12051207

1208+
def unescape(value)
1209+
require 'cgi/util'
1210+
CGI.unescape(value)
1211+
end
1212+
12061213
# without proxy, obsolete
12071214

12081215
def conn_address # :nodoc:

Diff for: test/net/http/test_http.rb

+17
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,23 @@ def test_proxy_eh_ENV_with_user
188188
end
189189
end
190190

191+
def test_proxy_eh_ENV_with_urlencoded_user
192+
TestNetHTTPUtils.clean_http_proxy_env do
193+
ENV['http_proxy'] = 'http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000'
194+
195+
http = Net::HTTP.new 'hostname.example'
196+
197+
assert_equal true, http.proxy?
198+
if Net::HTTP::ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE
199+
assert_equal "Y\\X", http.proxy_user
200+
assert_equal "R%S] ?X", http.proxy_pass
201+
else
202+
assert_nil http.proxy_user
203+
assert_nil http.proxy_pass
204+
end
205+
end
206+
end
207+
191208
def test_proxy_eh_ENV_none_set
192209
TestNetHTTPUtils.clean_http_proxy_env do
193210
assert_equal false, Net::HTTP.new('hostname.example').proxy?

0 commit comments

Comments
 (0)