Merge pull request #144 from ruby-ldap/queue-reads-everywhere

Queue reads everywhere

lib/net/ldap/connection.rb

@@ -87,10 +87,18 @@ def setup_encryption(args)
       # additional branches requiring server validation and peer certs, etc.
       # go here.
     when :start_tls
-      request = [Net::LDAP::StartTlsOid.to_ber_contextspecific(0)].to_ber_appsequence(Net::LDAP::PDU::ExtendedRequest)
-      write(request)
-      pdu = read
-      raise Net::LDAP::LdapError, "no start_tls result" if pdu.nil?
+      message_id = next_msgid
+      request    = [
+        Net::LDAP::StartTlsOid.to_ber_contextspecific(0)
+      ].to_ber_appsequence(Net::LDAP::PDU::ExtendedRequest)
+
+      write(request, nil, message_id)
+      pdu = queued_read(message_id)
+
+      if pdu.nil? || pdu.app_tag != Net::LDAP::PDU::ExtendedResponse
+        raise Net::LDAP::LdapError, "no start_tls result"
+      end
+
       if pdu.result_code.zero?
         @conn = self.class.wrap_with_ssl(@conn)
       else
@@ -226,12 +234,18 @@ 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(Net::LDAP::PDU::BindRequest)
-    write(request)
+    message_id = next_msgid
+    request    = [
+      LdapVersion.to_ber, user.to_ber,
+      psw.to_ber_contextspecific(0)
+    ].to_ber_appsequence(Net::LDAP::PDU::BindRequest)
 
-    pdu = read
-    raise Net::LDAP::LdapError, "no bind result" unless pdu
+    write(request, nil, message_id)
+    pdu = queued_read(message_id)
+
+    if !pdu || pdu.app_tag != Net::LDAP::PDU::BindResult
+      raise Net::LDAP::LdapError, "no bind result"
+    end
 
     pdu
   end
@@ -262,14 +276,21 @@ def bind_sasl(auth)
       auth[:challenge_response]
     raise Net::LDAP::LdapError, "Invalid binding information" unless (mech && cred && chall)
 
+    message_id = next_msgid
+
     n = 0
     loop {
       sasl = [mech.to_ber, cred.to_ber].to_ber_contextspecific(3)
-      request = [LdapVersion.to_ber, "".to_ber, sasl].to_ber_appsequence(Net::LDAP::PDU::BindRequest)
-      write(request)
+      request = [
+        LdapVersion.to_ber, "".to_ber, sasl
+      ].to_ber_appsequence(Net::LDAP::PDU::BindRequest)
 
-      pdu = read
-      raise Net::LDAP::LdapError, "no bind result" unless pdu
+      write(request, nil, message_id)
+      pdu = queued_read(message_id)
+
+      if !pdu || pdu.app_tag != Net::LDAP::PDU::BindResult
+        raise Net::LDAP::LdapError, "no bind result"
+      end
 
       return pdu unless pdu.result_code == Net::LDAP::ResultCodeSaslBindInProgress
       raise Net::LDAP::LdapError, "sasl-challenge overflow" if ((n += 1) > MaxSaslChallenges)
@@ -583,11 +604,15 @@ def self.modify_ops(operations)
   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(Net::LDAP::PDU::ModifyRequest)
-    write(request)
 
-    pdu = read
+    message_id = next_msgid
+    request    = [
+      modify_dn.to_ber,
+      ops.to_ber_sequence
+    ].to_ber_appsequence(Net::LDAP::PDU::ModifyRequest)
+
+    write(request, nil, message_id)
+    pdu = queued_read(message_id)
 
     if !pdu || pdu.app_tag != Net::LDAP::PDU::ModifyResponse
       raise Net::LDAP::LdapError, "response missing or invalid"
@@ -610,10 +635,11 @@ 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(Net::LDAP::PDU::AddRequest)
-    write(request)
+    message_id = next_msgid
+    request    = [add_dn.to_ber, add_attrs.to_ber_sequence].to_ber_appsequence(Net::LDAP::PDU::AddRequest)
 
-    pdu = read
+    write(request, nil, message_id)
+    pdu = queued_read(message_id)
 
     if !pdu || pdu.app_tag != Net::LDAP::PDU::AddResponse
       raise Net::LDAP::LdapError, "response missing or invalid"
@@ -631,12 +657,12 @@ def rename(args)
     delete_attrs = args[:delete_attributes] ? true : false
     new_superior = args[:new_superior]
 
-    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(Net::LDAP::PDU::ModifyRDNRequest))
+    message_id = next_msgid
+    request    = [old_dn.to_ber, new_rdn.to_ber, delete_attrs.to_ber]
+    request   << new_superior.to_ber_contextspecific(0) unless new_superior == nil
 
-    pdu = read
+    write(request.to_ber_appsequence(Net::LDAP::PDU::ModifyRDNRequest), nil, message_id)
+    pdu = queued_read(message_id)
 
     if !pdu || pdu.app_tag != Net::LDAP::PDU::ModifyRDNResponse
       raise Net::LDAP::LdapError.new "response missing or invalid"
@@ -650,11 +676,12 @@ 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(Net::LDAP::PDU::DeleteRequest)
-    write(request, controls)
+    controls   = args.include?(:control_codes) ? args[:control_codes].to_ber_control : nil #use nil so we can compact later
+    message_id = next_msgid
+    request    = dn.to_s.to_ber_application_string(Net::LDAP::PDU::DeleteRequest)
 
-    pdu = read
+    write(request, controls, message_id)
+    pdu = queued_read(message_id)
 
     if !pdu || pdu.app_tag != Net::LDAP::PDU::DeleteResponse
       raise Net::LDAP::LdapError, "response missing or invalid"

test/integration/test_open.rb

@@ -21,6 +21,10 @@ def test_binds_with_open
     assert_equal 1, events.size
   end
 
+  # NOTE: query for two or more entries so that the socket must be read
+  # multiple times.
+  # See The Problem: https://github.com/ruby-ldap/ruby-net-ldap/issues/136
+
   def test_nested_search_without_open
     entries = []
     nested_entry = nil
@@ -48,4 +52,37 @@ def test_nested_search_with_open
     assert_equal "user3", nested_entry.uid.first
     assert_equal %w(user1 user2), entries
   end
+
+  def test_nested_add_with_open
+    entries = []
+    nested_entry = nil
+
+    dn = "uid=nested-open-added-user1,ou=People,dc=rubyldap,dc=com"
+    attrs = {
+      objectclass: %w(top inetOrgPerson organizationalPerson person),
+      uid:  "nested-open-added-user1",
+      cn:   "nested-open-added-user1",
+      sn:   "nested-open-added-user1",
+      mail: "nested-open-added-user1@rubyldap.com"
+    }
+
+    @ldap.authenticate "cn=admin,dc=rubyldap,dc=com", "passworD1"
+    @ldap.delete dn: dn
+
+    @ldap.open do
+      @ldap.search(filter: "(|(uid=user1)(uid=user2))", base: "ou=People,dc=rubyldap,dc=com") do |entry|
+        entries << entry.uid.first
+
+        nested_entry ||= begin
+          assert @ldap.add(dn: dn, attributes: attrs), @ldap.get_operation_result.inspect
+          @ldap.search(base: dn, scope: Net::LDAP::SearchScope_BaseObject).first
+        end
+      end
+    end
+
+    assert_equal %w(user1 user2), entries
+    assert_equal "nested-open-added-user1", nested_entry.uid.first
+  ensure
+    @ldap.delete dn: dn
+  end
 end