Skip to content

Update parser.js#9

Closed
Vasya-Chajko wants to merge 1 commit intozont:masterfrom
Vasya-Chajko:parseLoop
Closed

Update parser.js#9
Vasya-Chajko wants to merge 1 commit intozont:masterfrom
Vasya-Chajko:parseLoop

Conversation

@Vasya-Chajko
Copy link

The response parser recursively retrieves messages, potentially causing a Node.js stack overflow. To fix this, it was refactored from a recursive to an iterative approach.

@ruinedme
Copy link

I do believe i fixed this already. #8

@Vasya-Chajko
Copy link
Author

I do believe i fixed this already. #8

The problem isn't infinite recursion, but the recursive processing of messages—when there are too many, the stack overflows.

@ruinedme
Copy link

I guess i'm unclear on what you mean by recursive processing of messages.

With my parserFix + your pagedResult fix i was able to run a query that returns 17k users with all attributes, with a page size of 10, and it works fine. (slow, but does work). If i understand processing of messages to be the number of responses sent back to the client that would be 1700 messages, and the messages would not be small either. Can you maybe provide an example query that was having this problem? I can see if i can reproduce on my end.

My query is something like (&(objectclass=*)(memberOf=${groupDN})) , all attributes, pageSize 10

I did test this patch + the pagedResult fix, I will say that if nothing else it is faster. With the recursive approach my query was taking ~1 minute 45 seconds. With this fix the same query now takes ~45 seconds.

@Vasya-Chajko
Copy link
Author

The issue occurs when processing search query responses. Whether paged search is used or not doesn't matter in this case. With your setup of 17k users and a page size of 10, this will execute 1,700 queries. Each found object corresponds to 1 message, meaning each response will contain exactly 10 messages. The responses will be parsed sequentially with 10 messages per response.

But if you make a search query that returns, for example, 900 users, they will come as a sequence of 900 messages, which will be processed recursively—and that could cause a stack overflow.

@ruinedme
Copy link

That makes sense. I'm not able to actually able to produce a stack overflow in this way though. I tested another group with 929 users, not setting page size, and returning all attributes. I was able to parse all the messages without issue.

Approximate timings
Recursive: ~8-10 seconds (Pretty large variance from run to run, likely network latency issues).
Iterative: ~4-10 seconds (The variance is even larger, but can be faster, likely network latency issues).

So again if nothing else the iterative approach does a least seem to be more performant.

@Vasya-Chajko
Copy link
Author

Vasya-Chajko commented Apr 24, 2025

and returning all attributes

If you request only selected attributes instead of all attributes, more objects will fit into a single chunk. This will cause problems with such queries.

client.search('OU=Courses Teachers,OU=Moodle,DC=nhtk,DC=su', {scope: 'sub', attributes: ['cn', 'description']})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants