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

Valery Kharseko

23 hours ago 324d3c5dcbb19cd2e44ec53235ba1e90880d86f7

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

 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.

 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();

 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);

 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)
    {

 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 {