From f559b0907a4ce0348b466d953b8b0a2d9fff7b28 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Fri, 11 Sep 2026 13:13:05 +0000
Subject: [PATCH] [#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 | 25 ++++++------
opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/SynchronizationMsgTest.java | 50 +++++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/AddMsg.java | 25 ++++++------
3 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java
index 2fa2094..25b6418 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java
+++ b/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();
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/AddMsg.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/AddMsg.java
index fdbde7f6..80a372c 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/AddMsg.java
+++ b/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,28 +337,27 @@
/**
* 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);
+ new LDAPAttribute(name, value).write(writer);
- try
- {
- new LDAPAttribute(name, value).write(writer);
-
- encodedAttributes = byteBuilder.toByteArray();
- }
- catch(Exception e)
- {
- // DO SOMETHING
- }
+ encodedAttributes = byteBuilder.toByteArray();
}
/**
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/SynchronizationMsgTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/SynchronizationMsgTest.java
index bdced44..bba5b3a 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/replication/protocol/SynchronizationMsgTest.java
+++ b/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<>();
--
Gitblit v1.10.0