From a3f92d339613d2703be9dba3d06e1e229fc69482 Mon Sep 17 00:00:00 2001
From: Yannick Lecaillez <yannick.lecaillez@forgerock.com>
Date: Tue, 19 May 2015 09:40:14 +0000
Subject: [PATCH] OPENDJ-1973: Unindexed searches should limit dn2id cursoring based on hard-coded limit and user's look through limit.

---
 opendj-sdk/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java               |   47 ++++++---
 opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ClientConnectionStub.java         |  198 +++++++++++++++++++++++++++++++++++++++
 opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java |   38 +++++++
 3 files changed, 266 insertions(+), 17 deletions(-)

diff --git a/opendj-sdk/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java b/opendj-sdk/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
index 5ca5b2b..b725930 100644
--- a/opendj-sdk/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
+++ b/opendj-sdk/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
@@ -865,7 +865,11 @@
 
             if (!isBelowFilterThreshold(entryIDSet))
             {
-              final EntryIDSet scopeSet = getIDSetFromScope(txn, aBaseDN, searchScope);
+              final int lookThroughLimit = searchOperation.getClientConnection().getLookthroughLimit();
+              final int idSetLimit =
+                  lookThroughLimit == 0 ? SCOPE_IDSET_LIMIT : Math.min(SCOPE_IDSET_LIMIT, lookThroughLimit);
+
+              final EntryIDSet scopeSet = getIDSetFromScope(txn, aBaseDN, searchScope, idSetLimit);
               entryIDSet.retainAll(scopeSet);
               if (debugBuffer != null)
               {
@@ -969,8 +973,8 @@
           return null;
         }
 
-        private EntryIDSet getIDSetFromScope(final ReadableTransaction txn, DN aBaseDN, SearchScope searchScope)
-            throws DirectoryException
+        private EntryIDSet getIDSetFromScope(final ReadableTransaction txn, DN aBaseDN, SearchScope searchScope,
+            int idSetLimit) throws DirectoryException
         {
           final EntryIDSet scopeSet;
           try
@@ -986,14 +990,14 @@
             case SINGLE_LEVEL:
               try (final SequentialCursor<?, EntryID> scopeCursor = dn2id.openChildrenCursor(txn, aBaseDN))
               {
-                scopeSet = newIDSetFromCursor(scopeCursor, false);
+                scopeSet = newIDSetFromCursor(scopeCursor, false, idSetLimit);
               }
               break;
             case SUBORDINATES:
             case WHOLE_SUBTREE:
               try (final SequentialCursor<?, EntryID> scopeCursor = dn2id.openSubordinatesCursor(txn, aBaseDN))
               {
-                scopeSet = newIDSetFromCursor(scopeCursor, searchScope.equals(SearchScope.WHOLE_SUBTREE));
+                scopeSet = newIDSetFromCursor(scopeCursor, searchScope.equals(SearchScope.WHOLE_SUBTREE), idSetLimit);
               }
               break;
             default:
@@ -1016,23 +1020,32 @@
     }
   }
 
-  private static EntryIDSet newIDSetFromCursor(SequentialCursor<?, EntryID> cursor, boolean includeCurrent)
+  private static EntryIDSet newIDSetFromCursor(SequentialCursor<?, EntryID> cursor, boolean includeCurrent,
+      int idSetLimit)
   {
-    long ids[] = new long[SCOPE_IDSET_LIMIT];
+    long entryIDs[] = new long[idSetLimit];
     int offset = 0;
-    if (includeCurrent) {
-      ids[offset++] = cursor.getValue().longValue();
-    }
-    for(; offset < ids.length && cursor.next() ; offset++) {
-      ids[offset] = cursor.getValue().longValue();
+    if (includeCurrent)
+    {
+      entryIDs[offset++] = cursor.getValue().longValue();
     }
 
-    ids = Arrays.copyOf(ids, offset);
-    Arrays.sort(ids);
+    while(offset < idSetLimit && cursor.next())
+    {
+      entryIDs[offset++] = cursor.getValue().longValue();
+    }
 
-    return offset == SCOPE_IDSET_LIMIT
-        ? EntryIDSet.newUndefinedSet()
-        : EntryIDSet.newDefinedSet(ids);
+    if (offset == idSetLimit && cursor.next())
+    {
+      return EntryIDSet.newUndefinedSet();
+    }
+    else if (offset != idSetLimit)
+    {
+      entryIDs = Arrays.copyOf(entryIDs, offset);
+    }
+    Arrays.sort(entryIDs);
+
+    return EntryIDSet.newDefinedSet(entryIDs);
   }
 
   private <E1 extends Exception, E2 extends Exception>
diff --git a/opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ClientConnectionStub.java b/opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ClientConnectionStub.java
new file mode 100644
index 0000000..5ef811e
--- /dev/null
+++ b/opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ClientConnectionStub.java
@@ -0,0 +1,198 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License, Version 1.0 only
+ * (the "License").  You may not use this file except in compliance
+ * with the License.
+ *
+ * You can obtain a copy of the license at legal-notices/CDDLv1_0.txt
+ * or http://forgerock.org/license/CDDLv1.0.html.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at legal-notices/CDDLv1_0.txt.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information:
+ *      Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ *
+ *
+ *      Copyright 2015 ForgeRock AS
+ */
+package org.opends.server.backends.pluggable;
+
+import java.net.InetAddress;
+import java.util.Collection;
+
+import org.forgerock.i18n.LocalizableMessage;
+import org.opends.server.api.ClientConnection;
+import org.opends.server.api.ConnectionHandler;
+import org.opends.server.core.SearchOperation;
+import org.opends.server.types.CancelRequest;
+import org.opends.server.types.CancelResult;
+import org.opends.server.types.DirectoryException;
+import org.opends.server.types.DisconnectReason;
+import org.opends.server.types.IntermediateResponse;
+import org.opends.server.types.Operation;
+import org.opends.server.types.SearchResultEntry;
+import org.opends.server.types.SearchResultReference;
+
+class ClientConnectionStub extends ClientConnection
+{
+
+  @Override
+  public long getConnectionID()
+  {
+    return 0;
+  }
+
+  @Override
+  public ConnectionHandler<?> getConnectionHandler()
+  {
+    return null;
+  }
+
+  @Override
+  public String getProtocol()
+  {
+    return null;
+  }
+
+  @Override
+  public String getClientAddress()
+  {
+    return null;
+  }
+
+  @Override
+  public int getClientPort()
+  {
+    return 0;
+  }
+
+  @Override
+  public String getServerAddress()
+  {
+    return null;
+  }
+
+  @Override
+  public int getServerPort()
+  {
+    return 0;
+  }
+
+  @Override
+  public InetAddress getRemoteAddress()
+  {
+    return null;
+  }
+
+  @Override
+  public InetAddress getLocalAddress()
+  {
+    return null;
+  }
+
+  @Override
+  public boolean isConnectionValid()
+  {
+    return false;
+  }
+
+  @Override
+  public boolean isSecure()
+  {
+    return false;
+  }
+
+  @Override
+  public long getNumberOfOperations()
+  {
+    return 0;
+  }
+
+  @Override
+  public void sendResponse(Operation operation)
+  {
+  }
+
+  @Override
+  public void sendSearchEntry(SearchOperation searchOperation, SearchResultEntry searchEntry) throws DirectoryException
+  {
+  }
+
+  @Override
+  public boolean sendSearchReference(SearchOperation searchOperation, SearchResultReference searchReference)
+      throws DirectoryException
+  {
+    return false;
+  }
+
+  @Override
+  protected boolean sendIntermediateResponseMessage(IntermediateResponse intermediateResponse)
+  {
+    return false;
+  }
+
+  @Override
+  public void disconnect(DisconnectReason disconnectReason, boolean sendNotification, LocalizableMessage message)
+  {
+  }
+
+  @Override
+  public Collection<Operation> getOperationsInProgress()
+  {
+    return null;
+  }
+
+  @Override
+  public Operation getOperationInProgress(int messageID)
+  {
+    return null;
+  }
+
+  @Override
+  public boolean removeOperationInProgress(int messageID)
+  {
+    return false;
+  }
+
+  @Override
+  public CancelResult cancelOperation(int messageID, CancelRequest cancelRequest)
+  {
+    return null;
+  }
+
+  @Override
+  public void cancelAllOperations(CancelRequest cancelRequest)
+  {
+  }
+
+  @Override
+  public void cancelAllOperationsExcept(CancelRequest cancelRequest, int messageID)
+  {
+  }
+
+  @Override
+  public String getMonitorSummary()
+  {
+    return null;
+  }
+
+  @Override
+  public void toString(StringBuilder buffer)
+  {
+  }
+
+  @Override
+  public int getSSF()
+  {
+    return 0;
+  }
+
+}
diff --git a/opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java b/opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
index 61c3dc6..e8c92aa 100644
--- a/opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
+++ b/opendj-sdk/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
@@ -25,6 +25,7 @@
  */
 package org.opends.server.backends.pluggable;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.forgerock.opendj.ldap.ModificationType.ADD;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -37,14 +38,18 @@
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Random;
 
 import org.assertj.core.api.Assertions;
+import org.forgerock.i18n.LocalizableMessage;
 import org.forgerock.opendj.ldap.ByteString;
 import org.forgerock.opendj.ldap.ConditionResult;
+import org.forgerock.opendj.ldap.ResultCode;
 import org.forgerock.opendj.ldap.SearchScope;
 import org.opends.server.DirectoryServerTestCase;
 import org.opends.server.TestCaseUtils;
@@ -54,7 +59,10 @@
 import org.opends.server.admin.std.server.BackendVLVIndexCfg;
 import org.opends.server.admin.std.server.PluggableBackendCfg;
 import org.opends.server.api.Backend.BackendOperation;
+import org.opends.server.api.ClientConnection;
+import org.opends.server.api.ConnectionHandler;
 import org.opends.server.core.DirectoryServer;
+import org.opends.server.core.SearchOperation;
 import org.opends.server.protocols.internal.InternalClientConnection;
 import org.opends.server.protocols.internal.InternalSearchOperation;
 import org.opends.server.protocols.internal.SearchRequest;
@@ -62,14 +70,20 @@
 import org.opends.server.types.AttributeType;
 import org.opends.server.types.BackupConfig;
 import org.opends.server.types.BackupDirectory;
+import org.opends.server.types.CancelRequest;
+import org.opends.server.types.CancelResult;
 import org.opends.server.types.DN;
 import org.opends.server.types.DirectoryException;
+import org.opends.server.types.DisconnectReason;
 import org.opends.server.types.Entry;
+import org.opends.server.types.IntermediateResponse;
 import org.opends.server.types.LDIFExportConfig;
 import org.opends.server.types.LDIFImportConfig;
 import org.opends.server.types.Modification;
+import org.opends.server.types.Operation;
 import org.opends.server.types.RestoreConfig;
 import org.opends.server.types.SearchResultEntry;
+import org.opends.server.types.SearchResultReference;
 import org.opends.server.workflowelement.localbackend.LocalBackendSearchOperation;
 import org.testng.Reporter;
 import org.testng.annotations.AfterClass;
@@ -716,6 +730,30 @@
     subTreeSearch(true);
   }
 
+  @Test(dependsOnMethods = "testAdd")
+  public void testSearchIsConsideredUnindexedBasedOnLookThroughLimit() throws DirectoryException {
+    final int nbEntries = topEntries.size() + entries.size() + workEntries.size();
+
+    final SearchRequest request = newSearchRequest(testBaseDN, SearchScope.WHOLE_SUBTREE, "objectclass=*");
+    final ClientConnection connection = new ClientConnectionStub();
+    connection.setLookthroughLimit(0);
+    InternalSearchOperation searchOperation = new InternalSearchOperation(connection, 1, 1, request, null);
+    searchOperation.run();
+    assertThat(searchOperation.getEntriesSent()).isEqualTo(nbEntries);
+
+    connection.setLookthroughLimit(nbEntries);
+    searchOperation = new InternalSearchOperation(connection, 1, 1, request, null);
+    searchOperation.run();
+    assertThat(searchOperation.getEntriesSent()).isEqualTo(nbEntries);
+
+    connection.setLookthroughLimit(nbEntries - 1);
+    searchOperation = new InternalSearchOperation(connection, 1, 1, request, null);
+    searchOperation.run();
+    assertThat(searchOperation.getResultCode()).isEqualTo(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
+    assertThat(searchOperation.getErrorMessage().toString()).contains("not have sufficient privileges", "unindexed search");
+    assertThat(searchOperation.getEntriesSent()).isEqualTo(0);
+  }
+
   private void subTreeSearch(boolean useInternalConnection) throws Exception
   {
     SearchRequest request = newSearchRequest(testBaseDN, SearchScope.WHOLE_SUBTREE, "objectclass=*");

--
Gitblit v1.10.0