Merge remote-tracking branch 'origin/master' into search-timeout

Conflicts:
	test/integration/test_search.rb

Author: Jerry Cheung

lib/net/ldap.rb

@@ -679,7 +679,18 @@ def search(args = {})
       end
 
       if return_result_set
-        (!@result.nil? && @result.result_code == 0) ? result_set : nil
+        unless @result.nil?
+          case @result.result_code
+          when ResultStrings.key("Success")
+            # everything good
+            result_set
+          when ResultStrings.key("Size Limit Exceeded")
+            # LDAP: Size limit exceeded
+            # This happens when we use size option and results are truncated
+            # Still we need to return user results
+            result_set
+          end
+        end
       else
         @result.success?
       end

lib/net/ldap/connection.rb

@@ -111,6 +111,45 @@ def close
     @conn = nil
   end
 
+  # Internal: Reads messages by ID from a queue, falling back to reading from
+  # the connected socket until a message matching the ID is read. Any messages
+  # with mismatched IDs gets queued for subsequent reads by the origin of that
+  # message ID.
+  #
+  # Returns a Net::LDAP::PDU object or nil.
+  def queued_read(message_id)
+    if pdu = message_queue[message_id].shift
+      return pdu
+    end
+
+    # read messages until we have a match for the given message_id
+    while pdu = read
+      if pdu.message_id == message_id
+        return pdu
+      else
+        message_queue[pdu.message_id].push pdu
+        next
+      end
+    end
+
+    pdu
+  end
+
+  # Internal: The internal queue of messages, read from the socket, grouped by
+  # message ID.
+  #
+  # Used by `queued_read` to return messages sent by the server with the given
+  # ID. If no messages are queued for that ID, `queued_read` will `read` from
+  # the socket and queue messages that don't match the given ID for other
+  # readers.
+  #
+  # Returns the message queue Hash.
+  def message_queue
+    @message_queue ||= Hash.new do |hash, key|
+      hash[key] = []
+    end
+  end
+
   # Internal: Reads and parses data from the configured connection.
   #
   # - syntax: the BER syntax to use to parse the read data with
@@ -146,9 +185,9 @@ def read(syntax = Net::LDAP::AsnSyntax)
   #
   # Returns the return value from writing to the connection, which in some
   # cases is the Integer number of bytes written to the socket.
-  def write(request, controls = nil)
+  def write(request, controls = nil, message_id = next_msgid)
     instrument "write.net_ldap_connection" do |payload|
-      packet = [next_msgid.to_ber, request, controls].compact.to_ber_sequence
+      packet = [message_id.to_ber, request, controls].compact.to_ber_sequence
       payload[:content_length] = @conn.write(packet)
     end
   end
@@ -377,7 +416,10 @@ def search(args = nil)
     result_pdu = nil
     n_results = 0
 
+    message_id = next_msgid
+
     instrument "search.net_ldap_connection",
+               message_id: message_id,
                filter:     filter,
                base:       base,
                scope:      scope,
@@ -425,12 +467,12 @@ def search(args = nil)
         controls << ber_sort if ber_sort
         controls = controls.empty? ? nil : controls.to_ber_contextspecific(0)
 
-        write(request, controls)
+        write(request, controls, message_id)
 
         result_pdu = nil
         controls = []
 
-        while pdu = read
+        while pdu = queued_read(message_id)
           case pdu.app_tag
           when Net::LDAP::PDU::SearchReturnedData
             n_results += 1
@@ -498,6 +540,16 @@ def search(args = nil)
 
       result_pdu || OpenStruct.new(:status => :failure, :result_code => 1, :message => "Invalid search")
     end # instrument
+  ensure
+    # clean up message queue for this search
+    messages = message_queue.delete(message_id)
+
+    # in the exceptional case some messages were *not* consumed from the queue,
+    # instrument the event but do not fail.
+    unless messages.empty?
+      instrument "search_messages_unread.net_ldap_connection",
+                 message_id: message_id, messages: messages
+    end
   end
 
   MODIFY_OPERATIONS = { #:nodoc:

test/integration/test_open.rb

@@ -0,0 +1,51 @@
+require_relative '../test_helper'
+
+class TestBindIntegration < LDAPIntegrationTestCase
+  def test_binds_without_open
+    events = @service.subscribe "bind.net_ldap_connection"
+
+    @ldap.search(filter: "uid=user1", base: "ou=People,dc=rubyldap,dc=com", ignore_server_caps: true)
+    @ldap.search(filter: "uid=user1", base: "ou=People,dc=rubyldap,dc=com", ignore_server_caps: true)
+
+    assert_equal 2, events.size
+  end
+
+  def test_binds_with_open
+    events = @service.subscribe "bind.net_ldap_connection"
+
+    @ldap.open do
+      @ldap.search(filter: "uid=user1", base: "ou=People,dc=rubyldap,dc=com", ignore_server_caps: true)
+      @ldap.search(filter: "uid=user1", base: "ou=People,dc=rubyldap,dc=com", ignore_server_caps: true)
+    end
+
+    assert_equal 1, events.size
+  end
+
+  def test_nested_search_without_open
+    entries = []
+    nested_entry = nil
+
+    @ldap.search(filter: "(|(uid=user1)(uid=user2))", base: "ou=People,dc=rubyldap,dc=com") do |entry|
+      entries << entry.uid.first
+      nested_entry ||= @ldap.search(filter: "uid=user3", base: "ou=People,dc=rubyldap,dc=com").first
+    end
+
+    assert_equal "user3", nested_entry.uid.first
+    assert_equal %w(user1 user2), entries
+  end
+
+  def test_nested_search_with_open
+    entries = []
+    nested_entry = nil
+
+    @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 ||= @ldap.search(filter: "uid=user3", base: "ou=People,dc=rubyldap,dc=com").first
+      end
+    end
+
+    assert_equal "user3", nested_entry.uid.first
+    assert_equal %w(user1 user2), entries
+  end
+end

test/integration/test_return_codes.rb

@@ -29,7 +29,7 @@ def test_time_limit_exceeded
   end
 
   def test_size_limit_exceeded
-    refute @ldap.search(filter: "cn=sizeLimitExceeded", base: "ou=Retcodes,dc=rubyldap,dc=com")
+    @ldap.search(filter: "cn=sizeLimitExceeded", base: "ou=Retcodes,dc=rubyldap,dc=com")
     assert result = @ldap.get_operation_result
 
     assert_equal 4, result.code

test/integration/test_search.rb

@@ -28,9 +28,22 @@ def test_search_without_result
   def test_search_timeout
     events = @service.subscribe "search.net_ldap_connection"
 
-    @ldap.search(base: "dc=rubyldap,dc=com", time: 5)
+    result = @ldap.search(base: "dc=rubyldap,dc=com", time: 5)
 
     payload, result = events.pop
-    assert_equal 1, payload[:time]
+    assert_equal 5, payload[:time]
+    assert_equal entries, result
+  end
+
+  def test_search_with_size
+    entries = []
+
+    result = @ldap.search(filter: "(uid=user1)", base: "dc=rubyldap,dc=com", size: 1) do |entry|
+      assert_kind_of Net::LDAP::Entry, entry
+      entries << entry
+    end
+
+    refute entries.empty?
+    assert_equal entries, result
   end
 end

test/test_ldap.rb

@@ -40,4 +40,21 @@ def test_instrument_search
     assert_equal [entry], payload[:result]
     assert_equal "(uid=user1)", payload[:filter]
   end
+
+  def test_instrument_search_with_size
+    events = @service.subscribe "search.net_ldap"
+
+    flexmock(@connection).should_receive(:bind).and_return(flexmock(:bind_result, :result_code => 0))
+    flexmock(@connection).should_receive(:search).with(Hash, Proc).
+                yields(entry = Net::LDAP::Entry.new("uid=user1,ou=users,dc=example,dc=com")).
+                and_return(flexmock(:search_result, :success? => true, :result_code => 4))
+
+    refute_nil @subject.search(:filter => "(uid=user1)", :size => 1)
+
+    payload, result = events.pop
+    assert_equal [entry], result
+    assert_equal [entry], payload[:result]
+    assert_equal "(uid=user1)", payload[:filter]
+    assert_equal result.size, payload[:size]
+  end
 end

test/test_ldap_connection.rb

@@ -185,20 +185,21 @@ def test_bind_net_ldap_connection_event
 
   def test_search_net_ldap_connection_event
     # search data
-    search_data_ber = Net::BER::BerIdentifiedArray.new([2, [
+    search_data_ber = Net::BER::BerIdentifiedArray.new([1, [
       "uid=user1,ou=People,dc=rubyldap,dc=com",
       [ ["uid", ["user1"]] ]
     ]])
     search_data_ber.ber_identifier = Net::LDAP::PDU::SearchReturnedData
-    search_data = [2, search_data_ber]
+    search_data = [1, search_data_ber]
     # search result (end of results)
     search_result_ber = Net::BER::BerIdentifiedArray.new([0, "", ""])
     search_result_ber.ber_identifier = Net::LDAP::PDU::SearchResult
-    search_result = [2, search_result_ber]
+    search_result = [1, search_result_ber]
     @tcp_socket.should_receive(:read_ber).and_return(search_data).
                                           and_return(search_result)
 
     events = @service.subscribe "search.net_ldap_connection"
+    unread = @service.subscribe "search_messages_unread.net_ldap_connection"
 
     result = @connection.search(filter: "(uid=user1)", base: "ou=People,dc=rubyldap,dc=com")
     assert result.success?, "should be success"
@@ -209,5 +210,8 @@ def test_search_net_ldap_connection_event
     assert payload.has_key?(:filter)
     assert_equal "(uid=user1)", payload[:filter].to_s
     assert result
+
+    # ensure no unread
+    assert unread.empty?, "should not have any leftover unread messages"
   end
 end