Deprecate omit_default_port option, add include_default_port option

README.md

@@ -132,8 +132,8 @@ connection.request(:write_timeout => 360)
 #
 connection = Excon.new('http://geemus.com/', :tcp_nodelay => true)
 
-# opt-in to omitting port from http:80 and https:443
-connection = Excon.new('http://geemus.com/', :omit_default_port => true)
+# opt-in to having Excon add a default port (http:80 and https:443)
+connection = Excon.new('http://geemus.com/', :include_default_port => true)
 
 # set longer connect_timeout (default is 60 seconds)
 connection = Excon.new('http://geemus.com/', :connect_timeout => 360)

lib/excon/constants.rb

@@ -72,7 +72,7 @@ module Excon
     :connect_timeout,
     :family,
     :host,
-    :omit_default_port,
+    :include_default_port,
     :nonblock,
     :reuseaddr,
     :password,
@@ -125,7 +125,7 @@ module WaitWritable; end
     ],
     :mock                 => false,
     :nonblock             => true,
-    :omit_default_port    => false,
+    :include_default_port => false,
     :persistent           => false,
     :read_timeout         => 60,
     :retry_limit          => DEFAULT_RETRY_LIMIT,

lib/excon/ssl_socket.rb

@@ -85,7 +85,7 @@ def initialize(data = {})
       end
 
       if @data[:proxy]
-        request = 'CONNECT ' << @data[:host] << port_string(@data.merge(:omit_default_port => false)) << Excon::HTTP_1_1
+        request = 'CONNECT ' << @data[:host] << Excon::HTTP_1_1
         request << 'Host: ' << @data[:host] << port_string(@data) << Excon::CR_NL
 
         if @data[:proxy][:password] || @data[:proxy][:user]

lib/excon/utils.rb

@@ -25,13 +25,18 @@ def request_uri(datum)
     end
 
     def port_string(datum)
-      if datum[:port].nil? || (datum[:omit_default_port] && ((datum[:scheme].casecmp('http') == 0 && datum[:port] == 80) || (datum[:scheme].casecmp('https') == 0 && datum[:port] == 443)))
-        ''
-      else
+      if datum[:include_default_port] || !default_port?(datum)
         ':' << datum[:port].to_s
+      else
+        ''
       end
     end
 
+    def default_port?(datum)
+      (datum[:scheme].casecmp('http') == 0 && datum[:port] == 80) ||
+        (datum[:scheme].casecmp('https') == 0 && datum[:port] == 443)
+    end
+
     def query_string(datum)
       str = ''
       case datum[:query]

tests/utils_tests.rb

@@ -10,15 +10,15 @@
 
     tests('using HTTP scheme') do
 
-      expected_uri = 'http://foo.com:80'
-      tests('with default port').returns(expected_uri) do
+      expected_uri = 'http://foo.com'
+      tests('without default port').returns(expected_uri) do
         connection = Excon.new('http://foo.com/some/path')
         Excon::Utils.connection_uri(connection.data)
       end
 
-      expected_uri = 'http://foo.com'
+      expected_uri = 'http://foo.com:80'
       tests('without default port').returns(expected_uri) do
-        connection = Excon.new('http://foo.com/some/path', :omit_default_port => true)
+        connection = Excon.new('http://foo.com/some/path', :include_default_port => true)
         Excon::Utils.connection_uri(connection.data)
       end
 
@@ -48,21 +48,65 @@
 
     tests('using HTTP scheme') do
 
-      expected_uri = 'http://foo.com:80/some/path'
+      expected_uri = 'http://foo.com/some/path'
       tests('without query').returns(expected_uri) do
         connection = Excon.new('http://foo.com')
         params = { :path => '/some/path' }
         Excon::Utils.request_uri(connection.data.merge(params))
       end
 
-      expected_uri = 'http://foo.com:80/some/path?bar=that&foo=this'
+      expected_uri = 'http://foo.com/some/path?bar=that&foo=this'
       tests('with query').returns(expected_uri) do
         connection = Excon.new('http://foo.com')
         params = { :path => '/some/path', :query => { :foo => 'this', :bar => 'that' } }
         Excon::Utils.request_uri(connection.data.merge(params))
       end
-
     end
 
+    test('detecting default ports') do
+      tests('http default port').returns true do
+        datum = {
+          scheme: 'http',
+          port: 80
+        }
+
+        Excon::Utils.default_port?(datum)
+      end
+
+      tests('http nonstandard port').returns false do
+        datum = {
+          scheme: 'http',
+          port: 9292
+        }
+
+        Excon::Utils.default_port?(datum)
+      end
+
+      tests('https standard port').returns true do
+        datum = {
+          scheme: 'https',
+          port: 443
+        }
+
+        Excon::Utils.default_port?(datum)
+      end
+
+      tests('https nonstandard port').returns false do
+        datum = {
+          scheme: 'https',
+          port: 8443
+        }
+
+        Excon::Utils.default_port?(datum)
+      end
+
+      tests('unix socket').returns false do
+        datum = {
+          scheme: 'unix'
+        }
+
+        Excon::Utils.default_port?(datum)
+      end
+    end
   end
 end