Add support for NO_PROXY env variable (#355)

Author: bwilczek

lib/em-http/http_connection_options.rb

@@ -22,7 +22,7 @@ def initialize(uri, options)
     uri.port ||= (@https ? 443 : 80)
     @tls[:sni_hostname] = uri.hostname
 
-    @proxy = options[:proxy] || proxy_from_env
+    @proxy = options[:proxy] || proxy_from_env(uri.hostname)
 
     if proxy
       @host = proxy[:host]
@@ -45,8 +45,7 @@ def socks_proxy?
     @proxy && (@proxy[:type] == :socks5)
   end
 
-  def proxy_from_env
-    # TODO: Add support for $http_no_proxy or $no_proxy ?
+  def proxy_from_env(host)
     proxy_str = if @https
                   ENV['HTTPS_PROXY'] || ENV['https_proxy']
                 else
@@ -59,6 +58,9 @@ def proxy_from_env
     # so, let's short-circuit that:
     return if !proxy_str || proxy_str.empty?
 
+    no_proxy_hosts = (ENV['NO_PROXY'] || ENV['no_proxy'] || '').split(',')
+    return if use_env_proxy_for_host?(no_proxy_hosts, host)
+
     proxy_env_uri = Addressable::URI::parse(proxy_str)
     { :host => proxy_env_uri.host, :port => proxy_env_uri.port, :type => :http }
 
@@ -67,4 +69,14 @@ def proxy_from_env
     # We should somehow log / warn about this, perhaps...
     return
   end
+
+  private
+
+  def use_env_proxy_for_host?(no_proxy_hosts, host)
+    return true if no_proxy_hosts.include?(host)
+
+    no_proxy_hosts
+      .select { |h| h.start_with?('.') }
+      .any? { |w| host.end_with?(w) }
+  end
 end

spec/http_proxy_spec.rb

@@ -17,8 +17,10 @@
 end
 
 shared_examples "*_PROXY var (testing var)" do
-  subject { HttpConnectionOptions.new("#{proxy_test_scheme}://example.com", {}) }
-  it { expect(subject.proxy_from_env).to eq({ :host => "127.0.0.1", :port => 8083, :type => :http }) }
+  subject { HttpConnectionOptions.new("#{proxy_test_scheme}://#{request_host}", {}) }
+  let(:request_host) { 'example.com' }
+
+  it { expect(subject.proxy_from_env(request_host)).to eq({ :host => "127.0.0.1", :port => 8083, :type => :http }) }
   it { expect(subject.host).to eq "127.0.0.1" }
   it { expect(subject.port).to be 8083 }
   it do
@@ -201,10 +203,10 @@
       before(:all) do
         PROXY_ENV_VARS.each {|k| ENV.delete k }
       end
-
-      subject { HttpConnectionOptions.new("http://example.com", {}) }
-      it { expect(subject.proxy_from_env).to be_nil }
-      it { expect(subject.host).to eq "example.com" }
+      let(:request_host) { 'example.com' }
+      subject { HttpConnectionOptions.new("http://#{request_host}", {}) }
+      it { expect(subject.proxy_from_env(request_host)).to be_nil }
+      it { expect(subject.host).to eq request_host }
       it { expect(subject.port).to be 80 }
       it { expect(subject.http_proxy?).to be_falsey }
       it { expect(subject.connect_proxy?).to be_falsey }
@@ -264,5 +266,60 @@
 
       include_examples "*_PROXY var (testing var)"
     end
+
+    context 'with $NO_PROXY env' do
+      let(:request_host) { 'ignore.me' }
+      let(:no_proxy_hosts) { ['host1', request_host, 'host2'] }
+      subject { HttpConnectionOptions.new("http://#{request_host}", {}) }
+
+      before(:each) do
+        PROXY_ENV_VARS.each { |k| ENV.delete k }
+        ENV['ALL_PROXY'] = 'http://127.0.0.1:8083'
+        ENV[no_proxy_var_name] = no_proxy_hosts&.join(',')
+      end
+
+      describe 'when $NO_PROXY includes host from current request' do
+        let(:no_proxy_var_name) { 'NO_PROXY' }
+
+        it 'should not apply proxy from env' do
+          expect(subject.proxy_from_env(request_host)).to be_nil
+        end
+      end
+
+      describe 'when $NO_PROXY does not include host from current request' do
+        let(:no_proxy_var_name) { 'NO_PROXY' }
+        let(:no_proxy_hosts) { %w[host1 host2] }
+
+        it 'should apply proxy from env' do
+          expect(subject.proxy_from_env(request_host)).not_to be_nil
+        end
+      end
+
+      describe 'when $no_proxy includes host from current request' do
+        let(:no_proxy_var_name) { 'no_proxy' }
+
+        it 'should not apply proxy from env' do
+          expect(subject.proxy_from_env(request_host)).to be_nil
+        end
+      end
+
+      describe 'when $no_proxy does not include host from current request' do
+        let(:no_proxy_var_name) { 'no_proxy' }
+        let(:no_proxy_hosts) { %w[host1 host2] }
+
+        it 'should apply proxy from env' do
+          expect(subject.proxy_from_env(request_host)).not_to be_nil
+        end
+      end
+
+      describe 'when .domain is set in $no_proxy' do
+        let(:no_proxy_var_name) { 'no_proxy' }
+        let(:no_proxy_hosts) { %w[host1 .me host2] }
+
+        it 'should not apply proxy from env' do
+          expect(subject.proxy_from_env(request_host)).to be_nil
+        end
+      end
+    end
   end
 end

spec/spec_helper.rb

@@ -1,4 +1,4 @@
-PROXY_ENV_VARS = %w[HTTP_PROXY http_proxy HTTPS_PROXY https_proxy ALL_PROXY]
+PROXY_ENV_VARS = %w[HTTP_PROXY http_proxy HTTPS_PROXY https_proxy ALL_PROXY NO_PROXY no_proxy].freeze
 
 RSpec.configure do |config|
   proxy_envs = {}