Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Client.search overwrites earlier searches before completion #278

Closed
jvanalst opened this issue Jul 7, 2015 · 9 comments
Closed

Client.search overwrites earlier searches before completion #278

jvanalst opened this issue Jul 7, 2015 · 9 comments
Assignees

Comments

@jvanalst
Copy link

jvanalst commented Jul 7, 2015

So I discovered that when I call LDAP search, if I do two searches in parallel (saying using Bluebird Promises), the first search never emits any events. There are no searchEntrys, no errors, and no end. It appears that even though API leaves the impression that you can perform two searches across the same client in parallel, you really can't the second search actually overwrites the first search, causing the first search to hang forever.

var ldap = require('ldap');
var Promise = require('bluebird');

var ims = ldap.createClient({
  url: 'ldaps://directory'
});

var imsSearch = Promise.promisify(ims.search, ims);
var imsSearchComplete = function (dn, opts) {
  return imsSearch(dn, opts).then(function (res) {
    return new Promise(function (resolve, reject) {
      var results = [];

      res.on('searchEntry', function (entry) {
        results.push(entry.object);
      });
      res.on('error', function (err) {
        if (err) throw err;
      });
      res.on('end', function () {
        resolve(results);
      });
    })
  });
};

Promise.props({
  classes: imsSearchComplete(
    'ou=calendar',
    {
      scope: 'sub',
      filter: '(&(id=667)(objectClass=OrgClass))'
    }
  ),
  persons: imsSearchComplete(
    'ou=people',
    {
      scope: 'sub',
      filter: '(objectClass=OrgPerson)'
    }
  )
}).then(function (results) {
  //this never fires because the results.classes promise never resolves because the 'end' event is never emitted. Only results.persons resolves correctly.
});
@jvanalst
Copy link
Author

jvanalst commented Jul 7, 2015

I.e. Even though the LDAP client API appears to imply making multiple calls at once to the same client is possible, in actuality you have to wait for each search to end before starting the next search.

@pfmooney
Copy link
Contributor

pfmooney commented Jul 7, 2015

It would be valuable to enable debug logging in ldapjs (via node-bunyan) to ensure that this issue is not caused by external control flow problems.

@pfmooney pfmooney self-assigned this Jul 7, 2015
@jvanalst
Copy link
Author

jvanalst commented Jul 8, 2015

I went to test this again to see if I could figure out the bunyan logger stuff and found the the order of the promises even affects whether they successfully finish or not.

If the persons search is second the classes search never finishes. But if classes is second they both finish successfully. Weird.

The persons search is expected to return oodles of records, the classes only 1.

@jvanalst
Copy link
Author

jvanalst commented Jul 8, 2015

I have a log but I have to figure out how to redact it and still keep it useful for you, because it's full of personal information I can't expose (it's also 300 lines long because of the number of persons retrieved from LDAP).

I can see both searches in the log. I can also see SearchEntries for the one that never emits events, as well, so it does ultimately get and parse that result from the server. Again the callbacks 'res' argument never triggers the appropriate events for some reason.

@jvanalst
Copy link
Author

jvanalst commented Jul 8, 2015

After analyzing the logs myself it did end up being the external flow control. Something about running promisify causes the callback registering the event listeners to be delayed until after those events had already fired in some instances.

When I rewrote it as a flat promise it worked as expected.

@jvanalst jvanalst closed this as completed Jul 8, 2015
@javefang
Copy link

Hi @jvanalst, I'm having exactly the same problem as you. I tried to write a flat promise as you have suggested but no luck. It works fine when I'm using "ldap", but not "ldaps". I saw you are also using ldaps, could you provide some details on how you solved the problem, please? Thanks.

'use strict';

var ldap = require('ldapjs');
var _ = require('lodash');
var bluebird = require('bluebird');

var LDAP_ADDR = 'ldaps://ldapserver';
var USER = 'user';
var PASSWORD = 'password';
var CAS = ['OUR_CA']

var log = console.log;

var users = ['user_a', 'user_b', 'user_c'];

var client = ldap.createClient({
  url: LDAP_ADDR,
  tlsOptions: {
    ca: CAS
  }
});

var base = 'OU=Users,DC=com';
var userCount = users.length;
var searchCount = 0;
var endCount = 0;

function searchAsync(user) {
  return new Promise(function (resolve, reject) {
    var opts = {
      filter: '(samAccountName=' + user + ')',
      scope: 'sub',
      attributes: ['dn']
    };

    client.search(base, opts, function(err, result) {
      var entries = [];

      var currentCount = ++searchCount;
      log('SEARCH_CB: %s/%s', currentCount, userCount);

      result.on('searchEntry', function (entry) {
        // log(entry.object.dn);
        entries.push(entry);
      });

      result.on('error', function (err) {
        log('ERROR');
        reject(err);
      });

      result.on('end', function (result) {
        endCount++;
        log('END: %s/%s', currentCount, userCount);
        resolve(entries);
      });
    });
  });
}

// callback style
client.bind(USER, PASSWORD, function () {
  return bluebird.map(users, searchAsync)
    .then(function () {
      log('unbind');
      client.unbind();
    });
});

@jvanalst
Copy link
Author

I create and end a new client as part of every search (promise). It's a bunch of overhead, but it seemed unavoidable.

@javefang
Copy link

@jvanalst I'm trying a possible solution which might work. #329

@jsumners
Copy link
Member

⚠️ This issue has been locked due to age. If you have encountered a recent
problem that seems to be covered by this issue, please open a new issue.

Please include a minimal reproducible example
when opening a new issue.

@ldapjs ldapjs locked as resolved and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants