From 2932826ed4d0c00eea2ceff19d65476593919821 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Wed, 29 Jun 2011 17:49:33 +0000
Subject: [PATCH] Fix OPENDJ-216: Missing entries in searches when legacy (objectclass=ldapSubEntry) filter is used in Search filter.
---
opends/src/server/org/opends/server/core/SearchOperationBasis.java | 78 +++++++++----------
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java | 3
opends/src/server/org/opends/server/core/SearchOperationWrapper.java | 9 +-
opends/tests/unit-tests-testng/src/server/org/opends/server/core/SearchOperationTestCase.java | 109 +++++++++++++++++++++-----
opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java | 2
opends/src/server/org/opends/server/core/SearchOperation.java | 5
6 files changed, 135 insertions(+), 71 deletions(-)
diff --git a/opends/src/server/org/opends/server/core/SearchOperation.java b/opends/src/server/org/opends/server/core/SearchOperation.java
index c69a599..5a1ec3a 100644
--- a/opends/src/server/org/opends/server/core/SearchOperation.java
+++ b/opends/src/server/org/opends/server/core/SearchOperation.java
@@ -23,6 +23,7 @@
*
*
* Copyright 2006-2009 Sun Microsystems, Inc.
+ * Portions Copyright 2011 ForgeRock AS
*/
package org.opends.server.core;
@@ -318,7 +319,7 @@
*
* @return true if the LDAP subentries should be returned, false otherwise
*/
- public abstract boolean isReturnLDAPSubentries();
+ public abstract boolean isReturnSubentriesOnly();
/**
* Set the flag indicating wether the LDAP subentries should be returned.
@@ -326,7 +327,7 @@
* @param returnLDAPSubentries - Boolean indicating wether the LDAP
* subentries should be returned or not
*/
- public abstract void setReturnLDAPSubentries(boolean returnLDAPSubentries);
+ public abstract void setReturnSubentriesOnly(boolean returnLDAPSubentries);
/**
* The matched values control associated with this search operation.
diff --git a/opends/src/server/org/opends/server/core/SearchOperationBasis.java b/opends/src/server/org/opends/server/core/SearchOperationBasis.java
index ace7465..c266177 100644
--- a/opends/src/server/org/opends/server/core/SearchOperationBasis.java
+++ b/opends/src/server/org/opends/server/core/SearchOperationBasis.java
@@ -89,8 +89,12 @@
// Indicates whether to only real attributes should be returned.
private boolean realAttributesOnly;
- // Indicates whether LDAP subentries should be returned.
- private boolean returnLDAPSubentries;
+ // Indicates whether only LDAP subentries should be returned.
+ private boolean returnSubentriesOnly;
+
+ // Indicates whether the filter references subentry or ldapSubentry object
+ // class.
+ private boolean filterIncludesSubentries;
// Indicates whether to include attribute types only or both types and values.
private boolean typesOnly;
@@ -246,7 +250,7 @@
clientAcceptsReferrals = true;
includeUsableControl = false;
responseSent = new AtomicBoolean(false);
- returnLDAPSubentries = false;
+ returnSubentriesOnly = false;
matchedValuesControl = null;
realAttributesOnly = false;
virtualAttributesOnly = false;
@@ -346,7 +350,7 @@
clientAcceptsReferrals = true;
includeUsableControl = false;
responseSent = new AtomicBoolean(false);
- returnLDAPSubentries = false;
+ returnSubentriesOnly = false;
matchedValuesControl = null;
}
@@ -517,6 +521,7 @@
if (filter == null)
{
filter = rawFilter.toSearchFilter();
+ filterIncludesSubentries = checkFilterForLDAPSubEntry(filter, 0);
}
}
catch (DirectoryException de)
@@ -612,27 +617,16 @@
// should be returned.
if (entry.isSubentry() || entry.isLDAPSubentry())
{
- if ((getScope() != SearchScope.BASE_OBJECT) &&
- (! isReturnLDAPSubentries()))
+ if ((getScope() != SearchScope.BASE_OBJECT)
+ && !filterIncludesSubentries
+ && !isReturnSubentriesOnly())
{
- // Check to see if the filter contains an equality element with the
- // objectclass attribute type and a value of "ldapSubentry". If so,
- // then we'll return it anyway. Technically, this isn't part of the
- // specification so we don't need to get carried away with really in
- // depth checks. Just do best effort for earlier draft compatibility.
- checkFilterForLDAPSubEntry(getFilter(), 0);
-
- if (! isReturnLDAPSubentries())
- {
- // We still shouldn't return it even based on the filter.
- // Just throw it away without doing anything.
- return true;
- }
+ return true;
}
}
else
{
- if (isReturnLDAPSubentries())
+ if (isReturnSubentriesOnly())
{
// Subentries are visible and normal entries are not.
return true;
@@ -1148,17 +1142,17 @@
/**
* {@inheritDoc}
*/
- public boolean isReturnLDAPSubentries()
+ public boolean isReturnSubentriesOnly()
{
- return returnLDAPSubentries;
+ return returnSubentriesOnly;
}
/**
* {@inheritDoc}
*/
- public void setReturnLDAPSubentries(boolean returnLDAPSubentries)
+ public void setReturnSubentriesOnly(boolean returnLDAPSubentries)
{
- this.returnLDAPSubentries = returnLDAPSubentries;
+ this.returnSubentriesOnly = returnLDAPSubentries;
}
/**
@@ -1464,19 +1458,22 @@
}
+
/**
- * Checks if the filter contains an equality element with the
- * objectclass attribute type and a value of "ldapSubentry"
- * and if so sets returnLDAPSubentries to <code>true</code>.
+ * Checks if the filter contains an equality element with the objectclass
+ * attribute type and a value of "ldapSubentry" and if so sets
+ * returnSubentriesOnly to <code>true</code>.
*
- * @param filter The complete filter being checked, of which
- * this filter may be a subset.
- * @param depth The current depth of the evaluation, which
- * is used to prevent infinite recursion due
- * to highly nested filters and eventually
- * running out of stack space.
+ * @param filter
+ * The complete filter being checked, of which this filter may be a
+ * subset.
+ * @param depth
+ * The current depth of the evaluation, which is used to prevent
+ * infinite recursion due to highly nested filters and eventually
+ * running out of stack space.
+ * @return {@code true} if the filter references the sub-entry object class.
*/
- private void checkFilterForLDAPSubEntry(SearchFilter filter, int depth)
+ private boolean checkFilterForLDAPSubEntry(SearchFilter filter, int depth)
{
// Paranoid check to avoid recursion deep enough to provoke
// the stack overflow. This should never happen because if
@@ -1488,7 +1485,7 @@
{
TRACER.debugError("Exceeded maximum filter depth");
}
- return;
+ return false;
}
switch (filter.getFilterType())
@@ -1503,7 +1500,7 @@
if (stringValueLC.equals(OC_LDAP_SUBENTRY_LC) ||
stringValueLC.equals(OC_SUBENTRY))
{
- setReturnLDAPSubentries(true);
+ return true;
}
}
break;
@@ -1511,15 +1508,14 @@
case OR:
for (SearchFilter f : filter.getFilterComponents())
{
- checkFilterForLDAPSubEntry(f, depth + 1);
-
- if (isReturnLDAPSubentries())
+ if (checkFilterForLDAPSubEntry(f, depth + 1))
{
- // No point in continuing.
- break;
+ return true;
}
}
break;
}
+
+ return false;
}
}
diff --git a/opends/src/server/org/opends/server/core/SearchOperationWrapper.java b/opends/src/server/org/opends/server/core/SearchOperationWrapper.java
index ba54d02..de8b75b 100644
--- a/opends/src/server/org/opends/server/core/SearchOperationWrapper.java
+++ b/opends/src/server/org/opends/server/core/SearchOperationWrapper.java
@@ -23,6 +23,7 @@
*
*
* Copyright 2008-2009 Sun Microsystems, Inc.
+ * Portions Copyright 2011 ForgeRock AS
*/
package org.opends.server.core;
@@ -300,17 +301,17 @@
/**
* {@inheritDoc}
*/
- public boolean isReturnLDAPSubentries()
+ public boolean isReturnSubentriesOnly()
{
- return search.isReturnLDAPSubentries();
+ return search.isReturnSubentriesOnly();
}
/**
* {@inheritDoc}
*/
- public void setReturnLDAPSubentries(boolean returnLDAPSubentries)
+ public void setReturnSubentriesOnly(boolean returnLDAPSubentries)
{
- search.setReturnLDAPSubentries(returnLDAPSubentries);
+ search.setReturnSubentriesOnly(returnLDAPSubentries);
}
/**
diff --git a/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java b/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java
index 46246b0..b1dbd07 100644
--- a/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java
@@ -545,7 +545,7 @@
{
SubentriesControl subentriesControl =
getRequestControl(SubentriesControl.DECODER);
- setReturnLDAPSubentries(subentriesControl.getVisibility());
+ setReturnSubentriesOnly(subentriesControl.getVisibility());
}
else if (oid.equals(OID_MATCHED_VALUES))
{
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
index ae387e4..7ae0f00 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
@@ -23,6 +23,7 @@
*
*
* Copyright 2008-2010 Sun Microsystems, Inc.
+ * Portions copyright 2011 ForgeRock AS
*/
package org.opends.server.workflowelement.localbackend;
@@ -512,7 +513,7 @@
{
SubentriesControl subentriesControl =
getRequestControl(SubentriesControl.DECODER);
- setReturnLDAPSubentries(subentriesControl.getVisibility());
+ setReturnSubentriesOnly(subentriesControl.getVisibility());
}
else if (oid.equals(OID_MATCHED_VALUES))
{
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/core/SearchOperationTestCase.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/core/SearchOperationTestCase.java
index 87f49db..238ed16 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/core/SearchOperationTestCase.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/core/SearchOperationTestCase.java
@@ -23,6 +23,7 @@
*
*
* Copyright 2006-2010 Sun Microsystems, Inc.
+ * Portions copyright 2011 ForgeRock AS
*/
package org.opends.server.core;
@@ -31,17 +32,12 @@
import java.io.IOException;
import java.net.Socket;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
import org.opends.server.TestCaseUtils;
import org.opends.server.controls.MatchedValuesControl;
import org.opends.server.controls.MatchedValuesFilter;
+import org.opends.server.controls.SubentriesControl;
import org.opends.server.plugins.InvocationCounterPlugin;
import org.opends.server.protocols.asn1.ASN1Exception;
import org.opends.server.protocols.internal.InternalClientConnection;
@@ -122,6 +118,18 @@
assertNotNull(DirectoryServer.getEntry(baseEntry.getDN()));
}
+ // Add a test ldapsubentry.
+ Entry ldapSubentry = TestCaseUtils.makeEntry(
+ "dn: cn=subentry," + BASE,
+ "objectclass: ldapsubentry");
+ AddOperation addOperation =
+ connection.processAdd(ldapSubentry.getDN(),
+ ldapSubentry.getObjectClasses(),
+ ldapSubentry.getUserAttributes(),
+ ldapSubentry.getOperationalAttributes());
+ assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS);
+ assertNotNull(DirectoryServer.getEntry(ldapSubentry.getDN()));
+
// Add a test entry.
testEntry = TestCaseUtils.makeEntry(
"dn: uid=rogasawara," + BASE,
@@ -162,7 +170,7 @@
// The add operation changes the attributes, so let's duplicate the entry.
Entry duplicateEntry = testEntry.duplicate(false);
- AddOperation addOperation =
+ addOperation =
connection.processAdd(duplicateEntry.getDN(),
duplicateEntry.getObjectClasses(),
duplicateEntry.getUserAttributes(),
@@ -170,18 +178,6 @@
assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS);
assertNotNull(DirectoryServer.getEntry(testEntry.getDN()));
- // Add a test ldapsubentry.
- Entry ldapSubentry = TestCaseUtils.makeEntry(
- "dn: cn=subentry," + BASE,
- "objectclass: ldapsubentry");
- addOperation =
- connection.processAdd(ldapSubentry.getDN(),
- ldapSubentry.getObjectClasses(),
- ldapSubentry.getUserAttributes(),
- ldapSubentry.getOperationalAttributes());
- assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS);
- assertNotNull(DirectoryServer.getEntry(ldapSubentry.getDN()));
-
// Add a test referral entry.
Entry referralEntry = TestCaseUtils.makeEntry(
"dn: ou=People," + BASE,
@@ -890,7 +886,7 @@
}
@Test
- public void testSearchInternalSubEntry() throws Exception
+ public void testSearchInternalSubEntryEqualityFilter() throws Exception
{
InvocationCounterPlugin.resetAllCounters();
@@ -917,8 +913,47 @@
assertEquals(searchOperation.getResultCode(), ResultCode.SUCCESS);
assertEquals(searchOperation.getEntriesSent(), 1);
assertEquals(searchOperation.getErrorMessage().length(), 0);
+ }
- searchOperation =
+ @Test
+ public void testSearchInternalSubEntryControl() throws Exception
+ {
+ InvocationCounterPlugin.resetAllCounters();
+
+ InternalClientConnection conn =
+ InternalClientConnection.getRootConnection();
+
+ InternalSearchOperation searchOperation =
+ new InternalSearchOperation(
+ conn,
+ InternalClientConnection.nextOperationID(),
+ InternalClientConnection.nextMessageID(),
+ Collections.singletonList((Control)new SubentriesControl(true, true)),
+ ByteString.valueOf(BASE),
+ SearchScope.WHOLE_SUBTREE,
+ DereferencePolicy.NEVER_DEREF_ALIASES,
+ Integer.MAX_VALUE,
+ Integer.MAX_VALUE,
+ false,
+ LDAPFilter.decode("(objectclass=*)"),
+ null, null);
+
+ searchOperation.run();
+
+ assertEquals(searchOperation.getResultCode(), ResultCode.SUCCESS);
+ assertEquals(searchOperation.getEntriesSent(), 1);
+ assertEquals(searchOperation.getErrorMessage().length(), 0);
+ }
+
+ @Test
+ public void testSearchInternalSubEntryAndFilter() throws Exception
+ {
+ InvocationCounterPlugin.resetAllCounters();
+
+ InternalClientConnection conn =
+ InternalClientConnection.getRootConnection();
+
+ InternalSearchOperation searchOperation =
new InternalSearchOperation(
conn,
InternalClientConnection.nextOperationID(),
@@ -962,6 +997,36 @@
}
@Test
+ public void testSearchInternalSubEntryOrFilter() throws Exception
+ {
+ InvocationCounterPlugin.resetAllCounters();
+
+ InternalClientConnection conn =
+ InternalClientConnection.getRootConnection();
+
+ InternalSearchOperation searchOperation =
+ new InternalSearchOperation(
+ conn,
+ InternalClientConnection.nextOperationID(),
+ InternalClientConnection.nextMessageID(),
+ new ArrayList<Control>(),
+ ByteString.valueOf(BASE),
+ SearchScope.WHOLE_SUBTREE,
+ DereferencePolicy.NEVER_DEREF_ALIASES,
+ Integer.MAX_VALUE,
+ Integer.MAX_VALUE,
+ false,
+ LDAPFilter.decode("(|(objectclass=ldapsubentry)(objectclass=top))"),
+ null, null);
+
+ searchOperation.run();
+
+ assertEquals(searchOperation.getResultCode(), ResultCode.SUCCESS);
+ assertEquals(searchOperation.getEntriesSent(), 3);
+ assertEquals(searchOperation.getErrorMessage().length(), 0);
+ }
+
+ @Test
public void testSearchInternalMatchedDN() throws Exception
{
InvocationCounterPlugin.resetAllCounters();
--
Gitblit v1.10.0