-
Notifications
You must be signed in to change notification settings - Fork 252
Specific errors subclassing Net::LDAP::Error #183
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
Changes from all commits
7438a2a
1436abe
a6d6ec8
beac837
b412ca0
c9d36cd
120d8c8
20d3a43
cb27e21
94d2d6a
5a2b2ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,13 +12,13 @@ def initialize(server) | |
begin | ||
@conn = server[:socket] || TCPSocket.new(server[:host], server[:port]) | ||
rescue SocketError | ||
raise Net::LDAP::LdapError, "No such address or other socket error." | ||
raise Net::LDAP::Error, "No such address or other socket error." | ||
rescue Errno::ECONNREFUSED | ||
raise Net::LDAP::LdapError, "Server #{server[:host]} refused connection on port #{server[:port]}." | ||
raise Net::LDAP::Error, "Server #{server[:host]} refused connection on port #{server[:port]}." | ||
rescue Errno::EHOSTUNREACH => error | ||
raise Net::LDAP::LdapError, "Host #{server[:host]} was unreachable (#{error.message})" | ||
raise Net::LDAP::Error, "Host #{server[:host]} was unreachable (#{error.message})" | ||
rescue Errno::ETIMEDOUT | ||
raise Net::LDAP::LdapError, "Connection to #{server[:host]} timed out." | ||
raise Net::LDAP::Error, "Connection to #{server[:host]} timed out." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're making changes to this, I propose removing this rescue block entirely. The stdlib's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jch definitely open to that, but let's tackle that separately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤘 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the meantime, should we be using error classes for these errors? Or are we OK with keeping |
||
end | ||
|
||
if server[:encryption] | ||
|
@@ -42,7 +42,7 @@ def close | |
end | ||
|
||
def self.wrap_with_ssl(io, tls_options = {}) | ||
raise Net::LDAP::LdapError, "OpenSSL is unavailable" unless Net::LDAP::HasOpenSSL | ||
raise Net::LDAP::NoOpenSSLError, "OpenSSL is unavailable" unless Net::LDAP::HasOpenSSL | ||
|
||
ctx = OpenSSL::SSL::SSLContext.new | ||
|
||
|
@@ -67,7 +67,7 @@ def self.wrap_with_ssl(io, tls_options = {}) | |
# successfully-opened @conn instance variable, which is a TCP connection. | ||
# Depending on the received arguments, we establish SSL, potentially | ||
# replacing the value of @conn accordingly. Don't generate any errors here | ||
# if no encryption is requested. DO raise Net::LDAP::LdapError objects if encryption | ||
# if no encryption is requested. DO raise Net::LDAP::Error objects if encryption | ||
# is requested and we have trouble setting it up. That includes if OpenSSL | ||
# is not set up on the machine. (Question: how does the Ruby OpenSSL | ||
# wrapper react in that case?) DO NOT filter exceptions raised by the | ||
|
@@ -105,16 +105,16 @@ def setup_encryption(args) | |
pdu = queued_read(message_id) | ||
|
||
if pdu.nil? || pdu.app_tag != Net::LDAP::PDU::ExtendedResponse | ||
raise Net::LDAP::LdapError, "no start_tls result" | ||
raise Net::LDAP::NoStartTLSResultError, "no start_tls result" | ||
end | ||
|
||
if pdu.result_code.zero? | ||
@conn = self.class.wrap_with_ssl(@conn, args[:tls_options]) | ||
else | ||
raise Net::LDAP::LdapError, "start_tls failed: #{pdu.result_code}" | ||
raise Net::LDAP::StartTlSError, "start_tls failed: #{pdu.result_code}" | ||
end | ||
else | ||
raise Net::LDAP::LdapError, "unsupported encryption method #{args[:method]}" | ||
raise Net::LDAP::EncMethodUnsupportedError, "unsupported encryption method #{args[:method]}" | ||
end | ||
end | ||
|
||
|
@@ -225,7 +225,7 @@ def bind(auth) | |
elsif meth == :gss_spnego | ||
bind_gss_spnego(auth) | ||
else | ||
raise Net::LDAP::LdapError, "Unsupported auth method (#{meth})" | ||
raise Net::LDAP::AuthMethodUnsupportedError, "Unsupported auth method (#{meth})" | ||
end | ||
end | ||
end | ||
|
@@ -241,7 +241,7 @@ def bind_simple(auth) | |
["", ""] | ||
end | ||
|
||
raise Net::LDAP::LdapError, "Invalid binding information" unless (user && psw) | ||
raise Net::LDAP::BindingInformationInvalidError, "Invalid binding information" unless (user && psw) | ||
|
||
message_id = next_msgid | ||
request = [ | ||
|
@@ -253,7 +253,7 @@ def bind_simple(auth) | |
pdu = queued_read(message_id) | ||
|
||
if !pdu || pdu.app_tag != Net::LDAP::PDU::BindResult | ||
raise Net::LDAP::LdapError, "no bind result" | ||
raise Net::LDAP::NoBindResultError, "no bind result" | ||
end | ||
|
||
pdu | ||
|
@@ -283,7 +283,7 @@ def bind_simple(auth) | |
def bind_sasl(auth) | ||
mech, cred, chall = auth[:mechanism], auth[:initial_credential], | ||
auth[:challenge_response] | ||
raise Net::LDAP::LdapError, "Invalid binding information" unless (mech && cred && chall) | ||
raise Net::LDAP::BindingInformationInvalidError, "Invalid binding information" unless (mech && cred && chall) | ||
|
||
message_id = next_msgid | ||
|
||
|
@@ -298,16 +298,16 @@ def bind_sasl(auth) | |
pdu = queued_read(message_id) | ||
|
||
if !pdu || pdu.app_tag != Net::LDAP::PDU::BindResult | ||
raise Net::LDAP::LdapError, "no bind result" | ||
raise Net::LDAP::NoBindResultError, "no bind result" | ||
end | ||
|
||
return pdu unless pdu.result_code == Net::LDAP::ResultCodeSaslBindInProgress | ||
raise Net::LDAP::LdapError, "sasl-challenge overflow" if ((n += 1) > MaxSaslChallenges) | ||
raise Net::LDAP::SASLChallengeOverflowError, "sasl-challenge overflow" if ((n += 1) > MaxSaslChallenges) | ||
|
||
cred = chall.call(pdu.result_server_sasl_creds) | ||
} | ||
|
||
raise Net::LDAP::LdapError, "why are we here?" | ||
raise Net::LDAP::SASLChallengeOverflowError, "why are we here?" | ||
end | ||
private :bind_sasl | ||
|
||
|
@@ -326,7 +326,7 @@ def bind_gss_spnego(auth) | |
require 'ntlm' | ||
|
||
user, psw = [auth[:username] || auth[:dn], auth[:password]] | ||
raise Net::LDAP::LdapError, "Invalid binding information" unless (user && psw) | ||
raise Net::LDAP::BindingInformationInvalidError, "Invalid binding information" unless (user && psw) | ||
|
||
nego = proc { |challenge| | ||
t2_msg = NTLM::Message.parse(challenge) | ||
|
@@ -412,10 +412,10 @@ def search(args = nil) | |
sort = args.fetch(:sort_controls, false) | ||
|
||
# arg validation | ||
raise Net::LDAP::LdapError, "search base is required" unless base | ||
raise Net::LDAP::LdapError, "invalid search-size" unless size >= 0 | ||
raise Net::LDAP::LdapError, "invalid search scope" unless Net::LDAP::SearchScopes.include?(scope) | ||
raise Net::LDAP::LdapError, "invalid alias dereferencing value" unless Net::LDAP::DerefAliasesArray.include?(deref) | ||
raise ArgumentError, "search base is required" unless base | ||
raise ArgumentError, "invalid search-size" unless size >= 0 | ||
raise ArgumentError, "invalid search scope" unless Net::LDAP::SearchScopes.include?(scope) | ||
raise ArgumentError, "invalid alias dereferencing value" unless Net::LDAP::DerefAliasesArray.include?(deref) | ||
|
||
# arg transforms | ||
filter = Net::LDAP::Filter.construct(filter) if filter.is_a?(String) | ||
|
@@ -527,7 +527,7 @@ def search(args = nil) | |
end | ||
break | ||
else | ||
raise Net::LDAP::LdapError, "invalid response-type in search: #{pdu.app_tag}" | ||
raise Net::LDAP::ResponseTypeInvalidError, "invalid response-type in search: #{pdu.app_tag}" | ||
end | ||
end | ||
|
||
|
@@ -625,7 +625,7 @@ def modify(args) | |
pdu = queued_read(message_id) | ||
|
||
if !pdu || pdu.app_tag != Net::LDAP::PDU::ModifyResponse | ||
raise Net::LDAP::LdapError, "response missing or invalid" | ||
raise Net::LDAP::ResponseMissingOrInvalidError, "response missing or invalid" | ||
end | ||
|
||
pdu | ||
|
@@ -639,7 +639,7 @@ def modify(args) | |
# to the error message and the matched-DN returned by the server. | ||
#++ | ||
def add(args) | ||
add_dn = args[:dn] or raise Net::LDAP::LdapError, "Unable to add empty DN" | ||
add_dn = args[:dn] or raise Net::LDAP::EmptyDNError, "Unable to add empty DN" | ||
add_attrs = [] | ||
a = args[:attributes] and a.each { |k, v| | ||
add_attrs << [ k.to_s.to_ber, Array(v).map { |m| m.to_ber}.to_ber_set ].to_ber_sequence | ||
|
@@ -652,7 +652,7 @@ def add(args) | |
pdu = queued_read(message_id) | ||
|
||
if !pdu || pdu.app_tag != Net::LDAP::PDU::AddResponse | ||
raise Net::LDAP::LdapError, "response missing or invalid" | ||
raise Net::LDAP::ResponseMissingError, "response missing or invalid" | ||
end | ||
|
||
pdu | ||
|
@@ -675,7 +675,7 @@ def rename(args) | |
pdu = queued_read(message_id) | ||
|
||
if !pdu || pdu.app_tag != Net::LDAP::PDU::ModifyRDNResponse | ||
raise Net::LDAP::LdapError.new "response missing or invalid" | ||
raise Net::LDAP::ResponseMissingOrInvalidError.new "response missing or invalid" | ||
end | ||
|
||
pdu | ||
|
@@ -694,7 +694,7 @@ def delete(args) | |
pdu = queued_read(message_id) | ||
|
||
if !pdu || pdu.app_tag != Net::LDAP::PDU::DeleteResponse | ||
raise Net::LDAP::LdapError, "response missing or invalid" | ||
raise Net::LDAP::ResponseMissingOrInvalidError, "response missing or invalid" | ||
end | ||
|
||
pdu | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
class Net::LDAP | ||
class LdapError < StandardError | ||
def message | ||
"Deprecation warning: Net::LDAP::LdapError is no longer used. Use Net::LDAP::Error or rescue one of it's subclasses. \n" + super | ||
end | ||
end | ||
|
||
class Error < StandardError; end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 for switching the name from class LdapError < Error
def message
"Deprecation warning: Net::LDAP::LdapError is no longer used. Use Net::LDAP::Error or rescue one of it's subclasses. [link to this file]\n" + super
end
end We're pre 1.0, there's no obligation for us to do this. I'm fine with either because it's an easy error to fix if anyone runs into it and we can document it in History.md There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jch I support the rename and keeping around the backwards compatible constant, though the semantics of the backwards compatible constant is a bit trickier than inheriting from begin
# something that could fail
rescue Net::LDAP::LdapError => error
# handle error
end The problem with the above is that it wouldn't actually catch anything that inherits from class Net::LDAP
class Error < StandardError; end
LdapError = Error
end
begin
rescue Net::LDAP::LdapError
# should work
end There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mtodd good eye. What about having class Net::LDAP
class LdapError < StandardError
def message
"Deprecation warning... " + super
end
end
class Error < LdapError; end
end This allows us to eventually remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would work! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
class AlreadyOpenedError < Error; end | ||
class SocketError < Error; end | ||
class ConnectionRefusedError < Error; end | ||
class NoOpenSSLError < Error; end | ||
class NoStartTLSResultError < Error; end | ||
class NoSearchBaseError < Error; end | ||
class StartTLSError < Error; end | ||
class EncryptionUnsupportedError < Error; end | ||
class EncMethodUnsupportedError < Error; end | ||
class AuthMethodUnsupportedError < Error; end | ||
class BindingInformationInvalidError < Error; end | ||
class NoBindResultError < Error; end | ||
class SASLChallengeOverflowError < Error; end | ||
class SearchSizeInvalidError < Error; end | ||
class SearchScopeInvalidError < Error; end | ||
class ResponseTypeInvalidError < Error; end | ||
class ResponseMissingOrInvalidError < Error; end | ||
class EmptyDNError < Error; end | ||
class HashTypeUnsupportedError < Error; end | ||
class OperatorError < Error; end | ||
class SubstringFilterError < Error; end | ||
class SearchFilterError < Error; end | ||
class BERInvalidError < Error; end | ||
class SearchFilterTypeUnknownError < Error; end | ||
class BadAttributeError < Error; end | ||
class FilterTypeUnknownError < Error; end | ||
class FilterSyntaxInvalidError < Error; end | ||
class EntryOverflowError < Error; end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 to putting this in it's own file