Concurrent calls to client.search() failed when using "ldaps"

[javefang]

I'm wondering if the functions on a client can be called concurrently?

I have a small test application which takes a list of usernames, fetch their details from ldap using ldapjs and print them to the console. Currently I implement it in the way that it create a single ldapjs client, bind it and then call "search" for each of the usernames concurrently.

It works fine when I'm connecting with unsecured "ldap" protocol. But when switched to "ldaps", all search still returns the event emitter, but some of them will never call "end" or "error".

I'm wondering if it is safe to call another search while one is still not finished? Thanks.

Environment: windows 7 node v0.12.9 ldapjs 1.0.0

[javefang]

might have found the problem:

in lib/client/client.js, on the last few lines of function "Client.prototype._sendSocket",

conn.write(message.toBer(), writeCallback);

If I understand correctly, this guarantees that the message (search request) is written to the socket before calling writeCallback, which will return the event emitter back to the caller. However, this might be too late in some cases. As I have noticed that the "end" event is fired before the callback of "search" is called. In which case the caller will never be able to receive that event as it hasn't got a chance to register event handler yet.

Maybe we should make sure the emitter is returned to the caller so that they have the chance to register event handler before any event arrive? My initial thought is as follow. Hopefully you could provide more help on this?

try {
    writeCallback();
    return conn.write(message.toBer());

  } catch (e) {
    if (timer)
      clearTimeout(timer);

    log.trace({err: e}, 'Error writing message to socket');
    emitter.emit('error', e);
  }

[javefang]

btw, the issue I'm having only seem to affect "ldaps" protocol so far. and the above fix works for me.

[pfmooney]

@javefang Could you collect trace-level logs from this situation?

[Loki-Afro]

any plan in integrating this? there is also a fork with this fix only based on an old version .. https://github.com/hpi-schul-cloud/node-ldapjs/commit/3d470ab339c203560ad9816158dfb5695fff36ab

[UziTech]

If someone wants to create a PR with the fix we can look at it. Make sure to add a test as well.

[jsumners]

👋

On February 22, 2023, we released version 3 of this library. As a result, we are closing this issue/pull request.

Please see issue #839 for more information, including how to proceed if you feel this closure is in error.