diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 455bbd6e..20a98934 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -1275,6 +1275,11 @@ def inspect inspected end + # Internal: Set @open_connection for testing + def connection=(connection) + @open_connection = connection + end + private # Yields an open connection if there is one, otherwise establishes a new @@ -1300,13 +1305,17 @@ def use_connection(args) # Establish a new connection to the LDAP server def new_connection - Net::LDAP::Connection.new \ + connection = Net::LDAP::Connection.new \ :host => @host, :port => @port, :hosts => @hosts, :encryption => @encryption, :instrumentation_service => @instrumentation_service, :connect_timeout => @connect_timeout + + # Force connect to see if there's a connection error + connection.socket + connection rescue Errno::ECONNREFUSED, Errno::ETIMEDOUT, Net::LDAP::ConnectionRefusedError => e @result = { :resultCode => 52, diff --git a/lib/net/ldap/connection.rb b/lib/net/ldap/connection.rb index 67757323..39cfd970 100644 --- a/lib/net/ldap/connection.rb +++ b/lib/net/ldap/connection.rb @@ -9,19 +9,28 @@ class Net::LDAP::Connection #:nodoc: LdapVersion = 3 MaxSaslChallenges = 10 - def initialize(server) + # Initialize a connection to an LDAP server + # + # :server + # :hosts Array of tuples specifying host, port + # :host host + # :port port + # :socket prepared socket + # + def initialize(server = {}) + @server = server @instrumentation_service = server[:instrumentation_service] - if server[:socket] - prepare_socket(server) - else - server[:hosts] = [[server[:host], server[:port]]] if server[:hosts].nil? - open_connection(server) - end + # Allows tests to parameterize what socket class to use + @socket_class = server.fetch(:socket_class, DefaultSocket) yield self if block_given? end + def socket_class=(socket_class) + @socket_class = socket_class + end + def prepare_socket(server) socket = server[:socket] encryption = server[:encryption] @@ -41,7 +50,7 @@ def open_connection(server) errors = [] hosts.each do |host, port| begin - prepare_socket(server.merge(socket: Socket.tcp(host, port, socket_opts))) + prepare_socket(server.merge(socket: @socket_class.new(host, port, socket_opts))) return rescue Net::LDAP::Error, SocketError, SystemCallError, OpenSSL::SSL::SSLError => e @@ -202,7 +211,7 @@ def message_queue def read(syntax = Net::LDAP::AsnSyntax) ber_object = instrument "read.net_ldap_connection", :syntax => syntax do |payload| - @conn.read_ber(syntax) do |id, content_length| + socket.read_ber(syntax) do |id, content_length| payload[:object_type_id] = id payload[:content_length] = content_length end @@ -232,7 +241,7 @@ def read(syntax = Net::LDAP::AsnSyntax) def write(request, controls = nil, message_id = next_msgid) instrument "write.net_ldap_connection" do |payload| packet = [message_id.to_ber, request, controls].compact.to_ber_sequence - payload[:content_length] = @conn.write(packet) + payload[:content_length] = socket.write(packet) end end private :write @@ -652,4 +661,33 @@ def delete(args) pdu end + + # Internal: Returns a Socket like object used internally to communicate with + # LDAP server. + # + # Typically a TCPSocket, but can be a OpenSSL::SSL::SSLSocket + def socket + return @conn if defined? @conn + + # First refactoring uses the existing methods open_connection and + # prepare_socket to set @conn. Next cleanup would centralize connection + # handling here. + if @server[:socket] + prepare_socket(@server) + else + @server[:hosts] = [[@server[:host], @server[:port]]] if @server[:hosts].nil? + open_connection(@server) + end + + @conn + end + + private + + # Wrap around Socket.tcp to normalize with other Socket initializers + class DefaultSocket + def self.new(host, port, socket_opts = {}) + Socket.tcp(host, port, socket_opts) + end + end end # class Connection diff --git a/test/test_auth_adapter.rb b/test/test_auth_adapter.rb index badde0fb..9e4c6002 100644 --- a/test/test_auth_adapter.rb +++ b/test/test_auth_adapter.rb @@ -1,10 +1,13 @@ require 'test_helper' class TestAuthAdapter < Test::Unit::TestCase - def test_undefined_auth_adapter - flexmock(Socket).should_receive(:tcp).ordered.with('ldap.example.com', 379, { connect_timeout: 5 }).once.and_return(nil) + class FakeSocket + def initialize(*args) + end + end - conn = Net::LDAP::Connection.new(host: 'ldap.example.com', port: 379) + def test_undefined_auth_adapter + conn = Net::LDAP::Connection.new(host: 'ldap.example.com', port: 379, :socket_class => FakeSocket) assert_raise Net::LDAP::AuthMethodUnsupportedError, "Unsupported auth method (foo)" do conn.bind(method: :foo) end diff --git a/test/test_ldap.rb b/test/test_ldap.rb index 0c241f69..85325457 100644 --- a/test/test_ldap.rb +++ b/test/test_ldap.rb @@ -1,6 +1,28 @@ require 'test_helper' class TestLDAPInstrumentation < Test::Unit::TestCase + # Fake Net::LDAP::Connection for testing + class FakeConnection + # It's difficult to instantiate Net::LDAP::PDU objects. Faking out what we + # need here until that object is brought under test and has it's constructor + # cleaned up. + class Result < Struct.new(:success?, :result_code); end + + def initialize + @bind_success = Result.new(true, Net::LDAP::ResultCodeSuccess) + @search_success = Result.new(true, Net::LDAP::ResultCodeSizeLimitExceeded) + end + + def bind(args = {}) + @bind_success + end + + def search(*args) + yield @search_success if block_given? + @search_success + end + end + def setup @connection = flexmock(:connection, :close => true) flexmock(Net::LDAP::Connection).should_receive(:new).and_return(@connection) @@ -15,8 +37,9 @@ def setup def test_instrument_bind events = @service.subscribe "bind.net_ldap" - bind_result = flexmock(:bind_result, :success? => true) - flexmock(@connection).should_receive(:bind).with(Hash).and_return(bind_result) + fake_connection = FakeConnection.new + @subject.connection = fake_connection + bind_result = fake_connection.bind assert @subject.bind @@ -28,10 +51,9 @@ def test_instrument_bind def test_instrument_search events = @service.subscribe "search.net_ldap" - flexmock(@connection).should_receive(:bind).and_return(flexmock(:bind_result, :result_code => Net::LDAP::ResultCodeSuccess)) - flexmock(@connection).should_receive(:search).with(Hash, Proc). - yields(entry = Net::LDAP::Entry.new("uid=user1,ou=users,dc=example,dc=com")). - and_return(flexmock(:search_result, :success? => true, :result_code => Net::LDAP::ResultCodeSuccess)) + fake_connection = FakeConnection.new + @subject.connection = fake_connection + entry = fake_connection.search refute_nil @subject.search(:filter => "(uid=user1)") @@ -44,10 +66,9 @@ def test_instrument_search def test_instrument_search_with_size events = @service.subscribe "search.net_ldap" - flexmock(@connection).should_receive(:bind).and_return(flexmock(:bind_result, :result_code => Net::LDAP::ResultCodeSuccess)) - flexmock(@connection).should_receive(:search).with(Hash, Proc). - yields(entry = Net::LDAP::Entry.new("uid=user1,ou=users,dc=example,dc=com")). - and_return(flexmock(:search_result, :success? => true, :result_code => Net::LDAP::ResultCodeSizeLimitExceeded)) + fake_connection = FakeConnection.new + @subject.connection = fake_connection + entry = fake_connection.search refute_nil @subject.search(:filter => "(uid=user1)", :size => 1) diff --git a/test/test_ldap_connection.rb b/test/test_ldap_connection.rb index 727b82a4..51e30c3f 100644 --- a/test/test_ldap_connection.rb +++ b/test/test_ldap_connection.rb @@ -9,44 +9,55 @@ def capture_stderr $stderr = stderr end + # Fake socket for testing + # + # FakeTCPSocket.new("success", 636) + # FakeTCPSocket.new("fail.SocketError", 636) # raises SocketError + class FakeTCPSocket + def initialize(host, port, socket_opts = {}) + status, error = host.split(".") + if status == "fail" + raise Object.const_get(error) + end + end + end + def test_list_of_hosts_with_first_host_successful hosts = [ - ['test.mocked.com', 636], - ['test2.mocked.com', 636], - ['test3.mocked.com', 636], + ["success.host", 636], + ["fail.SocketError", 636], + ["fail.SocketError", 636], ] - 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) + + connection = Net::LDAP::Connection.new(:hosts => hosts, :socket_class => FakeTCPSocket) + connection.socket end def test_list_of_hosts_with_first_host_failure hosts = [ - ['test.mocked.com', 636], - ['test2.mocked.com', 636], - ['test3.mocked.com', 636], + ["fail.SocketError", 636], + ["success.host", 636], + ["fail.SocketError", 636], ] - 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) + + connection = Net::LDAP::Connection.new(:hosts => hosts, :socket_class => FakeTCPSocket) + connection.socket end def test_list_of_hosts_with_all_hosts_failure hosts = [ - ['test.mocked.com', 636], - ['test2.mocked.com', 636], - ['test3.mocked.com', 636], + ["fail.SocketError", 636], + ["fail.SocketError", 636], + ["fail.SocketError", 636], ] - 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 + + connection = Net::LDAP::Connection.new(:hosts => hosts, :socket_class => FakeTCPSocket) assert_raise Net::LDAP::ConnectionError do - Net::LDAP::Connection.new(:hosts => hosts) + connection.socket end end + # This belongs in test_ldap, not test_ldap_connection def test_result_for_connection_failed_is_set flexmock(Socket).should_receive(:tcp).and_raise(Errno::ECONNREFUSED) @@ -61,42 +72,42 @@ def test_result_for_connection_failed_is_set end def test_unresponsive_host + connection = Net::LDAP::Connection.new(:host => "fail.Errno::ETIMEDOUT", :port => 636, :socket_class => FakeTCPSocket) assert_raise Net::LDAP::Error do - Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636) + connection.socket end end def test_blocked_port - flexmock(Socket).should_receive(:tcp).and_raise(SocketError) + connection = Net::LDAP::Connection.new(:host => "fail.SocketError", :port => 636, :socket_class => FakeTCPSocket) assert_raise Net::LDAP::Error do - Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636) + connection.socket end end def test_connection_refused - flexmock(Socket).should_receive(:tcp).and_raise(Errno::ECONNREFUSED) + connection = Net::LDAP::Connection.new(:host => "fail.Errno::ECONNREFUSED", :port => 636, :socket_class => FakeTCPSocket) stderr = capture_stderr do assert_raise Net::LDAP::ConnectionRefusedError do - Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636) + connection.socket end end 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) + def test_connection_timeout + connection = Net::LDAP::Connection.new(:host => "fail.Errno::ETIMEDOUT", :port => 636, :socket_class => FakeTCPSocket) stderr = capture_stderr do assert_raise Net::LDAP::Error do - Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636) + connection.socket end end end def test_raises_unknown_exceptions - error = Class.new(StandardError) - flexmock(Socket).should_receive(:tcp).and_raise(error) - assert_raise error do - Net::LDAP::Connection.new(:host => 'test.mocked.com', :port => 636) + connection = Net::LDAP::Connection.new(:host => "fail.StandardError", :port => 636, :socket_class => FakeTCPSocket) + assert_raise StandardError do + connection.socket end end