Merge pull request #141 from ruby-ldap/magic-number-cleanup

Magic number/constant cleanup

Author: mtodd

lib/net/ldap.rb

@@ -321,29 +321,103 @@ class LdapError < StandardError; end
 
   StartTlsOid = "1.3.6.1.4.1.1466.20037"
 
+  # https://tools.ietf.org/html/rfc4511#section-4.1.9
+  # https://tools.ietf.org/html/rfc4511#appendix-A
+  ResultCodeSuccess                      = 0
+  ResultCodeOperationsError              = 1
+  ResultCodeProtocolError                = 2
+  ResultCodeTimeLimitExceeded            = 3
+  ResultCodeSizeLimitExceeded            = 4
+  ResultCodeCompareFalse                 = 5
+  ResultCodeCompareTrue                  = 6
+  ResultCodeAuthMethodNotSupported       = 7
+  ResultCodeStrongerAuthRequired         = 8
+  ResultCodeReferral                     = 10
+  ResultCodeAdminLimitExceeded           = 11
+  ResultCodeUnavailableCriticalExtension = 12
+  ResultCodeConfidentialityRequired      = 13
+  ResultCodeSaslBindInProgress           = 14
+  ResultCodeNoSuchAttribute              = 16
+  ResultCodeUndefinedAttributeType       = 17
+  ResultCodeInappropriateMatching        = 18
+  ResultCodeConstraintViolation          = 19
+  ResultCodeAttributeOrValueExists       = 20
+  ResultCodeInvalidAttributeSyntax       = 21
+  ResultCodeNoSuchObject                 = 32
+  ResultCodeAliasProblem                 = 33
+  ResultCodeInvalidDNSyntax              = 34
+  ResultCodeAliasDereferencingProblem    = 36
+  ResultCodeInappropriateAuthentication  = 48
+  ResultCodeInvalidCredentials           = 49
+  ResultCodeInsufficientAccessRights     = 50
+  ResultCodeBusy                         = 51
+  ResultCodeUnavailable                  = 52
+  ResultCodeUnwillingToPerform           = 53
+  ResultCodeNamingViolation              = 64
+  ResultCodeObjectClassViolation         = 65
+  ResultCodeNotAllowedOnNonLeaf          = 66
+  ResultCodeNotAllowedOnRDN              = 67
+  ResultCodeEntryAlreadyExists           = 68
+  ResultCodeObjectClassModsProhibited    = 69
+  ResultCodeAffectsMultipleDSAs          = 71
+  ResultCodeOther                        = 80
+
+  # https://tools.ietf.org/html/rfc4511#appendix-A.1
+  ResultCodesNonError = [
+    ResultCodeSuccess,
+    ResultCodeCompareFalse,
+    ResultCodeCompareTrue,
+    ResultCodeReferral,
+    ResultCodeSaslBindInProgress
+  ]
+
+  # nonstandard list of "successful" result codes for searches
+  ResultCodesSearchSuccess = [
+    ResultCodeSuccess,
+    ResultCodeTimeLimitExceeded,
+    ResultCodeSizeLimitExceeded
+  ]
+
+  # map of result code to human message
   ResultStrings = {
-    0 => "Success",
-    1 => "Operations Error",
-    2 => "Protocol Error",
-    3 => "Time Limit Exceeded",
-    4 => "Size Limit Exceeded",
-    10 => "Referral",
-    12 => "Unavailable crtical extension",
-    14 => "saslBindInProgress",
-    16 => "No Such Attribute",
-    17 => "Undefined Attribute Type",
-    19 => "Constraint Violation",
-    20 => "Attribute or Value Exists",
-    32 => "No Such Object",
-    34 => "Invalid DN Syntax",
-    48 => "Inappropriate Authentication",
-    49 => "Invalid Credentials",
-    50 => "Insufficient Access Rights",
-    51 => "Busy",
-    52 => "Unavailable",
-    53 => "Unwilling to perform",
-    65 => "Object Class Violation",
-    68 => "Entry Already Exists"
+    ResultCodeSuccess                      => "Success",
+    ResultCodeOperationsError              => "Operations Error",
+    ResultCodeProtocolError                => "Protocol Error",
+    ResultCodeTimeLimitExceeded            => "Time Limit Exceeded",
+    ResultCodeSizeLimitExceeded            => "Size Limit Exceeded",
+    ResultCodeCompareFalse                 => "False Comparison",
+    ResultCodeCompareTrue                  => "True Comparison",
+    ResultCodeAuthMethodNotSupported       => "Auth Method Not Supported",
+    ResultCodeStrongerAuthRequired         => "Stronger Auth Needed",
+    ResultCodeReferral                     => "Referral",
+    ResultCodeAdminLimitExceeded           => "Admin Limit Exceeded",
+    ResultCodeUnavailableCriticalExtension => "Unavailable crtical extension",
+    ResultCodeConfidentialityRequired      => "Confidentiality Required",
+    ResultCodeSaslBindInProgress           => "saslBindInProgress",
+    ResultCodeNoSuchAttribute              => "No Such Attribute",
+    ResultCodeUndefinedAttributeType       => "Undefined Attribute Type",
+    ResultCodeInappropriateMatching        => "Inappropriate Matching",
+    ResultCodeConstraintViolation          => "Constraint Violation",
+    ResultCodeAttributeOrValueExists       => "Attribute or Value Exists",
+    ResultCodeInvalidAttributeSyntax       => "Invalide Attribute Syntax",
+    ResultCodeNoSuchObject                 => "No Such Object",
+    ResultCodeAliasProblem                 => "Alias Problem",
+    ResultCodeInvalidDNSyntax              => "Invalid DN Syntax",
+    ResultCodeAliasDereferencingProblem    => "Alias Dereferencing Problem",
+    ResultCodeInappropriateAuthentication  => "Inappropriate Authentication",
+    ResultCodeInvalidCredentials           => "Invalid Credentials",
+    ResultCodeInsufficientAccessRights     => "Insufficient Access Rights",
+    ResultCodeBusy                         => "Busy",
+    ResultCodeUnavailable                  => "Unavailable",
+    ResultCodeUnwillingToPerform           => "Unwilling to perform",
+    ResultCodeNamingViolation              => "Naming Violation",
+    ResultCodeObjectClassViolation         => "Object Class Violation",
+    ResultCodeNotAllowedOnNonLeaf          => "Not Allowed On Non-Leaf",
+    ResultCodeNotAllowedOnRDN              => "Not Allowed On RDN",
+    ResultCodeEntryAlreadyExists           => "Entry Already Exists",
+    ResultCodeObjectClassModsProhibited    => "ObjectClass Modifications Prohibited",
+    ResultCodeAffectsMultipleDSAs          => "Affects Multiple DSAs",
+    ResultCodeOther                        => "Other"
   }
 
   module LDAPControls
@@ -549,7 +623,7 @@ def get_operation_result
     elsif result
       os.code = result
     else
-      os.code = 0
+      os.code = Net::LDAP::ResultCodeSuccess
     end
     os.message = Net::LDAP.result2string(os.code)
     os
@@ -667,7 +741,7 @@ def search(args = {})
             :port                    => @port,
             :encryption              => @encryption,
             :instrumentation_service => @instrumentation_service
-          if (@result = conn.bind(args[:auth] || @auth)).result_code == 0
+          if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
             @result = conn.search(args) { |entry|
               result_set << entry if result_set
               yield entry if block_given?
@@ -680,14 +754,7 @@ def search(args = {})
 
       if return_result_set
         unless @result.nil?
-          case @result.result_code
-          when ResultStrings.key("Success")
-            # everything good
-            result_set
-          when ResultStrings.key("Size Limit Exceeded"), ResultStrings.key("Time Limit Exceeded")
-            # LDAP: Size/Time limit exceeded
-            # This happens when we use size option and results are truncated
-            # Still we need to return user results
+          if ResultCodesSearchSuccess.include?(@result.result_code)
             result_set
           end
         end
@@ -873,7 +940,7 @@ def add(args)
             :port                    => @port,
             :encryption              => @encryption,
             :instrumentation_service => @instrumentation_service
-          if (@result = conn.bind(args[:auth] || @auth)).result_code == 0
+          if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
             @result = conn.add(args)
           end
         ensure
@@ -977,7 +1044,7 @@ def modify(args)
             :port                    => @port,
             :encryption              => @encryption,
             :instrumentation_service => @instrumentation_service
-          if (@result = conn.bind(args[:auth] || @auth)).result_code == 0
+          if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
             @result = conn.modify(args)
           end
         ensure
@@ -1054,7 +1121,7 @@ def rename(args)
             :port                    => @port,
             :encryption              => @encryption,
             :instrumentation_service => @instrumentation_service
-          if (@result = conn.bind(args[:auth] || @auth)).result_code == 0
+          if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
             @result = conn.rename(args)
           end
         ensure
@@ -1087,7 +1154,7 @@ def delete(args)
             :port                    => @port,
             :encryption              => @encryption,
             :instrumentation_service => @instrumentation_service
-          if (@result = conn.bind(args[:auth] || @auth)).result_code == 0
+          if (@result = conn.bind(args[:auth] || @auth)).result_code == Net::LDAP::ResultCodeSuccess
             @result = conn.delete(args)
           end
         ensure

lib/net/ldap/connection.rb

@@ -227,7 +227,7 @@ def bind_simple(auth)
     raise Net::LDAP::LdapError, "Invalid binding information" unless (user && psw)
 
     request = [LdapVersion.to_ber, user.to_ber,
-      psw.to_ber_contextspecific(0)].to_ber_appsequence(0)
+      psw.to_ber_contextspecific(0)].to_ber_appsequence(Net::LDAP::PDU::BindRequest)
     write(request)
 
     pdu = read
@@ -265,13 +265,13 @@ def bind_sasl(auth)
     n = 0
     loop {
       sasl = [mech.to_ber, cred.to_ber].to_ber_contextspecific(3)
-      request = [LdapVersion.to_ber, "".to_ber, sasl].to_ber_appsequence(0)
+      request = [LdapVersion.to_ber, "".to_ber, sasl].to_ber_appsequence(Net::LDAP::PDU::BindRequest)
       write(request)
 
       pdu = read
       raise Net::LDAP::LdapError, "no bind result" unless pdu
 
-      return pdu unless pdu.result_code == 14 # saslBindInProgress
+      return pdu unless pdu.result_code == Net::LDAP::ResultCodeSaslBindInProgress
       raise Net::LDAP::LdapError, "sasl-challenge overflow" if ((n += 1) > MaxSaslChallenges)
 
       cred = chall.call(pdu.result_server_sasl_creds)
@@ -450,7 +450,7 @@ def search(args = nil)
           attrs_only.to_ber,
           filter.to_ber,
           ber_attrs.to_ber_sequence
-        ].to_ber_appsequence(3)
+        ].to_ber_appsequence(Net::LDAP::PDU::SearchRequest)
 
         # rfc2696_cookie sometimes contains binary data from Microsoft Active Directory
         # this breaks when calling to_ber. (Can't force binary data to UTF-8)
@@ -488,7 +488,7 @@ def search(args = nil)
           when Net::LDAP::PDU::SearchResult
             result_pdu = pdu
             controls = pdu.result_controls
-            if refs && pdu.result_code == 10
+            if refs && pdu.result_code == Net::LDAP::ResultCodeReferral
               if block_given?
                 se = Net::LDAP::Entry.new
                 se[:search_referrals] = (pdu.search_referrals || [])
@@ -516,7 +516,7 @@ def search(args = nil)
         # of type OCTET STRING, covered in the default syntax supported by
         # read_ber, so I guess we're ok.
         more_pages = false
-        if result_pdu.result_code == 0 and controls
+        if result_pdu.result_code == Net::LDAP::ResultCodeSuccess and controls
           controls.each do |c|
             if c.oid == Net::LDAP::LDAPControls::PAGED_RESULTS
               # just in case some bogus server sends us more than 1 of these.
@@ -538,7 +538,7 @@ def search(args = nil)
       # track total result count
       payload[:result_count] = n_results
 
-      result_pdu || OpenStruct.new(:status => :failure, :result_code => 1, :message => "Invalid search")
+      result_pdu || OpenStruct.new(:status => :failure, :result_code => Net::LDAP::ResultCodeOperationsError, :message => "Invalid search")
     end # instrument
   ensure
     # clean up message queue for this search
@@ -584,7 +584,7 @@ def modify(args)
     modify_dn = args[:dn] or raise "Unable to modify empty DN"
     ops = self.class.modify_ops args[:operations]
     request = [ modify_dn.to_ber,
-      ops.to_ber_sequence ].to_ber_appsequence(6)
+      ops.to_ber_sequence ].to_ber_appsequence(Net::LDAP::PDU::ModifyRequest)
     write(request)
 
     pdu = read
@@ -610,7 +610,7 @@ def add(args)
       add_attrs << [ k.to_s.to_ber, Array(v).map { |m| m.to_ber}.to_ber_set ].to_ber_sequence
     }
 
-    request = [add_dn.to_ber, add_attrs.to_ber_sequence].to_ber_appsequence(8)
+    request = [add_dn.to_ber, add_attrs.to_ber_sequence].to_ber_appsequence(Net::LDAP::PDU::AddRequest)
     write(request)
 
     pdu = read
@@ -634,7 +634,7 @@ def rename(args)
     request = [old_dn.to_ber, new_rdn.to_ber, delete_attrs.to_ber]
     request << new_superior.to_ber_contextspecific(0) unless new_superior == nil
 
-    write(request.to_ber_appsequence(12))
+    write(request.to_ber_appsequence(Net::LDAP::PDU::ModifyRDNRequest))
 
     pdu = read
 
@@ -651,7 +651,7 @@ def rename(args)
   def delete(args)
     dn = args[:dn] or raise "Unable to delete empty DN"
     controls = args.include?(:control_codes) ? args[:control_codes].to_ber_control : nil #use nil so we can compact later
-    request = dn.to_s.to_ber_application_string(10)
+    request = dn.to_s.to_ber_application_string(Net::LDAP::PDU::DeleteRequest)
     write(request, controls)
 
     pdu = read

lib/net/ldap/pdu.rb

@@ -18,24 +18,48 @@
 # well with our approach.
 #
 # Currently, we only support controls on SearchResult.
+#
+# http://tools.ietf.org/html/rfc4511#section-4.1.1
+# http://tools.ietf.org/html/rfc4511#section-4.1.9
 class Net::LDAP::PDU
   class Error < RuntimeError; end
 
-  ##
-  # This message packet is a bind request.
+  # http://tools.ietf.org/html/rfc4511#section-4.2
   BindRequest = 0
+  # http://tools.ietf.org/html/rfc4511#section-4.2.2
   BindResult = 1
+  # http://tools.ietf.org/html/rfc4511#section-4.3
   UnbindRequest = 2
+  # http://tools.ietf.org/html/rfc4511#section-4.5.1
   SearchRequest = 3
+  # http://tools.ietf.org/html/rfc4511#section-4.5.2
   SearchReturnedData = 4
   SearchResult = 5
+  # see also SearchResultReferral (19)
+  # http://tools.ietf.org/html/rfc4511#section-4.6
+  ModifyRequest  = 6
   ModifyResponse = 7
+  # http://tools.ietf.org/html/rfc4511#section-4.7
+  AddRequest = 8
   AddResponse = 9
+  # http://tools.ietf.org/html/rfc4511#section-4.8
+  DeleteRequest = 10
   DeleteResponse = 11
+  # http://tools.ietf.org/html/rfc4511#section-4.9
+  ModifyRDNRequest  = 12
   ModifyRDNResponse = 13
+  # http://tools.ietf.org/html/rfc4511#section-4.10
+  CompareRequest = 14
+  CompareResponse = 15
+  # http://tools.ietf.org/html/rfc4511#section-4.11
+  AbandonRequest = 16
+  # http://tools.ietf.org/html/rfc4511#section-4.5.2
   SearchResultReferral = 19
+  # http://tools.ietf.org/html/rfc4511#section-4.12
   ExtendedRequest = 23
   ExtendedResponse = 24
+  # unused: http://tools.ietf.org/html/rfc4511#section-4.13
+  IntermediateResponse = 25
 
   ##
   # The LDAP packet message ID.
@@ -125,7 +149,7 @@ def result_code(code = :resultCode)
   end
 
   def status
-    result_code == 0 ? :success : :failure
+    Net::LDAP::ResultCodesNonError.include?(result_code) ? :success : :failure
   end
 
   def success?
@@ -152,7 +176,7 @@ def parse_ldap_result(sequence)
       :matchedDN => sequence[1],
       :errorMessage => sequence[2]
     }
-    parse_search_referral(sequence[3]) if @ldap_result[:resultCode] == 10
+    parse_search_referral(sequence[3]) if @ldap_result[:resultCode] == Net::LDAP::ResultCodeReferral
   end
   private :parse_ldap_result
 

test/integration/test_bind.rb

@@ -9,8 +9,8 @@ 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
 
     result = @ldap.get_operation_result
-    assert_equal 53, result.code
-    assert_equal "Unwilling to perform", result.message
+    assert_equal Net::LDAP::ResultCodeUnwillingToPerform, result.code
+    assert_equal Net::LDAP::ResultStrings[Net::LDAP::ResultCodeUnwillingToPerform], result.message
     assert_equal "unauthenticated bind (DN with no password) disallowed",
       result.error_message
     assert_equal "", result.matched_dn

test/integration/test_delete.rb

@@ -25,7 +25,7 @@ def test_delete
     refute @ldap.search(base: @dn, scope: Net::LDAP::SearchScope_BaseObject)
 
     result = @ldap.get_operation_result
-    assert_equal 32, result.code
-    assert_equal Net::LDAP::ResultStrings[32], result.message
+    assert_equal Net::LDAP::ResultCodeNoSuchObject, result.code
+    assert_equal Net::LDAP::ResultStrings[Net::LDAP::ResultCodeNoSuchObject], result.message
   end
 end