From d18cc04be5c179b1b22b9dd1004030dd43b0ca58 Mon Sep 17 00:00:00 2001 From: Mattia Date: Mon, 24 Jul 2017 10:14:52 +0100 Subject: [PATCH] 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 | 56 ++++++++----- test/connection_test.rb | 163 +++++++++++++++++++++----------------- test/env_test.rb | 7 +- 3 files changed, 128 insertions(+), 98 deletions(-) diff --git a/lib/faraday/connection.rb b/lib/faraday/connection.rb index ae7bcd8a..02dce2e1 100644 --- a/lib/faraday/connection.rb +++ b/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 diff --git a/test/connection_test.rb b/test/connection_test.rb index 69cde6ea..2422f804 100644 --- a/test/connection_test.rb +++ b/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) diff --git a/test/env_test.rb b/test/env_test.rb index 146bc330..5360275e 100644 --- a/test/env_test.rb +++ b/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"