Merge branch 'fix-no-proxy' into 'master'

fix proxy discovery using proxy env vars

See merge request os85/httpx!242

lib/httpx/plugins/proxy.rb

@@ -18,6 +18,43 @@ module HTTPX
       Error = HTTPProxyError
       PROXY_ERRORS = [TimeoutError, IOError, SystemCallError, Error].freeze
 
+      class << self
+        def configure(klass)
+          klass.plugin(:"proxy/http")
+          klass.plugin(:"proxy/socks4")
+          klass.plugin(:"proxy/socks5")
+        end
+
+        if URI::Generic.methods.include?(:use_proxy?)
+          def use_proxy?(*args)
+            URI::Generic.use_proxy?(*args)
+          end
+        else
+          # https://github.com/ruby/uri/blob/ae07f956a4bea00b4f54a75bd40b8fa918103eed/lib/uri/generic.rb
+          def use_proxy?(hostname, addr, port, no_proxy)
+            hostname = hostname.downcase
+            dothostname = ".#{hostname}"
+            no_proxy.scan(/([^:,\s]+)(?::(\d+))?/) do |p_host, p_port|
+              if !p_port || port == p_port.to_i
+                if p_host.start_with?(".")
+                  return false if hostname.end_with?(p_host.downcase)
+                else
+                  return false if dothostname.end_with?(".#{p_host.downcase}")
+                end
+                if addr
+                  begin
+                    return false if IPAddr.new(p_host).include?(addr)
+                  rescue IPAddr::InvalidAddressError
+                    next
+                  end
+                end
+              end
+            end
+            true
+          end
+        end
+      end
+
       class Parameters
         attr_reader :uri, :username, :password, :scheme
 
@@ -77,14 +114,6 @@ module HTTPX
         end
       end
 
-      class << self
-        def configure(klass)
-          klass.plugin(:"proxy/http")
-          klass.plugin(:"proxy/socks4")
-          klass.plugin(:"proxy/socks5")
-        end
-      end
-
       module OptionsMethods
         def option_proxy(value)
           value.is_a?(Parameters) ? value : Hash[value]
@@ -94,34 +123,39 @@ module HTTPX
       module InstanceMethods
         private
 
-        def proxy_uris(uri, options)
-          @_proxy_uris ||= begin
-            uris = options.proxy ? Array(options.proxy[:uri]) : []
-            if uris.empty?
-              uri = URI(uri).find_proxy
-              uris << uri if uri
-            end
-            uris
-          end
-          return if @_proxy_uris.empty?
-
-          proxy = options.proxy
-
-          return { uri: uri.host } if proxy && proxy.key?(:no_proxy) && !Array(proxy[:no_proxy]).grep(uri.host).empty?
-
-          proxy_opts = { uri: @_proxy_uris.first }
-          proxy_opts = options.proxy.merge(proxy_opts) if options.proxy
-          proxy_opts
-        end
-
         def find_connection(request, connections, options)
           return super unless options.respond_to?(:proxy)
 
           uri = URI(request.uri)
-          next_proxy = proxy_uris(uri, options)
-          raise Error, "Failed to connect to proxy" unless next_proxy
 
-          proxy = Parameters.new(**next_proxy) unless next_proxy[:uri] == uri.host
+          proxy_opts = if (next_proxy = uri.find_proxy)
+            { uri: next_proxy }
+          else
+            proxy = options.proxy
+
+            return super unless proxy
+
+            return super(request, connections, options.merge(proxy: nil)) unless proxy.key?(:uri)
+
+            @_proxy_uris ||= Array(proxy[:uri])
+
+            next_proxy = @_proxy_uris.first
+            raise Error, "Failed to connect to proxy" unless next_proxy
+
+            if proxy.key?(:no_proxy)
+              next_proxy = URI(next_proxy)
+
+              no_proxy = proxy[:no_proxy]
+              no_proxy = no_proxy.join(",") if no_proxy.is_a?(Array)
+
+              return super(request, connections, options.merge(proxy: nil)) unless Proxy.use_proxy?(uri.host, next_proxy.host,
+                                                                                                    next_proxy.port, no_proxy)
+            end
+
+            proxy.merge(uri: next_proxy)
+          end
+
+          proxy = Parameters.new(**proxy_opts)
 
           proxy_options = options.merge(proxy: proxy)
           connection = pool.find_connection(uri, proxy_options) || build_connection(uri, proxy_options)

sig/plugins/proxy.rbs

@@ -35,11 +35,7 @@ module HTTPX
       def self.extra_options: (Options) -> (Options & _ProxyOptions)
 
       module InstanceMethods
-        private
-
-        def proxy_uris: (generic_uri, Options & _ProxyOptions) -> { uri: generic_uri, username: String, password: String }
-                      | (generic_uri, Options & _ProxyOptions) -> { uri: generic_uri }
-                      | (generic_uri, Options & _ProxyOptions) -> nil
+        @__proxy_uris: Array[generic_uri]
       end
     end
 

standalone_tests/env_proxy_test.rb

@@ -1,9 +1,11 @@
 # frozen_string_literal: true
 
 HTTP_PROXY = ENV["HTTPX_HTTP_PROXY"]
-ENV["HTTP_PROXY"] = HTTP_PROXY
+ENV["http_proxy"] = HTTP_PROXY
 HTTPS_PROXY = ENV["HTTPX_HTTPS_PROXY"]
-ENV["HTTPS_PROXY"] = HTTPS_PROXY
+ENV["https_proxy"] = HTTPS_PROXY
+NO_PROXY = ENV["HTTPBIN_NO_PROXY_HOST"]
+ENV["no_proxy"] = URI(NO_PROXY).authority
 
 require "test_helper"
 require "support/http_helpers"
@@ -13,6 +15,22 @@ class EnvProxyTest < Minitest::Test
   include HTTPHelpers
   using HTTPX::URIExtensions
 
+  def test_plugin_http_no_proxy
+    HTTPX.plugin(SessionWithPool).plugin(ProxyResponseDetector).wrap do |session|
+      # proxy
+      response = session.get("https://#{httpbin}/get")
+      verify_status(response, 200)
+      verify_body_length(response)
+      assert response.proxied?
+
+      # no proxy
+      no_proxy_response = session.get("#{NO_PROXY}/get")
+      verify_status(no_proxy_response, 200)
+      verify_body_length(no_proxy_response)
+      assert !no_proxy_response.proxied?
+    end
+  end
+
   def test_env_proxy_coalescing
     HTTPX.plugin(SessionWithPool).wrap do |session|
       response = session.get("https://#{httpbin}/get")

test/http_test.rb

@@ -100,7 +100,7 @@ class HTTPTest < Minitest::Test
 
   private
 
-  def origin(orig = httpbin)
-    "http://#{orig}"
+  def scheme
+    "http://"
   end
 end

test/https_test.rb

@@ -180,7 +180,7 @@ class HTTPSTest < Minitest::Test
 
   private
 
-  def origin(orig = httpbin)
-    "https://#{orig}"
+  def scheme
+    "https://"
   end
 end

test/support/http_helpers.rb

@@ -23,6 +23,10 @@ module HTTPHelpers
   end
 
   def httpbin_no_proxy
-    URI(ENV.fetch("HTTPBIN_NO_PROXY_HOST", "httpbin.org"))
+    URI(ENV.fetch("HTTPBIN_NO_PROXY_HOST", "#{scheme}httpbin.org"))
+  end
+
+  def origin(orig = httpbin)
+    "#{scheme}#{orig}"
   end
 end