Merge pull request #139 from ruby-ldap/search-connection-cleanup

Net::LDAP::Connection#search cleanup

Author: mtodd

lib/net/ldap/connection.rb

@@ -311,26 +311,45 @@ def encode_sort_controls(sort_definitions)
   # type-5 packet, which might never come. We need to support the time-limit
   # in the protocol.
   #++
-  def search(args = {})
-    search_filter = (args && args[:filter]) ||
-      Net::LDAP::Filter.eq("objectclass", "*")
-    search_filter = Net::LDAP::Filter.construct(search_filter) if search_filter.is_a?(String)
-    search_base = (args && args[:base]) || "dc=example, dc=com"
-    search_attributes = ((args && args[:attributes]) || []).map { |attr| attr.to_s.to_ber}
-    return_referrals = args && args[:return_referrals] == true
-    sizelimit = (args && args[:size].to_i) || 0
-    raise Net::LDAP::LdapError, "invalid search-size" unless sizelimit >= 0
-    paged_searches_supported = (args && args[:paged_searches_supported])
-
-    attributes_only = (args and args[:attributes_only] == true)
-    scope = args[:scope] || Net::LDAP::SearchScope_WholeSubtree
+  def search(args = nil)
+    args ||= {}
+
+    # filtering, scoping, search base
+    # filter: https://tools.ietf.org/html/rfc4511#section-4.5.1.7
+    # base:   https://tools.ietf.org/html/rfc4511#section-4.5.1.1
+    # scope:  https://tools.ietf.org/html/rfc4511#section-4.5.1.2
+    filter = args[:filter] || Net::LDAP::Filter.eq("objectClass", "*")
+    base   = args[:base]
+    scope  = args[:scope] || Net::LDAP::SearchScope_WholeSubtree
+
+    # attr handling
+    # attrs:      https://tools.ietf.org/html/rfc4511#section-4.5.1.8
+    # attrs_only: https://tools.ietf.org/html/rfc4511#section-4.5.1.6
+    attrs  = Array(args[:attributes])
+    attrs_only = args[:attributes_only] == true
+
+    # references
+    # refs:  https://tools.ietf.org/html/rfc4511#section-4.5.3
+    # deref: https://tools.ietf.org/html/rfc4511#section-4.5.1.3
+    refs   = args[:return_referrals] == true
+    deref  = args[:deref] || Net::LDAP::DerefAliases_Never
+
+    # limiting, paging, sorting
+    # size: https://tools.ietf.org/html/rfc4511#section-4.5.1.4
+    size   = args[:size].to_i
+    paged  = args[:paged_searches_supported]
+    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)
 
-    sort_control = encode_sort_controls(args.fetch(:sort_controls){ false })
-
-  deref = args[:deref] || Net::LDAP::DerefAliases_Never
-  raise Net::LDAP::LdapError.new( "invalid alias dereferencing value" ) unless Net::LDAP::DerefAliasesArray.include?(deref)
-
+    # arg transforms
+    filter = Net::LDAP::Filter.construct(filter) if filter.is_a?(String)
+    ber_attrs = attrs.map { |attr| attr.to_s.to_ber }
+    ber_sort  = encode_sort_controls(sort)
 
     # An interesting value for the size limit would be close to A/D's
     # built-in page limit of 1000 records, but openLDAP newer than version
@@ -357,35 +376,35 @@ def search(args = {})
     n_results = 0
 
     instrument "search.net_ldap_connection",
-               :filter     => search_filter,
-               :base       => search_base,
-               :scope      => scope,
-               :limit      => sizelimit,
-               :sort       => sort_control,
-               :referrals  => return_referrals,
-               :deref      => deref,
-               :attributes => search_attributes do |payload|
+               filter:     filter,
+               base:       base,
+               scope:      scope,
+               limit:      size,
+               sort:       sort,
+               referrals:  refs,
+               deref:      deref,
+               attributes: attrs do |payload|
       loop do
         # should collect this into a private helper to clarify the structure
         query_limit = 0
-        if sizelimit > 0
-          if paged_searches_supported
-            query_limit = (((sizelimit - n_results) < 126) ? (sizelimit -
+        if size > 0
+          if paged
+            query_limit = (((size - n_results) < 126) ? (size -
                                                               n_results) : 0)
           else
-            query_limit = sizelimit
+            query_limit = size
           end
         end
 
         request = [
-          search_base.to_ber,
+          base.to_ber,
           scope.to_ber_enumerated,
           deref.to_ber_enumerated,
           query_limit.to_ber, # size limit
           0.to_ber,
-          attributes_only.to_ber,
-          search_filter.to_ber,
-          search_attributes.to_ber_sequence
+          attrs_only.to_ber,
+          filter.to_ber,
+          ber_attrs.to_ber_sequence
         ].to_ber_appsequence(3)
 
         # rfc2696_cookie sometimes contains binary data from Microsoft Active Directory
@@ -399,8 +418,8 @@ def search(args = {})
             # Criticality MUST be false to interoperate with normal LDAPs.
             false.to_ber,
             rfc2696_cookie.map{ |v| v.to_ber}.to_ber_sequence.to_s.to_ber
-          ].to_ber_sequence if paged_searches_supported
-        controls << sort_control if sort_control
+          ].to_ber_sequence if paged
+        controls << ber_sort if ber_sort
         controls = controls.empty? ? nil : controls.to_ber_contextspecific(0)
 
         write(request, controls)
@@ -414,7 +433,7 @@ def search(args = {})
             n_results += 1
             yield pdu.search_entry if block_given?
           when Net::LDAP::PDU::SearchResultReferral
-            if return_referrals
+            if refs
               if block_given?
                 se = Net::LDAP::Entry.new
                 se[:search_referrals] = (pdu.search_referrals || [])
@@ -424,7 +443,7 @@ def search(args = {})
           when Net::LDAP::PDU::SearchResult
             result_pdu = pdu
             controls = pdu.result_controls
-            if return_referrals && pdu.result_code == 10
+            if refs && pdu.result_code == 10
               if block_given?
                 se = Net::LDAP::Entry.new
                 se[:search_referrals] = (pdu.search_referrals || [])

test/integration/test_delete.rb

@@ -14,7 +14,9 @@ def setup
       sn:   "delete-user1",
       mail: "[email protected]"
     }
-    assert @ldap.add(dn: @dn, attributes: attrs), @ldap.get_operation_result.inspect
+    unless @ldap.search(base: @dn, scope: Net::LDAP::SearchScope_BaseObject)
+      assert @ldap.add(dn: @dn, attributes: attrs), @ldap.get_operation_result.inspect
+    end
     assert @ldap.search(base: @dn, scope: Net::LDAP::SearchScope_BaseObject)
   end
 

test/test_ldap_connection.rb

@@ -186,7 +186,7 @@ def test_bind_net_ldap_connection_event
   def test_search_net_ldap_connection_event
     # search data
     search_data_ber = Net::BER::BerIdentifiedArray.new([2, [
-      "uid=user1,ou=OrgUnit2,ou=OrgUnitTop,dc=openldap,dc=ghe,dc=local",
+      "uid=user1,ou=People,dc=rubyldap,dc=com",
       [ ["uid", ["user1"]] ]
     ]])
     search_data_ber.ber_identifier = Net::LDAP::PDU::SearchReturnedData
@@ -200,7 +200,7 @@ def test_search_net_ldap_connection_event
 
     events = @service.subscribe "search.net_ldap_connection"
 
-    result = @connection.search(filter: "(uid=user1)")
+    result = @connection.search(filter: "(uid=user1)", base: "ou=People,dc=rubyldap,dc=com")
     assert result.success?, "should be success"
 
     # a search event