[#739] Fix alias dereferencing dropping entries and accumulating DNs

[vharseko]

Fixes #739.

What was wrong

SearchOperationBasis.returnEntry kept a single Set<DN> dereferenced for the whole operation, meant to break alias loops. It recorded the DN of every entry the search returned and was never cleared. Three defects came out of that.

The dereferenced target was never returned. The target DN was added to the set on line 575, immediately before recursing onto it; the recursion then found its own DN in the set on line 581 and returned early without sending the entry. So an alias in the search results yielded neither itself nor its target — the result path only ever suppressed entries. The existing tests did not catch this because in all of them the target was independently in scope and happened to be returned before the alias (cn=John Doe sorts before cn=President), so the dereferencing path never had to deliver anything.

Persistent searches silently dropped changes. PersistentSearch holds its SearchOperation for the lifetime of the search and calls returnEntry per notification, so the set outlived the search phase. The first change to an entry put its DN in the set; every later change to the same entry hit dereferenced.contains(entry.getName()) and was dropped without reaching the client. This also bounds the growth reported in #739 — the set only ever grew by distinct DNs, so it is not the unbounded/OOM shape described there, but a correctness bug in change notification, which matters more for the sync clients that persistent search exists for.

The set was raced. Notifications run from the post-response callback of every modifying operation, i.e. from that operation's worker thread, so a plain HashSet was mutated concurrently.

Plus the two adjacent defects from the issue: a dangling alias produced an NPE (DirectoryServer.getEntry returns null, passed straight into the recursion) and a DirectoryException was wrapped in a RuntimeException — both degrading into result code 80 via the generic catch (Exception e) in LocalBackendSearchOperation.

Regression from f771315 (#369). The CVE-2025-27497 fix (08aee47) guards the base-DN path with its own properly scoped set and does not touch this code.

What changed

• The alias loop guard is now local to the alias chain: aliasChain is threaded through the recursion and allocated lazily, only once an alias is actually met, so a search without aliases allocates nothing. Scoped guard modelled on 08aee47.
• Deduplication of returned entries moved to its own returnedDNs, live only for the search phase. LocalBackendSearchOperation calls the new SearchOperation.endSearchPhase() after backend.search(this), which clears the set and stops matching against it, so persistent search notifications neither accumulate nor get dropped. The set is concurrent because a persistent search is registered before the search phase starts and its notifications can overlap it.
• An alias whose target does not exist is skipped instead of recursing onto null.
• A DirectoryException from reading an alias is logged and the alias skipped, rather than wrapped in a RuntimeException that fails the whole search. Propagating it as such would mean changing the SearchOperation.returnEntry signature, which is implemented by SearchOperationBasis, wrapped by SearchOperationWrapper and called from PersistentSearch and several backends.

Tests

Three tests added to AliasTestCase, each verified to fail without the fix:

• test_deref_target_outside_scope — an alias whose target is outside the scope: the target is now returned. Fails on master (target missing).
• test_persistent_search_reports_repeated_changes — a persistent search with derefAliases: always receives every change to the same entry. Fails on master (second notification never arrives, 30s timeout).
• test_dangling_alias — a dangling alias is skipped and its neighbours still returned, instead of error 80.

AliasTestCase 18/18. Also green: SearchOperationTestCase, PersistentSearchControlTest, ChangelogBackendTestCase, LDIFBackendTestCase — 151 tests total.

Open point for review

Deduplication across the result set is kept, so a search with always/search still retains one DN per returned entry for the duration of the search phase — #739 asks for this to shrink to the number of aliases. It cannot, while the "same entry is returned once" behaviour holds: the traversal order is arbitrary, so suppressing a duplicate requires knowing every DN already sent. That behaviour is what test_one_always, test_sub_search and test_sub_always assert, following the Oracle OID documentation those tests cite. Dropping it (duplicates are permitted by the spec, and OpenLDAP returns them) would reduce the state to the length of the alias chain but changes observable behaviour and rewrites those three tests. Happy to go that way if preferred.

One narrow pre-existing gap is left untouched: while the initial search phase is still running, an already-registered persistent search's notifications still go through the deduplication.

[maximthomas]

Test coverage gaps

Both are non-blocking follow-ups; the three added tests already cover the core fixes.

• DirectoryException catch branch is untested. test_dangling_alias exercises the aliasedEntry == null path, but not the catch (DirectoryException) branch in returnAliasedEntry. Add a test with a malformed aliasedObjectName (invalid DN syntax) to lock in the "log and skip, don't fail the whole search" behavior.

• No test pins dedup in the alias-first / target-in-scope order. The original regression was order-sensitive, and the existing always/sub tests only cover the target-before-alias ordering. Add a test where an alias is visited before an in-scope target and assert the target is returned exactly once, guarding the dedup path against future regressions.