Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set a connect_timeout for the creation of a socket #243

Merged
merged 2 commits into from
Jan 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions lib/net/ber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'.
Expand All @@ -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
Expand Down
24 changes: 17 additions & 7 deletions lib/net/ldap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down Expand Up @@ -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" }
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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]
Expand Down
9 changes: 8 additions & 1 deletion lib/net/ldap/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions script/install-openldap
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 helpful comment

iptables -A OUTPUT -p tcp -j DROP --dport 8389

service slapd restart
4 changes: 2 additions & 2 deletions test/ber/test_ber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions test/integration/test_bind.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion test/test_auth_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc #235

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to chime in here: that would definitely simplify a lot of tests 👍

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)
Expand Down
39 changes: 24 additions & 15 deletions test/test_ldap_connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 \
Expand Down