Merge pull request #243 from astratto/connect_timeout

Set a connect_timeout for the creation of a socket

Author: jch

lib/net/ber.rb

@@ -293,24 +293,24 @@ def to_arr
 
 ##
 # A String object with a BER identifier attached.
-# 
+#
 class Net::BER::BerIdentifiedString < String
   attr_accessor :ber_identifier
 
   # The binary data provided when parsing the result of the LDAP search
   # has the encoding 'ASCII-8BIT' (which is basically 'BINARY', or 'unknown').
-  # 
+  #
   # This is the kind of a backtrace showing how the binary `data` comes to
   # BerIdentifiedString.new(data):
   #
   #  @conn.read_ber(syntax)
   #     -> StringIO.new(self).read_ber(syntax), i.e. included from module
-  #     -> Net::BER::BERParser.read_ber(syntax) 
+  #     -> Net::BER::BERParser.read_ber(syntax)
   #        -> (private)Net::BER::BERParser.parse_ber_object(syntax, id, data)
-  # 
+  #
   # In the `#parse_ber_object` method `data`, according to its OID, is being
   # 'casted' to one of the Net::BER:BerIdentifiedXXX classes.
-  # 
+  #
   # As we are using LDAP v3 we can safely assume that the data is encoded
   # in UTF-8 and therefore the only thing to be done when instantiating is to
   # switch the encoding from 'ASCII-8BIT' to 'UTF-8'.
@@ -322,9 +322,9 @@ class Net::BER::BerIdentifiedString < String
   # I have no clue how this encodings function.
   def initialize args
     super
-    # 
+    #
     # Check the encoding of the newly created String and set the encoding
-    # to 'UTF-8' (NOTE: we do NOT change the bytes, but only set the 
+    # to 'UTF-8' (NOTE: we do NOT change the bytes, but only set the
     # encoding to 'UTF-8').
     current_encoding = encoding
     if current_encoding == Encoding::BINARY

lib/net/ldap.rb

@@ -79,6 +79,14 @@ class LDAP
 #
 #  p ldap.get_operation_result
 #
+# === Setting connect timeout
+#
+# By default, Net::LDAP uses TCP sockets with a connection timeout of 5 seconds.
+#
+# This value can be tweaked passing the :connect_timeout parameter.
+# i.e.
+#  ldap = Net::LDAP.new ...,
+#                       :connect_timeout => 3
 #
 # == A Brief Introduction to LDAP
 #
@@ -487,22 +495,22 @@ def self.result2string(code) #:nodoc:
   #   The :start_tls like the :simple_tls encryption method also encrypts all
   #   communcations with the LDAP server. With the exception that it operates
   #   over the standard TCP port.
-  #  
+  #
   #   In order to verify certificates and enable other TLS options, the
   #   :tls_options hash can be passed alongside :simple_tls or :start_tls.
   #   This hash contains any options that can be passed to
   #   OpenSSL::SSL::SSLContext#set_params(). The most common options passed
   #   should be OpenSSL::SSL::SSLContext::DEFAULT_PARAMS, or the :ca_file option,
   #   which contains a path to a Certificate Authority file (PEM-encoded).
-  #  
+  #
   #   Example for a default setup without custom settings:
   #     {
   #       :method => :simple_tls,
   #       :tls_options => OpenSSL::SSL::SSLContext::DEFAULT_PARAMS
   #     }
-  #  
+  #
   #   Example for specifying a CA-File and only allowing TLSv1.1 connections:
-  #  
+  #
   #     {
   #       :method => :start_tls,
   #       :tls_options => { :ca_file => "/etc/cafile.pem", :ssl_version => "TLSv1_1" }
@@ -524,6 +532,7 @@ def initialize(args = {})
     @base = args[:base] || DefaultTreebase
     @force_no_page = args[:force_no_page] || DefaultForceNoPage
     @encryption = args[:encryption] # may be nil
+    @connect_timeout = args[:connect_timeout]
 
     if pr = @auth[:password] and pr.respond_to?(:call)
       @auth[:password] = pr.call
@@ -587,7 +596,7 @@ def authenticate(username, password)
   # additional capabilities are added, more configuration values will be
   # added here.
   #
-  # This method is deprecated. 
+  # This method is deprecated.
   #
   def encryption(args)
     warn "Deprecation warning: please give :encryption option as a Hash to Net::LDAP.new"
@@ -1247,8 +1256,9 @@ def new_connection
       :port                    => @port,
       :hosts                   => @hosts,
       :encryption              => @encryption,
-      :instrumentation_service => @instrumentation_service
-  rescue Errno::ECONNREFUSED, Net::LDAP::ConnectionRefusedError => e
+      :instrumentation_service => @instrumentation_service,
+      :connect_timeout         => @connect_timeout
+  rescue Errno::ECONNREFUSED, Errno::ETIMEDOUT, Net::LDAP::ConnectionRefusedError => e
     @result = {
       :resultCode   => 52,
       :errorMessage => ResultStrings[ResultCodeUnavailable]

lib/net/ldap/connection.rb

@@ -3,6 +3,9 @@
 class Net::LDAP::Connection #:nodoc:
   include Net::LDAP::Instrumentation
 
+  # Seconds before failing for socket connect timeout
+  DefaultConnectTimeout = 5
+
   LdapVersion = 3
   MaxSaslChallenges = 10
 
@@ -31,10 +34,14 @@ def open_connection(server)
     hosts = server[:hosts]
     encryption = server[:encryption]
 
+    socket_opts = {
+      connect_timeout: server[:connect_timeout] || DefaultConnectTimeout
+    }
+
     errors = []
     hosts.each do |host, port|
       begin
-        prepare_socket(server.merge(socket: TCPSocket.new(host, port)))
+        prepare_socket(server.merge(socket: Socket.tcp(host, port, socket_opts)))
         return
       rescue Net::LDAP::Error, SocketError, SystemCallError,
              OpenSSL::SSL::SSLError => e

script/install-openldap

@@ -109,4 +109,7 @@ chgrp ssl-cert /etc/ssl/private/ldap01_slapd_key.pem
 chmod g+r /etc/ssl/private/ldap01_slapd_key.pem
 chmod o-r /etc/ssl/private/ldap01_slapd_key.pem
 
+# Drop packets on a secondary port used to specific timeout tests
+iptables -A OUTPUT -p tcp -j DROP --dport 8389
+
 service slapd restart

test/ber/test_ber.rb

@@ -130,11 +130,11 @@ def test_binary_data
   def test_ascii_data_in_utf8
     data = "some text".force_encoding("UTF-8")
     bis = Net::BER::BerIdentifiedString.new(data)
-    
+
     assert bis.valid_encoding?, "should be a valid encoding"
     assert_equal "UTF-8", bis.encoding.name
   end
-  
+
   def test_umlaut_data_in_utf8
     data = "Müller".force_encoding("UTF-8")
     bis = Net::BER::BerIdentifiedString.new(data)

test/integration/test_bind.rb

@@ -5,6 +5,14 @@ def test_bind_success
     assert @ldap.bind(method: :simple, username: "uid=user1,ou=People,dc=rubyldap,dc=com", password: "passworD1"), @ldap.get_operation_result.inspect
   end
 
+  def test_bind_timeout
+    @ldap.port = 8389
+    error = assert_raise Net::LDAP::Error do
+      @ldap.bind(method: :simple, username: "uid=user1,ou=People,dc=rubyldap,dc=com", password: "passworD1")
+    end
+    assert_equal('Connection timed out - user specified timeout', error.message)
+  end
+
   def test_bind_anonymous_fail
     refute @ldap.bind(method: :simple, username: "uid=user1,ou=People,dc=rubyldap,dc=com", password: ""), @ldap.get_operation_result.inspect
 

test/test_auth_adapter.rb

@@ -2,7 +2,8 @@
 
 class TestAuthAdapter < Test::Unit::TestCase
   def test_undefined_auth_adapter
-    flexmock(TCPSocket).should_receive(:new).ordered.with('ldap.example.com', 379).once.and_return(nil)
+    flexmock(Socket).should_receive(:tcp).ordered.with('ldap.example.com', 379, { connect_timeout: 5 }).once.and_return(nil)
+
     conn = Net::LDAP::Connection.new(host: 'ldap.example.com', port: 379)
     assert_raise Net::LDAP::AuthMethodUnsupportedError, "Unsupported auth method (foo)" do
       conn.bind(method: :foo)

test/test_ldap_connection.rb

@@ -15,8 +15,8 @@ def test_list_of_hosts_with_first_host_successful
       ['test2.mocked.com', 636],
       ['test3.mocked.com', 636],
     ]
-    flexmock(TCPSocket).should_receive(:new).ordered.with(*hosts[0]).once.and_return(nil)
-    flexmock(TCPSocket).should_receive(:new).ordered.never
+    flexmock(Socket).should_receive(:tcp).ordered.with(*hosts[0], { connect_timeout: 5 }).once.and_return(nil)
+    flexmock(Socket).should_receive(:tcp).ordered.never
     Net::LDAP::Connection.new(:hosts => hosts)
   end
 
@@ -26,9 +26,9 @@ def test_list_of_hosts_with_first_host_failure
       ['test2.mocked.com', 636],
       ['test3.mocked.com', 636],
     ]
-    flexmock(TCPSocket).should_receive(:new).ordered.with(*hosts[0]).once.and_raise(SocketError)
-    flexmock(TCPSocket).should_receive(:new).ordered.with(*hosts[1]).once.and_return(nil)
-    flexmock(TCPSocket).should_receive(:new).ordered.never
+    flexmock(Socket).should_receive(:tcp).ordered.with(*hosts[0], { connect_timeout: 5 }).once.and_raise(SocketError)
+    flexmock(Socket).should_receive(:tcp).ordered.with(*hosts[1], { connect_timeout: 5 }).once.and_return(nil)
+    flexmock(Socket).should_receive(:tcp).ordered.never
     Net::LDAP::Connection.new(:hosts => hosts)
   end
 
@@ -38,17 +38,17 @@ def test_list_of_hosts_with_all_hosts_failure
       ['test2.mocked.com', 636],
       ['test3.mocked.com', 636],
     ]
-    flexmock(TCPSocket).should_receive(:new).ordered.with(*hosts[0]).once.and_raise(SocketError)
-    flexmock(TCPSocket).should_receive(:new).ordered.with(*hosts[1]).once.and_raise(SocketError)
-    flexmock(TCPSocket).should_receive(:new).ordered.with(*hosts[2]).once.and_raise(SocketError)
-    flexmock(TCPSocket).should_receive(:new).ordered.never
+    flexmock(Socket).should_receive(:tcp).ordered.with(*hosts[0], { connect_timeout: 5 }).once.and_raise(SocketError)
+    flexmock(Socket).should_receive(:tcp).ordered.with(*hosts[1], { connect_timeout: 5 }).once.and_raise(SocketError)
+    flexmock(Socket).should_receive(:tcp).ordered.with(*hosts[2], { connect_timeout: 5 }).once.and_raise(SocketError)
+    flexmock(Socket).should_receive(:tcp).ordered.never
     assert_raise Net::LDAP::ConnectionError do
       Net::LDAP::Connection.new(:hosts => hosts)
     end
   end
 
   def test_result_for_connection_failed_is_set
-    flexmock(TCPSocket).should_receive(:new).and_raise(Errno::ECONNREFUSED)
+    flexmock(Socket).should_receive(:tcp).and_raise(Errno::ECONNREFUSED)
 
     ldap_client = Net::LDAP.new(host: '127.0.0.1', port: 12345)
 
@@ -67,14 +67,14 @@ def test_unresponsive_host
   end
 
   def test_blocked_port
-    flexmock(TCPSocket).should_receive(:new).and_raise(SocketError)
+    flexmock(Socket).should_receive(:tcp).and_raise(SocketError)
     assert_raise Net::LDAP::Error do
       Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636)
     end
   end
 
   def test_connection_refused
-    flexmock(TCPSocket).should_receive(:new).and_raise(Errno::ECONNREFUSED)
+    flexmock(Socket).should_receive(:tcp).and_raise(Errno::ECONNREFUSED)
     stderr = capture_stderr do
       assert_raise Net::LDAP::ConnectionRefusedError do
         Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636)
@@ -83,9 +83,18 @@ def test_connection_refused
     assert_equal("Deprecation warning: Net::LDAP::ConnectionRefused will be deprecated. Use Errno::ECONNREFUSED instead.\n",  stderr)
   end
 
+  def test_connection_timedout
+    flexmock(Socket).should_receive(:tcp).and_raise(Errno::ETIMEDOUT)
+    stderr = capture_stderr do
+      assert_raise Net::LDAP::Error do
+        Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636)
+      end
+    end
+  end
+
   def test_raises_unknown_exceptions
     error = Class.new(StandardError)
-    flexmock(TCPSocket).should_receive(:new).and_raise(error)
+    flexmock(Socket).should_receive(:tcp).and_raise(error)
     assert_raise error do
       Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636)
     end
@@ -328,7 +337,7 @@ class TestLDAPConnectionErrors < Test::Unit::TestCase
   def setup
     @tcp_socket = flexmock(:connection)
     @tcp_socket.should_receive(:write)
-    flexmock(TCPSocket).should_receive(:new).and_return(@tcp_socket)
+    flexmock(Socket).should_receive(:tcp).and_return(@tcp_socket)
     @connection = Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636)
   end
 
@@ -357,7 +366,7 @@ class TestLDAPConnectionInstrumentation < Test::Unit::TestCase
   def setup
     @tcp_socket = flexmock(:connection)
     @tcp_socket.should_receive(:write)
-    flexmock(TCPSocket).should_receive(:new).and_return(@tcp_socket)
+    flexmock(Socket).should_receive(:tcp).and_return(@tcp_socket)
 
     @service = MockInstrumentationService.new
     @connection = Net::LDAP::Connection.new \