From 324d3c5dcbb19cd2e44ec53235ba1e90880d86f7 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Thu, 16 Jul 2026 15:32:35 +0000
Subject: [PATCH] [#739] Fix alias dereferencing dropping entries and accumulating DNs (#742)
---
opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java | 7 +
opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java | 226 +++++++++++++++++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java | 105 +++++++++++++++--
opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java | 3
opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java | 9 +
5 files changed, 335 insertions(+), 15 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java
index 3ddefde..3b06726 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperation.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2009 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.core;
@@ -263,6 +264,14 @@
boolean evaluateAci);
/**
+ * Indicates that the search phase is over and that any further entry comes from a persistent
+ * search. State kept to dereference aliases during the search phase is released, and no further
+ * entry is matched against it: a persistent search must report every change it is notified of,
+ * whether or not the entry was returned by the search phase.
+ */
+ void endSearchPhase();
+
+ /**
* Used as a callback for backends to indicate that the provided search
* reference was encountered during processing and that additional processing
* should be performed to potentially send it back to the client.
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java
index 00a8074..9b3bb83 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationBasis.java
@@ -13,11 +13,12 @@
*
* Copyright 2006-2010 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
- * Portions Copyright 2024 3A Systems, LLC.
+ * Portions Copyright 2024-2026 3A Systems, LLC.
*/
package org.opends.server.core;
import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import org.forgerock.i18n.LocalizedIllegalArgumentException;
@@ -449,12 +450,47 @@
{
return returnEntry(entry, controls, true);
}
- Set<DN> dereferenced=new HashSet<>();
+
+ /**
+ * The DNs of the entries already returned by the search phase. An alias may be dereferenced onto
+ * an entry which is in the scope of the search as well, and that entry must only be returned once.
+ * It is emptied once the search phase is over, because a persistent search must report every
+ * change it is notified of, whether or not the entry was returned before.
+ */
+ private final Set<DN> returnedDNs = ConcurrentHashMap.newKeySet();
+
+ /** Whether the search phase is over and only persistent search notifications remain. */
+ private volatile boolean searchPhaseOver;
+
+ @Override
+ public final void endSearchPhase()
+ {
+ searchPhaseOver = true;
+ returnedDNs.clear();
+ }
@Override
public final boolean returnEntry(Entry entry, List<Control> controls,
boolean evaluateAci)
{
+ return returnEntry(entry, controls, evaluateAci, null);
+ }
+
+ /**
+ * Returns the provided entry to the client, dereferencing it first if it is an alias and the
+ * deref policy asks for it.
+ *
+ * @param entry The entry to return.
+ * @param controls The controls to attach to the entry.
+ * @param evaluateAci Whether the access control handler must be consulted.
+ * @param aliasChain The DNs of the aliases already dereferenced on the way to this entry, or
+ * {@code null} if no alias was dereferenced yet. It only spans the current
+ * chain, so it cannot grow beyond the length of that chain.
+ * @return {@code true} if the search should continue, {@code false} if it should stop.
+ */
+ private boolean returnEntry(Entry entry, List<Control> controls,
+ boolean evaluateAci, Set<DN> aliasChain)
+ {
boolean typesOnly = getTypesOnly();
// See if the size limit has been exceeded. If so, then don't send the
@@ -566,22 +602,12 @@
//DereferenceAliasesPolicy
if ( DereferenceAliasesPolicy.ALWAYS.equals(getDerefPolicy()) || DereferenceAliasesPolicy.IN_SEARCHING.equals(getDerefPolicy()) ) {
if (entry.isAlias() && !baseDN.equals(entry.getName())) {
- try {
- final DN dn = entry.getAliasedDN();
- final Entry dereference = DirectoryServer.getEntry(dn);
- if (dereferenced.contains(dn)) {
- return true;
- }
- dereferenced.add(dn);
- return returnEntry(dereference, controls, true);
- } catch (DirectoryException e) {
- throw new RuntimeException(e);
- }
+ return returnAliasedEntry(entry, controls, aliasChain);
}
- if (dereferenced.contains(entry.getName())) {
+ if (!searchPhaseOver && !returnedDNs.add(entry.getName())) {
+ // This entry was already returned by the search, through an alias or on its own.
return true;
}
- dereferenced.add(entry.getName());
}
// Make a copy of the entry and pare it down to only include the set
@@ -709,6 +735,55 @@
return pluginResult.continueProcessing();
}
+ /**
+ * Returns the entry the provided alias points to, in place of the alias itself.
+ *
+ * @param alias The alias entry to dereference.
+ * @param controls The controls to attach to the entry.
+ * @param aliasChain The DNs of the aliases already dereferenced on the way to this alias, or
+ * {@code null} if this alias is the first one of the chain.
+ * @return {@code true} if the search should continue, {@code false} if it should stop.
+ */
+ private boolean returnAliasedEntry(Entry alias, List<Control> controls, Set<DN> aliasChain)
+ {
+ final DN aliasedDN;
+ final Entry aliasedEntry;
+ try
+ {
+ // getAliasedDN() parses the aliasedObjectName value, which throws the unchecked
+ // LocalizedIllegalArgumentException when that value is not a valid DN.
+ aliasedDN = alias.getAliasedDN();
+ aliasedEntry = DirectoryServer.getEntry(aliasedDN);
+ }
+ catch (DirectoryException | LocalizedIllegalArgumentException e)
+ {
+ // A single alias which cannot be read must not fail the whole search.
+ logger.traceException(e);
+ return true;
+ }
+
+ if (aliasedEntry == null)
+ {
+ // The alias points to an entry which does not exist: there is nothing to return for it.
+ return true;
+ }
+ if (!searchPhaseOver && returnedDNs.contains(aliasedDN))
+ {
+ // The aliased entry was already returned by the search.
+ return true;
+ }
+ if (aliasChain == null)
+ {
+ aliasChain = new HashSet<>();
+ }
+ if (!aliasChain.add(aliasedDN))
+ {
+ // The aliases point at each other: stop before looping forever.
+ return true;
+ }
+ return returnEntry(aliasedEntry, controls, true, aliasChain);
+ }
+
private AccessControlHandler<?> getACIHandler()
{
return AccessControlConfigManager.getInstance().getAccessControlHandler();
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java
index 42afbd3..5bc8617 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/core/SearchOperationWrapper.java
@@ -13,6 +13,7 @@
*
* Copyright 2008-2009 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.core;
@@ -58,6 +59,12 @@
}
@Override
+ public void endSearchPhase()
+ {
+ getOperation().endSearchPhase();
+ }
+
+ @Override
public boolean returnReference(DN dn, SearchResultReference reference)
{
return getOperation().returnReference(dn, reference);
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
index 8d9db9b..7b47d98 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
@@ -258,6 +258,9 @@
// Process the search in the backend and all its subordinates.
backend.search(this);
}
+
+ // Any entry returned from now on comes from the persistent search, if there is one.
+ endSearchPhase();
}
catch (DirectoryException de)
{
diff --git a/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java b/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java
index 75c2def..b3360fe 100644
--- a/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/openidentityplatform/opendj/AliasTestCase.java
@@ -15,21 +15,33 @@
*/
package org.openidentityplatform.opendj;
+import org.forgerock.opendj.config.client.ManagementContext;
import org.forgerock.opendj.ldap.*;
+import org.forgerock.opendj.ldap.controls.PersistentSearchChangeType;
+import org.forgerock.opendj.ldap.controls.PersistentSearchRequestControl;
import org.forgerock.opendj.ldap.requests.Requests;
import org.forgerock.opendj.ldap.requests.SearchRequest;
import org.forgerock.opendj.ldap.responses.SearchResultEntry;
+import org.forgerock.opendj.ldap.responses.SearchResultReference;
import org.forgerock.opendj.ldif.ConnectionEntryReader;
+import org.forgerock.opendj.server.config.client.GlobalCfgClient;
+import org.forgerock.opendj.server.config.meta.GlobalCfgDefn;
import org.opends.server.DirectoryServerTestCase;
import org.opends.server.TestCaseUtils;
+import org.opends.server.api.LocalBackend;
import org.opends.server.backends.MemoryBackend;
+import org.opends.server.core.DirectoryServer;
+import org.opends.server.types.AcceptRejectWarn;
import org.opends.server.types.Entry;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import java.util.HashMap;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
@@ -464,6 +476,220 @@
}
}
+ // An alias whose target lies outside the search scope: the target is only
+ // reachable by dereferencing, so it must be returned by the search.
+ @Test
+ public void test_deref_target_outside_scope() throws Exception {
+ TestCaseUtils.addEntries(
+ "dn: ou=pointers,o=test",
+ "objectClass: top",
+ "objectClass: organizationalUnit",
+ "ou: pointers",
+ "",
+ "dn: cn=ptr,ou=pointers,o=test",
+ "objectClass: alias",
+ "objectClass: top",
+ "objectClass: extensibleObject",
+ "cn: ptr",
+ "aliasedObjectName: cn=John Doe,o=MyCompany,o=test",
+ ""
+ );
+ HashMap<String, SearchResultEntry> res =
+ search("ou=pointers,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS);
+
+ assertThat(res.containsKey("cn=John Doe,o=MyCompany,o=test")).isTrue();
+ }
+
+ // An alias pointing at an entry which does not exist is skipped, and the entries next to it are
+ // still returned: aliases are not checked when they are written, so a dangling one is expected.
+ @Test
+ public void test_dangling_alias() throws Exception {
+ TestCaseUtils.addEntries(
+ "dn: ou=dangling,o=test",
+ "objectClass: top",
+ "objectClass: organizationalUnit",
+ "ou: dangling",
+ "",
+ "dn: cn=broken,ou=dangling,o=test",
+ "objectClass: alias",
+ "objectClass: top",
+ "objectClass: extensibleObject",
+ "cn: broken",
+ "aliasedObjectName: cn=does-not-exist,ou=dangling,o=test",
+ "",
+ "dn: cn=intact,ou=dangling,o=test",
+ "cn: intact",
+ "sn: intact",
+ "objectClass: person",
+ ""
+ );
+ HashMap<String, SearchResultEntry> res =
+ search("ou=dangling,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS);
+
+ assertThat(res.containsKey("cn=does-not-exist,ou=dangling,o=test")).isFalse();
+ assertThat(res.containsKey("ou=dangling,o=test")).isTrue();
+ assertThat(res.containsKey("cn=intact,ou=dangling,o=test")).isTrue();
+ }
+
+ // A persistent search reports every change it is notified of. Dereferencing aliases must not
+ // make it drop a change because the entry was notified before.
+ @Test
+ public void test_persistent_search_reports_repeated_changes() throws Exception {
+ TestCaseUtils.addEntries(
+ "dn: ou=psearch,o=test",
+ "objectClass: top",
+ "objectClass: organizationalUnit",
+ "ou: psearch",
+ "",
+ "dn: cn=changing,ou=psearch,o=test",
+ "cn: changing",
+ "sn: changing",
+ "objectClass: person",
+ ""
+ );
+
+ final BlockingQueue<String> notified = new LinkedBlockingQueue<>();
+ final SearchRequest request =
+ Requests.newSearchRequest("ou=psearch,o=test", SearchScope.WHOLE_SUBTREE, "(objectclass=*)")
+ .setDereferenceAliasesPolicy(DereferenceAliasesPolicy.ALWAYS)
+ .addControl(PersistentSearchRequestControl.newControl(
+ true, true, false, PersistentSearchChangeType.MODIFY));
+
+ final LDAPConnectionFactory factory =
+ new LDAPConnectionFactory("localhost", TestCaseUtils.getServerLdapPort());
+ try (Connection psearch = factory.getConnection()) {
+ psearch.bind("cn=Directory Manager", "password".toCharArray());
+ psearch.searchAsync(request, new SearchResultHandler() {
+ @Override
+ public boolean handleEntry(SearchResultEntry entry) {
+ notified.add(entry.getName().toString());
+ return true;
+ }
+
+ @Override
+ public boolean handleReference(SearchResultReference reference) {
+ return true;
+ }
+ });
+
+ // searchAsync returns before the server has registered the persistent search, so wait
+ // until the backend reports it; otherwise the first modification below can be notified
+ // before the search is listening and be missed.
+ final LocalBackend<?> backend = TestCaseUtils.getServerContext()
+ .getBackendConfigManager().getLocalBackendById(TestCaseUtils.TEST_BACKEND_ID);
+ for (int i = 0; backend.getPersistentSearches().isEmpty() && i < 500; i++) {
+ Thread.sleep(10);
+ }
+ assertThat(backend.getPersistentSearches()).isNotEmpty();
+
+ // The same entry is modified repeatedly: each change must reach the persistent search.
+ for (int i = 1; i <= 3; i++) {
+ connection.modify(Requests.newModifyRequest("cn=changing,ou=psearch,o=test")
+ .addModification(ModificationType.REPLACE, "description", "change " + i));
+ assertThat(notified.poll(30, TimeUnit.SECONDS))
+ .as("notification for change " + i)
+ .isEqualTo("cn=changing,ou=psearch,o=test");
+ }
+ }
+ }
+
+ // An alias is dereferenced before its target is reached on its own: the target must still be
+ // returned, and exactly once. The original regression was order-sensitive, dropping the target
+ // when the alias reached it first, so this pins the alias-before-target order specifically. The
+ // MemoryBackend serving o=test walks entries in insertion order, so listing the alias before the
+ // target below guarantees the alias is visited first.
+ @Test
+ public void test_deref_alias_before_target_returns_target_once() throws Exception {
+ TestCaseUtils.addEntries(
+ "dn: ou=order,o=test",
+ "objectClass: top",
+ "objectClass: organizationalUnit",
+ "ou: order",
+ "",
+ "dn: cn=aaa-alias,ou=order,o=test",
+ "objectClass: alias",
+ "objectClass: top",
+ "objectClass: extensibleObject",
+ "cn: aaa-alias",
+ "aliasedObjectName: cn=zzz-target,ou=order,o=test",
+ "",
+ "dn: cn=zzz-target,ou=order,o=test",
+ "cn: zzz-target",
+ "sn: target",
+ "objectClass: person",
+ ""
+ );
+ // search() asserts every returned DN is unique, so a duplicated target would fail this call.
+ HashMap<String, SearchResultEntry> res =
+ search("ou=order,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS);
+
+ // The target is returned, reached through the alias visited before it ...
+ assertThat(res.containsKey("cn=zzz-target,ou=order,o=test")).isTrue();
+ // ... and the alias entry itself is replaced by its target, never returned as-is.
+ assertThat(res.containsKey("cn=aaa-alias,ou=order,o=test")).isFalse();
+ }
+
+ // An alias whose aliasedObjectName is not a valid DN cannot be dereferenced. Reading it throws a
+ // DirectoryException, which must be logged and the alias skipped, not turned into a failed search.
+ @Test
+ public void test_alias_with_malformed_target_is_skipped() throws Exception {
+ // A malformed DN can only be stored while syntax enforcement is relaxed; the default REJECT
+ // policy refuses the add outright.
+ final AcceptRejectWarn previous = DirectoryServer.getCoreConfigManager().getSyntaxEnforcementPolicy();
+ setSyntaxEnforcementPolicy(GlobalCfgDefn.InvalidAttributeSyntaxBehavior.WARN);
+ try {
+ TestCaseUtils.addEntries(
+ "dn: ou=malformed,o=test",
+ "objectClass: top",
+ "objectClass: organizationalUnit",
+ "ou: malformed",
+ "",
+ "dn: cn=badptr,ou=malformed,o=test",
+ "objectClass: alias",
+ "objectClass: top",
+ "objectClass: extensibleObject",
+ "cn: badptr",
+ "aliasedObjectName: not a valid dn",
+ "",
+ "dn: cn=neighbour,ou=malformed,o=test",
+ "cn: neighbour",
+ "sn: neighbour",
+ "objectClass: person",
+ ""
+ );
+ } finally {
+ setSyntaxEnforcementPolicy(asSyntaxBehavior(previous));
+ }
+
+ // The whole search still succeeds: the broken alias is skipped, its neighbours returned.
+ HashMap<String, SearchResultEntry> res =
+ search("ou=malformed,o=test", SearchScope.WHOLE_SUBTREE, DereferenceAliasesPolicy.ALWAYS);
+
+ assertThat(res.containsKey("cn=badptr,ou=malformed,o=test")).isFalse();
+ assertThat(res.containsKey("ou=malformed,o=test")).isTrue();
+ assertThat(res.containsKey("cn=neighbour,ou=malformed,o=test")).isTrue();
+ }
+
+ private static void setSyntaxEnforcementPolicy(GlobalCfgDefn.InvalidAttributeSyntaxBehavior value) throws Exception {
+ try (ManagementContext conf = TestCaseUtils.getServer().getConfiguration()) {
+ GlobalCfgClient globalCfg = conf.getRootConfiguration().getGlobalConfiguration();
+ globalCfg.setInvalidAttributeSyntaxBehavior(value);
+ globalCfg.commit();
+ }
+ }
+
+ private static GlobalCfgDefn.InvalidAttributeSyntaxBehavior asSyntaxBehavior(AcceptRejectWarn policy) {
+ switch (policy) {
+ case ACCEPT:
+ return GlobalCfgDefn.InvalidAttributeSyntaxBehavior.ACCEPT;
+ case WARN:
+ return GlobalCfgDefn.InvalidAttributeSyntaxBehavior.WARN;
+ case REJECT:
+ default:
+ return GlobalCfgDefn.InvalidAttributeSyntaxBehavior.REJECT;
+ }
+ }
+
@Test(expectedExceptions = LdapException.class)
public void test_stackoverflow() throws Exception {
--
Gitblit v1.10.0