mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Jean-Noel Rouvignac
24.14.2015 7b44fa6b33c5441b25e900fb906e280641ce3737
(CR-6756) Fixed IllegalArgumentException when import triggers index entry limits

IllegalArgumentException was triggered in ImportIDSet.addEntryID() when called with a negative entryID.
The negative entryID was read in IndexInputBuffer.mergeIDSet().
The problem comes from Importer.ScratchFileWriterTask.writeByteStreams() which writes a special value instead of an entryID which means "treat this ImportIDSet has undefined".
First problem, this special value is written as a byte, but read as a long. I fixed the code to write it as a long.
Second problem, the (illogical) code handling this specific case in ImportIDSet.addEntryID() was removed as part of r11948. I fixed this by properly handling this case at a higher level in IndexInputBuffer.mergeIDSet().
I finally applied similar changes to the JE code to explicit what it is doing.



pluggable.ImportIDSet.java:
Changed setUndefined() from private to package private.
Extracted method setUndefinedWithSize().

pluggable.IndexInputBuffer.java:
Added UNDEFINED_SIZE constant.
In mergeIDSet(), handled the undefined size case.
Added toString().

pluggable.Importer.java:
In writeByteStreams(), wrote UNDEFINED_SIZE as a long to the BAOS.
Changed toString() to output the index name.

pluggable.IndexOutputBuffer.java:
Added toString().



Reflected similar changes to JEB. In addition:

jeb.ImportIDSet.java:
In addEntryID(), reject negative entryIDs.

jeb.Importer.java:
In writeByteStreams(), write a packed long (although the encoded representation is the same as packed int, code was conceptually incorrect) + added writePackedLong() method.

jeb.IndexInputBuffer.java:
Extracted readKey().
In mergeIDSet(), handled the undefined size. There is a difference in behaviour: previous the undefined size could be maintained, but now it is no longer maintained: this is correct since there was no count to maintain for an undefined size.
7 files modified
104 ■■■■ changed files
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/ImportIDSet.java 8 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/Importer.java 26 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/IndexInputBuffer.java 20 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportIDSet.java 10 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/Importer.java 13 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/IndexInputBuffer.java 17 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/IndexOutputBuffer.java 10 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/ImportIDSet.java
@@ -28,6 +28,8 @@
import java.nio.ByteBuffer;
import org.forgerock.util.Reject;
/**
 * This class manages the set of ID that are to be eventually added to an index
 * database. It is responsible for determining if the number of IDs is above
@@ -89,7 +91,7 @@
  }
  /** Set an import ID set to undefined. */
  private void setUndefined() {
  void setUndefined() {
    array = null;
    isDefined = false;
  }
@@ -109,13 +111,15 @@
   * @param entryID The long value to add to an import ID set.
   */
  void addEntryID(long entryID) {
    Reject.ifTrue(entryID < 0, "entryID must always be positive");
    if(!isDefined()) {
      if(doCount)  {
        undefinedSize++;
      }
      return;
    }
    if (entryID < 0 || (isDefined() && count + 1 > limit))
    if (isDefined() && count + 1 > limit)
    {
      setUndefined();
      if(doCount)  {
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/Importer.java
@@ -30,6 +30,7 @@
import static org.opends.messages.JebMessages.*;
import static org.opends.server.admin.std.meta.LocalDBIndexCfgDefn.IndexType.*;
import static org.opends.server.backends.jeb.IndexOutputBuffer.*;
import static org.opends.server.backends.pluggable.SuffixContainer.*;
import static org.opends.server.util.DynamicConstants.*;
import static org.opends.server.util.ServerConstants.*;
import static org.opends.server.util.StaticUtils.*;
@@ -2417,7 +2418,8 @@
   */
  private final class ScratchFileWriterTask implements Callable<Void>
  {
    private final int DRAIN_TO = 3;
    private static final int DRAIN_TO = 3;
    private final IndexManager indexMgr;
    private final BlockingQueue<IndexOutputBuffer> queue;
    private final ByteArrayOutputStream insertByteStream = new ByteArrayOutputStream(2 * bufferSize);
@@ -2642,9 +2644,10 @@
    {
      if (insertKeyCount > indexMgr.getLimit())
      {
        // special handling when index entry limit has been exceeded
        insertKeyCount = 1;
        insertByteStream.reset();
        writePackedInt(insertByteStream, -1);
        writePackedLong(insertByteStream, IndexInputBuffer.UNDEFINED_SIZE);
      }
      int insertSize = writePackedInt(bufferStream, insertKeyCount);
@@ -2693,6 +2696,14 @@
      return writeSize;
    }
    private int writePackedLong(OutputStream stream, long value) throws IOException
    {
      int writeSize = PackedInteger.getWriteLongLength(value);
      PackedInteger.writeLong(tmpArray, 0, value);
      stream.write(tmpArray, 0, writeSize);
      return writeSize;
    }
    /** {@inheritDoc} */
    @Override
    public String toString()
@@ -3404,11 +3415,11 @@
      for (String index : rebuildList)
      {
        final String lowerName = index.toLowerCase();
        if ("dn2id".equals(lowerName))
        if (DN2ID_INDEX_NAME.equals(lowerName))
        {
          indexCount += 3;
        }
        else if ("dn2uri".equals(lowerName))
        else if (DN2URI_INDEX_NAME.equals(lowerName))
        {
          indexCount++;
        }
@@ -3420,8 +3431,8 @@
          }
          indexCount++;
        }
        else if ("id2subtree".equals(lowerName)
            || "id2children".equals(lowerName))
        else if (ID2SUBTREE_INDEX_NAME.equals(lowerName)
            || ID2CHILDREN_INDEX_NAME.equals(lowerName))
        {
          throw attributeIndexNotConfigured(index);
        }
@@ -4148,8 +4159,7 @@
    public String toString()
    {
      return getClass().getSimpleName()
          + "(attributeType=" + attributeType
          + ", indexType=" + indexType
          + "(index=" + attributeType.getNameOrOID() + "." + indexType
          + ", entryLimit=" + entryLimit
          + ")";
    }
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/IndexInputBuffer.java
@@ -51,6 +51,7 @@
  }
  private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
  static final long UNDEFINED_SIZE = -1;
  private final IndexManager indexMgr;
  private final FileChannel channel;
@@ -206,7 +207,13 @@
    }
    indexID = getInt();
    readKey();
    recordState = RecordState.NEED_INSERT_ID_SET;
  }
  private void readKey() throws IOException
  {
    ensureData(20);
    byte[] ba = cache.array();
    int p = cache.position();
@@ -221,8 +228,6 @@
    keyBuf.clear();
    cache.get(keyBuf.array(), 0, keyLen);
    keyBuf.limit(keyLen);
    recordState = RecordState.NEED_INSERT_ID_SET;
  }
  private int getInt() throws IOException
@@ -261,14 +266,21 @@
        p = cache.position();
      }
      len = PackedInteger.getReadLongLength(ba, p);
      long l = PackedInteger.readLong(ba, p);
      long entryID = PackedInteger.readLong(ba, p);
      p += len;
      cache.position(p);
      // idSet will be null if skipping.
      if (idSet != null)
      {
        idSet.addEntryID(l);
        if (entryID == UNDEFINED_SIZE)
        {
          idSet.setUndefined();
        }
        else
        {
          idSet.addEntryID(entryID);
        }
      }
    }
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportIDSet.java
@@ -84,10 +84,14 @@
    return entryIDSet.isDefined();
  }
  private void setUndefined() {
  void setUndefined() {
    entryIDSet = newUndefinedSetWithKey(key);
  }
  private void setUndefinedWithSize(final long newSize) {
    entryIDSet = maintainCount ? newUndefinedSetWithSize(key, newSize) : newUndefinedSetWithKey(key);
  }
  /**
   * @param entryID The entry ID to add to an import ID set.
   * @throws NullPointerException if entryID is null
@@ -103,7 +107,7 @@
  {
    Reject.ifTrue(entryID < 0, "entryID must always be positive");
    if (isDefined() && size() + 1 > indexEntryLimitSize) {
      entryIDSet = maintainCount ? newUndefinedSetWithSize(key, size() + 1) : newUndefinedSetWithKey(key);
      setUndefinedWithSize(size() + 1);
    } else if (isDefined() || maintainCount) {
      entryIDSet.add(new EntryID(entryID));
    }
@@ -138,7 +142,7 @@
    if (!definedBeforeMerge || !importIdSet.isDefined() || mergedSize > indexEntryLimitSize)
    {
      entryIDSet = maintainCount ? newUndefinedSetWithSize(key, mergedSize) : newUndefinedSetWithKey(key);
      setUndefinedWithSize(mergedSize);
      return definedBeforeMerge;
    }
    else if (isDefined() || maintainCount)
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/Importer.java
@@ -811,8 +811,6 @@
  /**
   * Rebuild the indexes using the specified root container.
   *
   * @param rootContainer
   *          The root container to rebuild indexes in.
   * @throws ConfigException
   *           If a configuration error occurred.
   * @throws InitializationException
@@ -895,8 +893,6 @@
  /**
   * Import a LDIF using the specified root container.
   *
   * @param rootContainer
   *          The root container to use during the import.
   * @return A LDIF result.
   * @throws Exception
   *           If the import failed
@@ -2319,11 +2315,13 @@
   */
  private final class ScratchFileWriterTask implements Callable<Void>
  {
    private final int DRAIN_TO = 3;
    private static final int DRAIN_TO = 3;
    private final IndexManager indexMgr;
    private final BlockingQueue<IndexOutputBuffer> queue;
    /** Stream where to output insert ImportIDSet data. */
    private final ByteArrayOutputStream insertByteStream = new ByteArrayOutputStream(2 * bufferSize);
    private final DataOutputStream insertByteDataStream = new DataOutputStream(insertByteStream);
    /** Stream where to output delete ImportIDSet data. */
    private final ByteArrayOutputStream deleteByteStream = new ByteArrayOutputStream(2 * bufferSize);
    private final DataOutputStream bufferStream;
@@ -2530,7 +2528,7 @@
        // special handling when index entry limit has been exceeded
        insertKeyCount = 1;
        insertByteStream.reset();
        insertByteStream.write(-1);
        insertByteDataStream.writeLong(IndexInputBuffer.UNDEFINED_SIZE);
      }
      int insertSize = INT_SIZE;
@@ -3695,8 +3693,7 @@
    public String toString()
    {
      return getClass().getSimpleName()
          + "(attributeType=" + attributeType
          + ", indexID=" + indexID
          + "(index=" + attributeType.getNameOrOID() + "." + indexID
          + ", entryLimit=" + entryLimit
          + ")";
    }
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/IndexInputBuffer.java
@@ -52,6 +52,7 @@
  }
  private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
  static final long UNDEFINED_SIZE = -1;
  private final IndexManager indexMgr;
  private final FileChannel channel;
@@ -231,9 +232,16 @@
      // idSet will be null if skipping.
      if (idSet != null)
      {
        if (entryID == UNDEFINED_SIZE)
        {
          idSet.setUndefined();
        }
        else
        {
        idSet.addEntryID(entryID);
      }
    }
    }
    switch (recordState)
    {
@@ -284,4 +292,13 @@
    }
    return cmp;
  }
  /** {@inheritDoc} */
  @Override
  public String toString()
  {
    return getClass().getSimpleName() + "("
        + ", indexMgr=" + indexMgr + "bufferID=" + bufferID + ", record=" + record + ", recordState=" + recordState
        + ")";
  }
}
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/IndexOutputBuffer.java
@@ -581,7 +581,6 @@
    this.indexKey = indexKey;
  }
  /**
   * Return the index key of an index buffer.
   * @return The index buffer's index key.
@@ -590,4 +589,13 @@
  {
    return indexKey;
  }
  /** {@inheritDoc} */
  @Override
  public String toString()
  {
    return getClass().getSimpleName() + "("
        + "bufferID=" + bufferID + ", " + currentRecord + ", indexKey=" + indexKey
        + ")";
  }
}