Environment proxy refactoring (#712)

* Dynamically reloads the proxy when performing a request on an absolute domain
Fixes #701

* Removes `proxy` attr_reader to avoid warning. Reintroduce it when removing the `get/set proxy`

* Request proxy is set in `run_request` and not `build_request` anymore, as url is not available in `build_reqeust`, had to remove a test for this.

* Adds new tests to check proxy working with `Faraday.get`

lib/faraday/connection.rb

@@ -39,6 +39,9 @@ module Faraday
     # Public: Sets the default parallel manager for this connection.
     attr_writer :default_parallel_manager
 
+    # Public: Gets or Sets the Hash proxy options.
+    # attr_reader :proxy
+
     # Public: Initializes a new Faraday::Connection.
     #
     # url     - URI or String base URL to use as a prefix for all
@@ -79,23 +82,8 @@ module Faraday
       @params.update(options.params)   if options.params
       @headers.update(options.headers) if options.headers
 
-      @proxy = nil
-      proxy(options.fetch(:proxy) {
-        uri = nil
-        if URI.parse("").respond_to?(:find_proxy)
-          case url
-          when String
-            uri = URI.parse(url).find_proxy
-          when URI
-            uri = url.find_proxy
-          when nil
-            uri = find_default_proxy
-          end
-        else
-          uri = find_default_proxy
-        end
-        uri
-      })
+      @proxy = options.proxy ? ProxyOptions.from(options.proxy) : proxy_from_env(url)
+      @temp_proxy = @proxy
 
       yield(self) if block_given?
 
@@ -292,9 +280,15 @@ module Faraday
     # Public: Gets or Sets the Hash proxy options.
     def proxy(arg = nil)
       return @proxy if arg.nil?
+      warn 'Warning: use of proxy(new_value) to set connection proxy have been DEPRECATED and will be removed in Faraday 1.0'
       @proxy = ProxyOptions.from(arg)
     end
 
+    # Public: Sets the Hash proxy options.
+    def proxy=(new_value)
+      @proxy = ProxyOptions.from(new_value)
+    end
+
     def_delegators :url_prefix, :scheme, :scheme=, :host, :host=, :port, :port=
     def_delegator :url_prefix, :path, :path_prefix
 
@@ -376,7 +370,14 @@ module Faraday
         raise ArgumentError, "unknown http method: #{method}"
       end
 
+      # Resets temp_proxy
+      @temp_proxy = self.proxy
+
+      # Set temporary proxy if request url is absolute
+      @temp_proxy = proxy_from_env(url) if url && URI(url).absolute?
+
       request = build_request(method) do |req|
+        req.options = req.options.merge(:proxy => @temp_proxy)
         req.url(url)                if url
         req.headers.update(headers) if headers
         req.body = body             if body
@@ -393,7 +394,7 @@ module Faraday
       Request.create(method) do |req|
         req.params  = self.params.dup
         req.headers = self.headers.dup
-        req.options = self.options.merge(:proxy => self.proxy)
+        req.options = self.options
         yield(req) if block_given?
       end
     end
@@ -443,8 +444,25 @@ module Faraday
       headers[Faraday::Request::Authorization::KEY] = header
     end
 
+    def proxy_from_env(url)
+      uri = nil
+      if URI.parse('').respond_to?(:find_proxy)
+        case url
+          when String
+            uri = URI.parse(url).find_proxy
+          when URI
+            uri = url.find_proxy
+          when nil
+            uri = find_default_proxy
+        end
+      else
+        warn 'no_proxy is unsupported' if ENV['no_proxy'] || ENV['NO_PROXY']
+        uri = find_default_proxy
+      end
+      ProxyOptions.from(uri) if uri
+    end
+
     def find_default_proxy
-      warn 'no_proxy is unsupported' if ENV['no_proxy'] || ENV['NO_PROXY']
       uri = ENV['http_proxy']
       if uri && !uri.empty?
         uri = 'http://' + uri if uri !~ /^http/i

test/connection_test.rb

@@ -28,37 +28,37 @@ class TestConnection < Faraday::TestCase
   end
 
   def test_initialize_parses_host_out_of_given_url
-    conn = Faraday::Connection.new "http://sushi.com"
+    conn = Faraday::Connection.new 'http://sushi.com'
     assert_equal 'sushi.com', conn.host
   end
 
   def test_initialize_inherits_default_port_out_of_given_url
-    conn = Faraday::Connection.new "http://sushi.com"
+    conn = Faraday::Connection.new 'http://sushi.com'
     assert_equal 80, conn.port
   end
 
   def test_initialize_parses_scheme_out_of_given_url
-    conn = Faraday::Connection.new "http://sushi.com"
+    conn = Faraday::Connection.new 'http://sushi.com'
     assert_equal 'http', conn.scheme
   end
 
   def test_initialize_parses_port_out_of_given_url
-    conn = Faraday::Connection.new "http://sushi.com:815"
+    conn = Faraday::Connection.new 'http://sushi.com:815'
     assert_equal 815, conn.port
   end
 
   def test_initialize_parses_nil_path_prefix_out_of_given_url
-    conn = Faraday::Connection.new "http://sushi.com"
+    conn = Faraday::Connection.new 'http://sushi.com'
     assert_equal '/', conn.path_prefix
   end
 
   def test_initialize_parses_path_prefix_out_of_given_url
-    conn = Faraday::Connection.new "http://sushi.com/fish"
+    conn = Faraday::Connection.new 'http://sushi.com/fish'
     assert_equal '/fish', conn.path_prefix
   end
 
   def test_initialize_parses_path_prefix_out_of_given_url_option
-    conn = Faraday::Connection.new :url => "http://sushi.com/fish"
+    conn = Faraday::Connection.new :url => 'http://sushi.com/fish'
     assert_equal '/fish', conn.path_prefix
   end
 
@@ -68,12 +68,12 @@ class TestConnection < Faraday::TestCase
   end
 
   def test_initialize_stores_default_params_from_uri
-    conn = Faraday::Connection.new "http://sushi.com/fish?a=1"
+    conn = Faraday::Connection.new 'http://sushi.com/fish?a=1'
     assert_equal({'a' => '1'}, conn.params)
   end
 
   def test_initialize_stores_default_params_from_uri_and_options
-    conn = Faraday::Connection.new "http://sushi.com/fish?a=1&b=2", :params => {'a' => 3}
+    conn = Faraday::Connection.new 'http://sushi.com/fish?a=1&b=2', :params => {'a' => 3}
     assert_equal({'a' => 3, 'b' => '2'}, conn.params)
   end
 
@@ -92,9 +92,9 @@ class TestConnection < Faraday::TestCase
   end
 
   def test_auto_parses_basic_auth_from_url_and_unescapes
-    conn = Faraday::Connection.new :url => "http://foo%40bar.com:pass%20word@sushi.com/fish"
+    conn = Faraday::Connection.new :url => 'http://foo%40bar.com:pass%20word@sushi.com/fish'
     assert auth = conn.headers['Authorization']
-    assert_equal Faraday::Request::BasicAuthentication.header("foo@bar.com", "pass word"), auth
+    assert_equal Faraday::Request::BasicAuthentication.header('foo@bar.com', 'pass word'), auth
   end
 
   def test_token_auth_sets_header
@@ -111,176 +111,176 @@ class TestConnection < Faraday::TestCase
   def test_build_exclusive_url_uses_connection_host_as_default_uri_host
     conn = Faraday::Connection.new
     conn.host = 'sushi.com'
-    uri = conn.build_exclusive_url("/sake.html")
+    uri = conn.build_exclusive_url('/sake.html')
     assert_equal 'sushi.com', uri.host
   end
 
   def test_build_exclusive_url_overrides_connection_port_for_absolute_urls
     conn = Faraday::Connection.new
     conn.port = 23
-    uri = conn.build_exclusive_url("http://sushi.com")
+    uri = conn.build_exclusive_url('http://sushi.com')
     assert_equal 80, uri.port
   end
 
   def test_build_exclusive_url_uses_connection_scheme_as_default_uri_scheme
     conn = Faraday::Connection.new 'http://sushi.com'
-    uri = conn.build_exclusive_url("/sake.html")
+    uri = conn.build_exclusive_url('/sake.html')
     assert_equal 'http', uri.scheme
   end
 
   def test_build_exclusive_url_uses_connection_path_prefix_to_customize_path
     conn = Faraday::Connection.new
     conn.path_prefix = '/fish'
-    uri = conn.build_exclusive_url("sake.html")
+    uri = conn.build_exclusive_url('sake.html')
     assert_equal '/fish/sake.html', uri.path
   end
 
   def test_build_exclusive_url_uses_root_connection_path_prefix_to_customize_path
     conn = Faraday::Connection.new
     conn.path_prefix = '/'
-    uri = conn.build_exclusive_url("sake.html")
+    uri = conn.build_exclusive_url('sake.html')
     assert_equal '/sake.html', uri.path
   end
 
   def test_build_exclusive_url_forces_connection_path_prefix_to_be_absolute
     conn = Faraday::Connection.new
     conn.path_prefix = 'fish'
-    uri = conn.build_exclusive_url("sake.html")
+    uri = conn.build_exclusive_url('sake.html')
     assert_equal '/fish/sake.html', uri.path
   end
 
   def test_build_exclusive_url_ignores_connection_path_prefix_trailing_slash
     conn = Faraday::Connection.new
     conn.path_prefix = '/fish/'
-    uri = conn.build_exclusive_url("sake.html")
+    uri = conn.build_exclusive_url('sake.html')
     assert_equal '/fish/sake.html', uri.path
   end
 
   def test_build_exclusive_url_allows_absolute_uri_to_ignore_connection_path_prefix
     conn = Faraday::Connection.new
     conn.path_prefix = '/fish'
-    uri = conn.build_exclusive_url("/sake.html")
+    uri = conn.build_exclusive_url('/sake.html')
     assert_equal '/sake.html', uri.path
   end
 
   def test_build_exclusive_url_parses_url_params_into_path
     conn = Faraday::Connection.new
-    uri = conn.build_exclusive_url("http://sushi.com/sake.html")
+    uri = conn.build_exclusive_url('http://sushi.com/sake.html')
     assert_equal '/sake.html', uri.path
   end
 
   def test_build_exclusive_url_doesnt_add_ending_slash_given_nil_url
     conn = Faraday::Connection.new
-    conn.url_prefix = "http://sushi.com/nigiri"
+    conn.url_prefix = 'http://sushi.com/nigiri'
     uri = conn.build_exclusive_url
-    assert_equal "/nigiri", uri.path
+    assert_equal '/nigiri', uri.path
   end
 
   def test_build_exclusive_url_doesnt_add_ending_slash_given_empty_url
     conn = Faraday::Connection.new
-    conn.url_prefix = "http://sushi.com/nigiri"
+    conn.url_prefix = 'http://sushi.com/nigiri'
     uri = conn.build_exclusive_url('')
-    assert_equal "/nigiri", uri.path
+    assert_equal '/nigiri', uri.path
   end
 
   def test_build_exclusive_url_doesnt_use_connection_params
-    conn = Faraday::Connection.new "http://sushi.com/nigiri"
+    conn = Faraday::Connection.new 'http://sushi.com/nigiri'
     conn.params = {:a => 1}
-    assert_equal "http://sushi.com/nigiri", conn.build_exclusive_url.to_s
+    assert_equal 'http://sushi.com/nigiri', conn.build_exclusive_url.to_s
   end
 
   def test_build_exclusive_url_uses_argument_params
-    conn = Faraday::Connection.new "http://sushi.com/nigiri"
+    conn = Faraday::Connection.new 'http://sushi.com/nigiri'
     conn.params = {:a => 1}
     params = Faraday::Utils::ParamsHash.new
     params[:a] = 2
     url = conn.build_exclusive_url(nil, params)
-    assert_equal "http://sushi.com/nigiri?a=2", url.to_s
+    assert_equal 'http://sushi.com/nigiri?a=2', url.to_s
   end
 
   def test_build_url_uses_params
-    conn = Faraday::Connection.new "http://sushi.com/nigiri"
+    conn = Faraday::Connection.new 'http://sushi.com/nigiri'
     conn.params = {:a => 1, :b => 1}
-    assert_equal "http://sushi.com/nigiri?a=1&b=1", conn.build_url.to_s
+    assert_equal 'http://sushi.com/nigiri?a=1&b=1', conn.build_url.to_s
   end
 
   def test_build_url_merges_params
-    conn = Faraday::Connection.new "http://sushi.com/nigiri"
+    conn = Faraday::Connection.new 'http://sushi.com/nigiri'
     conn.params = {:a => 1, :b => 1}
     url = conn.build_url(nil, :b => 2, :c => 3)
-    assert_equal "http://sushi.com/nigiri?a=1&b=2&c=3", url.to_s
+    assert_equal 'http://sushi.com/nigiri?a=1&b=2&c=3', url.to_s
   end
 
   def test_request_header_change_does_not_modify_connection_header
-    connection = Faraday.new(:url => "https://asushi.com/sake.html")
-    connection.headers = { "Authorization"=>"token abc123" }
+    connection = Faraday.new(:url => 'https://asushi.com/sake.html')
+    connection.headers = {'Authorization' => 'token abc123'}
 
     request = connection.build_request(:get)
-    request.headers.delete("Authorization")
+    request.headers.delete('Authorization')
 
-    assert_equal connection.headers.keys.sort, ["Authorization"]
-    assert connection.headers.include?("Authorization")
+    assert_equal connection.headers.keys.sort, ['Authorization']
+    assert connection.headers.include?('Authorization')
 
     assert_equal request.headers.keys.sort, []
-    assert !request.headers.include?("Authorization")
+    assert !request.headers.include?('Authorization')
   end
 
   def test_env_url_parses_url_params_into_query
-    uri = env_url("http://sushi.com/sake.html", 'a[b]' => '1 + 2')
-    assert_equal "a%5Bb%5D=1+%2B+2", uri.query
+    uri = env_url('http://sushi.com/sake.html', 'a[b]' => '1 + 2')
+    assert_equal 'a%5Bb%5D=1+%2B+2', uri.query
   end
 
   def test_env_url_escapes_per_spec
     uri = env_url('http:/', 'a' => '1+2 foo~bar.-baz')
-    assert_equal "a=1%2B2+foo~bar.-baz", uri.query
+    assert_equal 'a=1%2B2+foo~bar.-baz', uri.query
   end
 
   def test_env_url_bracketizes_nested_params_in_query
     url = env_url nil, 'a' => {'b' => 'c'}
-    assert_equal "a%5Bb%5D=c", url.query
+    assert_equal 'a%5Bb%5D=c', url.query
   end
 
   def test_env_url_bracketizes_repeated_params_in_query
-    uri = env_url("http://sushi.com/sake.html", 'a' => [1, 2])
-    assert_equal "a%5B%5D=1&a%5B%5D=2", uri.query
+    uri = env_url('http://sushi.com/sake.html', 'a' => [1, 2])
+    assert_equal 'a%5B%5D=1&a%5B%5D=2', uri.query
   end
 
   def test_env_url_without_braketizing_repeated_params_in_query
     uri = env_url 'http://sushi.com', 'a' => [1, 2] do |conn|
       conn.options.params_encoder = Faraday::FlatParamsEncoder
     end
-    assert_equal "a=1&a=2", uri.query
+    assert_equal 'a=1&a=2', uri.query
   end
 
   def test_build_exclusive_url_parses_url
     conn = Faraday::Connection.new
-    uri = conn.build_exclusive_url("http://sushi.com/sake.html")
-    assert_equal "http",       uri.scheme
-    assert_equal "sushi.com",  uri.host
+    uri = conn.build_exclusive_url('http://sushi.com/sake.html')
+    assert_equal 'http', uri.scheme
+    assert_equal 'sushi.com', uri.host
     assert_equal '/sake.html', uri.path
   end
 
   def test_build_exclusive_url_parses_url_and_changes_scheme
-    conn = Faraday::Connection.new :url => "http://sushi.com/sushi"
+    conn = Faraday::Connection.new :url => 'http://sushi.com/sushi'
     conn.scheme = 'https'
-    uri = conn.build_exclusive_url("sake.html")
+    uri = conn.build_exclusive_url('sake.html')
     assert_equal 'https://sushi.com/sushi/sake.html', uri.to_s
   end
 
   def test_build_exclusive_url_joins_url_to_base_with_ending_slash
-    conn = Faraday::Connection.new :url => "http://sushi.com/sushi/"
-    uri = conn.build_exclusive_url("sake.html")
+    conn = Faraday::Connection.new :url => 'http://sushi.com/sushi/'
+    uri = conn.build_exclusive_url('sake.html')
     assert_equal 'http://sushi.com/sushi/sake.html', uri.to_s
   end
 
   def test_build_exclusive_url_used_default_base_with_ending_slash
-    conn = Faraday::Connection.new :url => "http://sushi.com/sushi/"
+    conn = Faraday::Connection.new :url => 'http://sushi.com/sushi/'
     uri = conn.build_exclusive_url
     assert_equal 'http://sushi.com/sushi/', uri.to_s
   end
 
   def test_build_exclusive_url_overrides_base
-    conn = Faraday::Connection.new :url => "http://sushi.com/sushi/"
+    conn = Faraday::Connection.new :url => 'http://sushi.com/sushi/'
     uri = conn.build_exclusive_url('/sake/')
     assert_equal 'http://sushi.com/sake/', uri.to_s
   end
@@ -292,48 +292,48 @@ class TestConnection < Faraday::TestCase
   end
 
   def test_proxy_accepts_string
-    with_env 'http_proxy' => "http://duncan.proxy.com:80" do
+    with_env 'http_proxy' => 'http://duncan.proxy.com:80' do
       conn = Faraday::Connection.new
-      conn.proxy 'http://proxy.com'
+      conn.proxy = 'http://proxy.com'
       assert_equal 'proxy.com', conn.proxy.host
     end
   end
 
   def test_proxy_accepts_uri
-    with_env 'http_proxy' => "http://duncan.proxy.com:80" do
+    with_env 'http_proxy' => 'http://duncan.proxy.com:80' do
       conn = Faraday::Connection.new
-      conn.proxy URI.parse('http://proxy.com')
+      conn.proxy = URI.parse('http://proxy.com')
       assert_equal 'proxy.com', conn.proxy.host
     end
   end
 
   def test_proxy_accepts_hash_with_string_uri
-    with_env 'http_proxy' => "http://duncan.proxy.com:80" do
+    with_env 'http_proxy' => 'http://duncan.proxy.com:80' do
       conn = Faraday::Connection.new
-      conn.proxy :uri => 'http://proxy.com', :user => 'rick'
+      conn.proxy = {:uri => 'http://proxy.com', :user => 'rick'}
       assert_equal 'proxy.com', conn.proxy.host
       assert_equal 'rick',      conn.proxy.user
     end
   end
 
   def test_proxy_accepts_hash
-    with_env 'http_proxy' => "http://duncan.proxy.com:80" do
+    with_env 'http_proxy' => 'http://duncan.proxy.com:80' do
       conn = Faraday::Connection.new
-      conn.proxy :uri => URI.parse('http://proxy.com'), :user => 'rick'
+      conn.proxy = {:uri => URI.parse('http://proxy.com'), :user => 'rick'}
       assert_equal 'proxy.com', conn.proxy.host
       assert_equal 'rick',      conn.proxy.user
     end
   end
 
   def test_proxy_accepts_http_env
-    with_env 'http_proxy' => "http://duncan.proxy.com:80" do
+    with_env 'http_proxy' => 'http://duncan.proxy.com:80' do
       conn = Faraday::Connection.new
       assert_equal 'duncan.proxy.com', conn.proxy.host
     end
   end
 
   def test_proxy_accepts_http_env_with_auth
-    with_env 'http_proxy' => "http://a%40b:my%20pass@duncan.proxy.com:80" do
+    with_env 'http_proxy' => 'http://a%40b:my%20pass@duncan.proxy.com:80' do
       conn = Faraday::Connection.new
       assert_equal 'a@b',     conn.proxy.user
       assert_equal 'my pass', conn.proxy.password
@@ -341,7 +341,7 @@ class TestConnection < Faraday::TestCase
   end
 
   def test_proxy_accepts_env_without_scheme
-    with_env 'http_proxy' => "localhost:8888" do
+    with_env 'http_proxy' => 'localhost:8888' do
       uri = Faraday::Connection.new.proxy[:uri]
       assert_equal 'localhost', uri.host
       assert_equal 8888, uri.port
@@ -363,13 +363,20 @@ class TestConnection < Faraday::TestCase
   end
 
   def test_proxy_doesnt_accept_uppercase_env
-    with_env 'HTTP_PROXY' => "http://localhost:8888/" do
+    with_env 'HTTP_PROXY' => 'http://localhost:8888/' do
       conn = Faraday::Connection.new
       assert_nil conn.proxy
     end
   end
 
-  if URI.parse("").respond_to?(:find_proxy)
+  def test_dynamic_proxy
+    with_env 'http_proxy' => 'http://duncan.proxy.com:80' do
+      Faraday.get('http://google.co.uk')
+      assert_equal 'duncan.proxy.com', Faraday.default_connection.instance_variable_get('@temp_proxy').host
+    end
+  end
+
+  if URI.parse('').respond_to?(:find_proxy)
     def test_proxy_allowed_when_url_in_no_proxy_list
       with_env 'http_proxy' => 'http://proxy.com', 'no_proxy' => 'example.com' do
         conn = Faraday::Connection.new('http://example.com')
@@ -420,12 +427,22 @@ class TestConnection < Faraday::TestCase
         assert_equal 'proxy.com', Faraday::Connection.new('http://example2.com').proxy.host
       end
     end
+
+    def test_dynamic_no_proxy
+      with_env 'http_proxy' => 'http://proxy.com', 'no_proxy' => 'google.co.uk' do
+        conn = Faraday.new
+
+        assert_equal 'proxy.com', conn.instance_variable_get('@temp_proxy').host
+        conn.get('https://google.co.uk')
+        assert_nil conn.instance_variable_get('@temp_proxy')
+      end
+    end
   end
 
   def test_proxy_requires_uri
     conn = Faraday::Connection.new
     assert_raises ArgumentError do
-      conn.proxy :uri => :bad_uri, :user => 'rick'
+      conn.proxy = {:uri => :bad_uri, :user => 'rick'}
     end
   end
 
@@ -622,7 +639,7 @@ class TestRequestParams < Faraday::TestCase
     with_default_params_encoder(nil) do
       create_connection 'http://a.co/page1?color[]=red&color[]=blue'
       query = get
-      assert_equal "color%5B%5D=red&color%5B%5D=blue", query
+      assert_equal 'color%5B%5D=red&color%5B%5D=blue', query
     end
   end
 
@@ -630,7 +647,7 @@ class TestRequestParams < Faraday::TestCase
     with_default_params_encoder(nil) do
       create_connection 'http://a.co/page1', :params => {:color => ['red', 'blue']}
       query = get
-      assert_equal "color%5B%5D=red&color%5B%5D=blue", query
+      assert_equal 'color%5B%5D=red&color%5B%5D=blue', query
     end
   end
 
@@ -638,7 +655,7 @@ class TestRequestParams < Faraday::TestCase
     with_default_params_encoder(Faraday::FlatParamsEncoder) do
       create_connection 'http://a.co/page1?color=red&color=blue'
       query = get
-      assert_equal "color=red&color=blue", query
+      assert_equal 'color=red&color=blue', query
     end
   end
 
@@ -646,7 +663,7 @@ class TestRequestParams < Faraday::TestCase
     with_default_params_encoder(Faraday::FlatParamsEncoder) do
       create_connection 'http://a.co/page1', :params => {:color => ['red', 'blue']}
       query = get
-      assert_equal "color=red&color=blue", query
+      assert_equal 'color=red&color=blue', query
     end
   end
 
@@ -660,7 +677,7 @@ class TestRequestParams < Faraday::TestCase
     query = get('', :feeling => 'blue') do |req|
       req.options.params_encoder = encoder
     end
-    assert_equal ["COLOR-RED", "FEELING-BLUE"], query.split(",").sort
+    assert_equal ['COLOR-RED', 'FEELING-BLUE'], query.split(',').sort
   end
 
   def get(*args)

test/env_test.rb

@@ -9,7 +9,7 @@ class EnvTest < Faraday::TestCase
     @conn.options.timeout      = 3
     @conn.options.open_timeout = 5
     @conn.ssl.verify           = false
-    @conn.proxy 'http://proxy.com'
+    @conn.proxy = 'http://proxy.com'
   end
 
   def test_request_create_stores_method
@@ -78,11 +78,6 @@ class EnvTest < Faraday::TestCase
     assert_equal false, env.ssl.verify
   end
 
-  def test_request_create_stores_proxy_options
-    env = make_env
-    assert_equal 'proxy.com', env.request.proxy.host
-  end
-
   def test_custom_members_are_retained
     env = make_env
     env[:foo] = "custom 1"