From ba29b0f8216baba3c804a2a8336384dab389c286 Mon Sep 17 00:00:00 2001
From: Yannick Lecaillez <yannick.lecaillez@forgerock.com>
Date: Fri, 12 Jun 2015 14:35:23 +0000
Subject: [PATCH] OPENDJ-2135: verify-index: IndexOutOfBoundsException

---
 opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DN2IDTest.java                    |   56 +++++++++++++++---
 opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java                    |   20 +++---
 opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java                        |   13 +++-
 opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java |   38 ++++++++++++
 4 files changed, 105 insertions(+), 22 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java
index 777513a..433acb0 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java
@@ -190,14 +190,19 @@
       return false;
     }
     // Immediate children should only have one RDN separator past the parent length
-    for (int i = child.length(); i >= parent.length(); i--)
+    int nbSeparator = 0;
+    for (int i = parent.length() ; i < child.length(); i++)
     {
-      if (child.byteAt(i) == DN.NORMALIZED_RDN_SEPARATOR && i != parent.length())
+      if (child.byteAt(i) == DN.NORMALIZED_RDN_SEPARATOR)
       {
-        return false;
+        nbSeparator++;
+        if (nbSeparator > 1)
+        {
+          return false;
+        }
       }
     }
-    return true;
+    return (nbSeparator == 1);
   }
 
   /**
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java
index 3af55f2..390c0de 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java
@@ -35,13 +35,13 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.IdentityHashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Queue;
 import java.util.Set;
 import java.util.Timer;
 import java.util.TimerTask;
@@ -443,7 +443,7 @@
    */
   private void iterateDN2ID(ReadableTransaction txn) throws StorageRuntimeException
   {
-    final Queue<ChildrenCount> childrenCounters = new LinkedList<>();
+    final Deque<ChildrenCount> childrenCounters = new LinkedList<>();
     ChildrenCount currentNode = null;
 
     try(final Cursor<ByteString, ByteString> cursor = txn.openCursor(dn2id.getName()))
@@ -491,27 +491,29 @@
         }
       }
 
-      while ((currentNode = childrenCounters.poll()) != null)
+      while ((currentNode = childrenCounters.pollLast()) != null)
       {
         verifyID2ChildrenCount(txn, currentNode);
       }
     }
   }
 
-  private ChildrenCount verifyID2ChildrenCount(ReadableTransaction txn, final Queue<ChildrenCount> childrenCounters,
+  private ChildrenCount verifyID2ChildrenCount(ReadableTransaction txn, final Deque<ChildrenCount> childrenCounters,
       final ByteString key, final EntryID entryID)
   {
-    while (childrenCounters.peek() != null && !DN2ID.isChild(childrenCounters.peek().baseDN, key))
+    ChildrenCount currentParent = childrenCounters.peekLast();
+    while (currentParent != null && !DN2ID.isChild(currentParent.baseDN, key))
     {
       // This subtree is fully processed, pop the counter of the parent DN from the stack and verify it's value
-      verifyID2ChildrenCount(txn, childrenCounters.remove());
+      verifyID2ChildrenCount(txn, childrenCounters.removeLast());
+      currentParent = childrenCounters.getLast();
     }
-    if (childrenCounters.peek() != null)
+    if (currentParent != null)
     {
-      childrenCounters.peek().numberOfChildren++;
+      currentParent.numberOfChildren++;
     }
     final ChildrenCount node = new ChildrenCount(key, entryID);
-    childrenCounters.add(node);
+    childrenCounters.addLast(node);
     return node;
   }
 
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DN2IDTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DN2IDTest.java
index b1ab0c6..ae5eb05 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DN2IDTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DN2IDTest.java
@@ -33,6 +33,7 @@
 import java.util.concurrent.TimeUnit;
 
 import org.forgerock.opendj.config.server.ConfigException;
+import org.forgerock.opendj.ldap.ByteString;
 import org.forgerock.util.promise.NeverThrowsException;
 import org.forgerock.util.promise.PromiseImpl;
 import org.opends.server.DirectoryServerTestCase;
@@ -41,6 +42,7 @@
 import org.opends.server.admin.std.server.BackendIndexCfg;
 import org.opends.server.admin.std.server.PDBBackendCfg;
 import org.opends.server.backends.pdb.PDBStorage;
+import org.opends.server.backends.pluggable.spi.Cursor;
 import org.opends.server.backends.pluggable.spi.ReadOperation;
 import org.opends.server.backends.pluggable.spi.ReadableTransaction;
 import org.opends.server.backends.pluggable.spi.SequentialCursor;
@@ -54,7 +56,6 @@
 import org.opends.server.extensions.DiskSpaceMonitor;
 import org.opends.server.types.DN;
 import org.opends.server.types.DirectoryException;
-import org.testng.annotations.AfterClass;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
@@ -68,16 +69,12 @@
   private DN2ID dn2ID;
   private PDBStorage storage;
 
+  // FIXME: This is required since PDBStorage is now using
+  // DirectoryServer static method.
   @BeforeClass
-  public void startFakeServer() throws Exception
+  public void startServer() throws Exception
   {
-    TestCaseUtils.startFakeServer();
-  }
-
-  @AfterClass
-  public void stopFakeServer() throws Exception
-  {
-    TestCaseUtils.shutdownFakeServer();
+    TestCaseUtils.startServer();
   }
 
   @BeforeMethod
@@ -138,6 +135,47 @@
   }
 
   @Test
+  public void testIsChild() throws DirectoryException, Exception {
+    put(dn("dc=example,dc=com"), 1);
+    put(dn("ou=People,dc=example,dc=com"), 2);
+    put(dn("uid=user.0,ou=People,dc=example,dc=com"), 3);
+    put(dn("uid=user.1,ou=People,dc=example,dc=com"), 4);
+    put(dn("uid=user.10,ou=People,dc=example,dc=com"), 5);
+
+    storage.read(new ReadOperation<Void>()
+    {
+      @Override
+      public Void run(ReadableTransaction txn) throws Exception
+      {
+        try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(dn2ID.getName()))
+        {
+          cursor.next();
+          final ByteString rootDN = cursor.getKey();
+          cursor.next();
+          final ByteString parentDN = cursor.getKey();
+          cursor.next();
+          assertThat(DN2ID.isChild(rootDN, parentDN)).isTrue();
+
+          final ByteString childDN = cursor.getKey();
+          assertThat(DN2ID.isChild(parentDN, childDN)).isTrue();
+
+          cursor.next();
+          final ByteString otherChildDN = cursor.getKey();
+          assertThat(DN2ID.isChild(parentDN, otherChildDN)).isTrue();
+          assertThat(DN2ID.isChild(childDN, otherChildDN)).isFalse();
+
+          final ByteString lastChildDN = cursor.getKey();
+          assertThat(DN2ID.isChild(parentDN, lastChildDN)).isTrue();
+          assertThat(DN2ID.isChild(otherChildDN, lastChildDN)).isFalse();
+          assertThat(DN2ID.isChild(childDN, lastChildDN)).isFalse();
+        }
+        return null;
+      }
+    });
+
+  }
+
+  @Test
   public void testGetNonExistingDNReturnNull() throws Exception
   {
     assertThat(get("dc=non,dc=existing")).isNull();
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
index 1be05b4..03d8581 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
@@ -57,7 +57,10 @@
 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.backends.VerifyConfig;
 import org.opends.server.backends.pluggable.spi.ReadOnlyStorageException;
+import org.opends.server.backends.pluggable.spi.ReadOperation;
+import org.opends.server.backends.pluggable.spi.ReadableTransaction;
 import org.opends.server.backends.pluggable.spi.Storage;
 import org.opends.server.backends.pluggable.spi.Storage.AccessMode;
 import org.opends.server.backends.pluggable.spi.TreeName;
@@ -909,6 +912,41 @@
   }
 
   @Test(dependsOnMethods = "testImportLDIF")
+  public void testVerifyID2ChildrenCount() throws Exception
+  {
+    final Storage storage = backend.getRootContainer().getStorage();
+    final DN2ID dn2ID = backend.getRootContainer().getEntryContainer(testBaseDN).getDN2ID();
+    final ID2Count id2ChildrenCount = backend.getRootContainer().getEntryContainer(testBaseDN).getID2ChildrenCount();
+
+    final VerifyConfig config = new VerifyConfig();
+    config.setBaseDN(DN.valueOf("dc=test,dc=com"));
+    config.addCleanIndex("dn2id");
+
+    assertThat(backend.verifyBackend(config)).isEqualTo(0);
+
+    // Insert an error
+    final EntryID peopleID = storage.read(new ReadOperation<EntryID>()
+    {
+      @Override
+      public EntryID run(ReadableTransaction txn) throws Exception
+      {
+        return dn2ID.get(txn, testBaseDN.child(DN.valueOf("ou=People")));
+      }
+    });
+    storage.write(new WriteOperation()
+    {
+      @Override
+      public void run(WriteableTransaction txn) throws Exception
+      {
+        id2ChildrenCount.deleteCount(txn, peopleID);
+        id2ChildrenCount.addDelta(txn, peopleID, 1);
+      }
+    });
+
+    assertThat(backend.verifyBackend(config)).isEqualTo(1);
+  }
+
+  @Test(dependsOnMethods = "testImportLDIF")
   public void testBackup() throws Exception
   {
     assertEquals(backend.supports(BackendOperation.BACKUP), true, "Skip Backup");

--
Gitblit v1.10.0