percent encode @, : in proxy user/pass

2 files changed, 27 insertions, 8 deletions

diff --git a/lib/chef/application.rb b/lib/chef/application.rb
index 04ebbba25a..68108d21bc 100644
--- a/lib/chef/application.rb
+++ b/lib/chef/application.rb
@@ -282,13 +282,13 @@ class Chef::Application
   def configure_proxy(scheme)
     proxy = Chef::Config["#{scheme}_proxy"].split("#{scheme}://")
     proxy.shift if proxy[0].empty?
-    proxy = URI.escape(proxy[0])
+    proxy = URI.encode(proxy[0])
 
     full_proxy = "#{scheme}://"
     if Chef::Config["#{scheme}_proxy_user"]
-      full_proxy << encode_uri_full(Chef::Config["#{scheme}_proxy_user"])
+      full_proxy << encode_for_proxy(Chef::Config["#{scheme}_proxy_user"])
       if Chef::Config["#{scheme}_proxy_pass"]
-        full_proxy << ":#{encode_uri_full(Chef::Config["#{scheme}_proxy_pass"])}"
+        full_proxy << ":#{encode_for_proxy(Chef::Config["#{scheme}_proxy_pass"])}"
       end
       full_proxy << "@"
     end
@@ -297,11 +297,12 @@ class Chef::Application
     return full_proxy
   end
 
-  # URI doesn't encode/escape reserved characters from the percent encoding set.
-  # For strings such as proxy user and proxy password we need these reserved characters
-  # to be escaped, or else the fully proxy might not be interpreted correctly.
-  def encode_uri_full(uri_str)
-    URI.escape(uri_str, "!#$&'()*+,/:;=?@[]")
+  # URI doesn't encode/escape the reserved characters '@' and ':' which may exist
+  # in the proxy user/password.
+  def encode_for_proxy(uri_str)
+    # URI.escape(string, characters) will only escape the characters. So we first
+    # perform a standard URI escape, then escape other potentially offending characters.
+    URI.escape(URI.escape(uri_str), '@:')
   end
 
   # This is a hook for testing
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index efde6d6690..698999b8dc 100644
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -303,6 +303,12 @@ describe Chef::Application do
           @env['HTTP_PROXY'].should == "http://hostname:port"
         end
 
+        it "should percent encode the proxy, if necessary" do
+          Chef::Config[:http_proxy] = "http://needs\\some escaping:1234"
+          @app.configure_environment_variables
+          @env['HTTP_PROXY'].should == "http://needs%5Csome%20escaping:1234"
+        end
+
         describe "when Chef::Config[:http_proxy_user] is set" do
           before do
             Chef::Config[:http_proxy_user] = "username"
@@ -313,6 +319,12 @@ describe Chef::Application do
             @env['HTTP_PROXY'].should == "http://username@hostname:port"
           end
 
+          it "should percent encode the username, including @ and : characters" do
+            Chef::Config[:http_proxy_user] = "K:tty C@t"
+            @app.configure_environment_variables
+            @env['HTTP_PROXY'].should == "http://K%3Atty%20C%40t@hostname:port"
+          end
+
           describe "when Chef::Config[:http_proxy_pass] is set" do
             before do
               Chef::Config[:http_proxy_pass] = "password"
@@ -322,6 +334,12 @@ describe Chef::Application do
               @app.configure_environment_variables
               @env['HTTP_PROXY'].should == "http://username:password@hostname:port"
             end
+
+            it "should fully percent escape the password, including @ and : characters" do
+              Chef::Config[:http_proxy_pass] = ":P@ssword101"
+              @app.configure_environment_variables
+              @env['HTTP_PROXY'].should == "http://username:%3AP%40ssword101@hostname:port"
+            end
           end
         end