[#927] Report a ds-sync-conflict marker AddMsg could not encode (#975)

Valery Kharseko

yesterday f559b0907a4ce0348b466d953b8b0a2d9fff7b28

[#927] Report a ds-sync-conflict marker AddMsg could not encode (#975)

 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java

@@ -30,6 +30,7 @@
import static org.opends.server.util.StaticUtils.*;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.StringReader;
@@ -71,7 +72,6 @@
import org.forgerock.opendj.ldap.AVA;
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.DN;
import org.forgerock.opendj.ldap.DecodeException;
import org.forgerock.opendj.ldap.ModificationType;
import org.forgerock.opendj.ldap.RDN;
import org.forgerock.opendj.ldap.ResultCode;
@@ -2893,9 +2893,9 @@
           * is nothing to retry and no delivery which would build one any better.
           *
           * The decoding exceptions are caught here rather than in a catch of their
           * own because such a catch would span the whole replay: addConflict(), which
           * solveNamingConflict() calls once the operation has run, declares one, and a
           * change whose operation ran must never be given up on where it failed.
           * own because such a catch would span the whole replay, including what runs
           * after the operation did - solveNamingConflict() calls addConflict() there -
           * and a change whose operation ran must never be given up on where it failed.
           */
          replayErrorMsg = giveUpOnUndecodableChange(msg, e);
        }
@@ -2981,12 +2981,12 @@
   * <p>
   * Only a message which no operation could be built from comes here, and it is the
   * {@code op == null} of its single caller which says so rather than the type of the
   * exception: a decoding exception is declared past the point where the operation ran
   * as well - by addConflict() - so a catch which read the type would give up on a
   * change the backend may well have applied. A failure of the replay of an operation
   * which was built, whenever it happens, keeps its change out of the ServerState and
   * has it delivered again instead: that one is a failure of an attempt, not of every
   * delivery of the change.
   * exception: the replay throws past the point where the operation ran as well -
   * addConflict() does, once solveNamingConflict() has seen the result - so a catch which
   * read the type would give up on a change the backend may well have applied. A failure
   * of the replay of an operation which was built, whenever it happens, keeps its change
   * out of the ServerState and has it delivered again instead: that one is a failure of an
   * attempt, not of every delivery of the change.
   *
   * @param msg the message which could not be decoded
   * @param e the failure to decode it
@@ -3959,10 +3959,9 @@
   *
   * @param msg            The conflicting Add Operation.
   *
   * @throws DecodeException When an encoding error happened manipulating the
   *                       msg.
   * @throws IOException When the conflict marker could not be added to the msg.
   */
  private void addConflict(AddMsg msg) throws DecodeException
  private void addConflict(AddMsg msg) throws IOException
  {
    String normalizedDN = msg.getDN().toString();


 opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/AddMsg.java

@@ -13,12 +13,14 @@
 *
 * Copyright 2006-2010 Sun Microsystems, Inc.
 * Portions Copyright 2011-2016 ForgeRock AS.
 * Portions Copyright 2026 3A Systems, LLC.
 */
package org.opends.server.replication.protocol;

import static org.forgerock.opendj.ldap.schema.CoreSchema.*;
import static org.opends.server.replication.protocol.OperationContext.*;

import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.zip.DataFormatException;
@@ -335,29 +337,28 @@
  /**
   * Add the specified attribute/attribute value in the entry contained
   * in this AddMsg.
   * <p>
   * A failure to encode the attribute is reported rather than swallowed: the caller adds
   * the attribute because the entry must carry it - addConflict() adds the
   * ds-sync-conflict marker which says why an entry was renamed - and an entry stored
   * without it is a conflict the repair tool has nothing to find it by. The attributes
   * this message already carries are left as they were, so a message which could not take
   * the new attribute still encodes the entry it did before.
   *
   * @param name  The name of the attribute to add.
   * @param value The value of the attribute to add.
   * @throws DecodeException When this Msg is not valid.
   * @throws IOException When the attribute could not be encoded.
   */
  public void addAttribute(String name, String value) throws DecodeException
  public void addAttribute(String name, String value) throws IOException
  {
    ByteStringBuilder byteBuilder = new ByteStringBuilder();
    byteBuilder.appendBytes(encodedAttributes);

    ASN1Writer writer = ASN1.getWriter(byteBuilder);

    try
    {
      new LDAPAttribute(name, value).write(writer);

      encodedAttributes = byteBuilder.toByteArray();
    }
    catch(Exception e)
    {
      // DO SOMETHING
    }
  }

  /**
   * Get the attributes of this add msg.

 opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/SynchronizationMsgTest.java

@@ -13,6 +13,7 @@
 *
 * Copyright 2006-2010 Sun Microsystems, Inc.
 * Portions Copyright 2011-2016 ForgeRock AS.
 * Portions Copyright 2026 3A Systems, LLC.
 */
package org.opends.server.replication.protocol;

@@ -22,6 +23,7 @@
import static org.opends.server.TestCaseUtils.*;
import static org.opends.server.protocols.internal.InternalClientConnection.*;
import static org.opends.server.replication.common.AssuredMode.*;
import static org.opends.server.replication.plugin.LDAPReplicationDomain.DS_SYNC_CONFLICT;
import static org.opends.server.replication.protocol.OperationContext.*;
import static org.opends.server.replication.protocol.ProtocolVersion.*;
import static org.opends.server.util.CollectionUtils.*;
@@ -530,6 +532,54 @@
    assertEquals(msg.getCSN(), updateMsg.getCSN());
  }

  /**
   * The conflict marker reaches the entry: addAttribute() appends it to the attributes the
   * message already carries rather than replacing them.
   */
  @Test
  public void addMsgAddAttributeAppendsToTheAttributesAlreadyEncoded() throws Exception
  {
    final String conflictingDN = "o=test";
    final AddMsg msg = newAddMsg();
    final List<Attribute> attributesBefore = msg.getAttributes();

    msg.addAttribute(DS_SYNC_CONFLICT, conflictingDN);

    Assertions.assertThat(msg.getAttributes())
        .containsAll(attributesBefore)
        .contains(Attributes.create(DS_SYNC_CONFLICT, conflictingDN));
  }

  /**
   * A value addAttribute() can not encode is reported rather than dropped: its one caller,
   * LDAPReplicationDomain.addConflict(), would otherwise let the entry be added under its
   * conflict RDN without the ds-sync-conflict marker which says why, leaving the repair
   * tool nothing to find it by and nothing in the logs to say so (issue #927).
   */
  @Test
  public void addMsgAddAttributeReportsAValueItCanNotEncode() throws Exception
  {
    final AddMsg msg = newAddMsg();
    final List<Attribute> attributesBefore = msg.getAttributes();

    Assertions.assertThatThrownBy(() -> msg.addAttribute(DS_SYNC_CONFLICT, null))
        .isInstanceOf(NullPointerException.class);

    // The message is the one it was: a failure leaves no half encoded attribute behind.
    Assertions.assertThat(msg.getAttributes()).isEqualTo(attributesBefore);
  }

  private AddMsg newAddMsg() throws Exception
  {
    final Attribute objectClass = Attributes.create(getObjectClassAttributeType(), "organization");
    final List<Attribute> userAttributes = newArrayList(Attributes.create("o", "com"));
    final List<Attribute> operationalAttributes =
        newArrayList(Attributes.create("creatorsname", "dc=creator"));

    return new AddMsg(new CSN(TimeThread.getTime(), 123, 45), DN.valueOf("o=test"),
        "thisIsaUniqueID", "parentUniqueId", objectClass, userAttributes, operationalAttributes);
  }

  private Map<AttributeType, List<Attribute>> addAttribute(Attribute attr, List<Attribute> userAttributes)
  {
    Map<AttributeType, List<Attribute>> userAttList = new HashMap<>();