From 0c29df8cefccbf0722819c81fbb64ddc0be99b53 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Fri, 11 Sep 2026 13:07:46 +0000
Subject: [PATCH] [#928] Compare the permissive-modify DN with the entry DN the operation reports (#973)
---
opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java | 123 +++++++++++++++++++++++++++++++++++++----
opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java | 11 +++
2 files changed, 121 insertions(+), 13 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 8daed89..2fa2094 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
@@ -2662,7 +2662,16 @@
if (op instanceof ModifyOperation)
{
ModifyOperation modifyOperation = (ModifyOperation) op;
- if (modifyOperation.getEntryDN().equals(SET_PERMISSIVE_MODIFY_FOR_DN))
+ /*
+ * The DN this looks for is compared with what the operation reports rather
+ * than the other way round: getEntryDN() returns null - and reports
+ * INVALID_DN_SYNTAX - for an operation whose raw DN does not parse, and the
+ * replay must report that once and step over the change rather than throw
+ * before the CSN of the operation has been read, which would keep the change
+ * out of the ServerState and have it asked for again for the whole give-up
+ * window (issue #928).
+ */
+ if (SET_PERMISSIVE_MODIFY_FOR_DN.equals(modifyOperation.getEntryDN()))
{
op.addRequestControl(new LDAPControl(OID_PERMISSIVE_MODIFY_CONTROL));
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java
index 113d380..b2d03a7 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java
@@ -32,6 +32,7 @@
import java.net.SocketTimeoutException;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
@@ -78,6 +79,7 @@
import org.opends.server.replication.service.ReplicationBroker;
import org.opends.server.types.Attribute;
import org.opends.server.types.Attributes;
+import org.opends.server.types.Control;
import org.opends.server.types.Entry;
import org.opends.server.types.LDAPException;
import org.opends.server.types.Modification;
@@ -2569,11 +2571,11 @@
* its CSN: a message no operation could be built from will not build one on the next
* delivery either, so it is given up on where it is reported, while an operation which
* was built may well have reached the backend - so its change is kept out of the
- * ServerState and asked for again, wherever in the replay the failure happened. The
- * entry DN of a ModifyMsg which does not parse is that case: it leaves
- * {@code getEntryDN()} null and the replay throws before the CSN of the operation is
- * read, so a give-up keyed off that CSN would record a change which never reached the
- * backend as replayed, which is this issue by another route.
+ * ServerState and asked for again, wherever in the replay the failure happened. An
+ * operation which can not be given the ManageDsaIT control the replay adds to every one
+ * of them is that case: it throws before the CSN of the operation is read, so a give-up
+ * keyed off that CSN would record a change which never reached the backend as replayed,
+ * which is this issue by another route.
*/
@Test
public void aChangeWhoseOperationWasBuiltIsNotGivenUpOnWhereItFailed() throws Exception
@@ -2604,7 +2606,7 @@
final String description = "the replay must fail once the operation is built";
final List<Modification> mods = generatemods("description", description);
- domain.processUpdate(new ModifyMsgWithAnUnparseableOperationDN(csn, dn, mods, uuid));
+ domain.processUpdate(new ModifyMsgWhoseOperationRefusesAControl(csn, dn, mods, uuid));
/*
* Long enough to outlast the session restart the failure asks for: a change which is
@@ -2654,6 +2656,75 @@
}
/**
+ * Test case for [Issue 928]: a modify whose entry DN does not parse is reported once
+ * and stepped over rather than thrown on.
+ * <p>
+ * {@code ModifyOperationBasis.getEntryDN()} reports INVALID_DN_SYNTAX and returns null
+ * when the raw DN of the operation does not parse, and the permissive-modify check the
+ * replay makes before running the operation reads it. Comparing the DN this check looks
+ * for with what that returns, rather than the other way round, is what keeps this from
+ * being a NullPointerException thrown before the operation ran: the operation runs,
+ * reports the syntax of its DN, and the change is stepped over like any other change
+ * this replica can not apply. Thrown on instead, the change is kept out of the
+ * ServerState and asked for again for the whole give-up window, with a stack trace per
+ * delivery, where the syntax of its DN is a verdict on every delivery of it.
+ */
+ @Test
+ public void aModifyWhoseEntryDNDoesNotParseIsReportedRatherThanThrownOn() throws Exception
+ {
+ testSetUp("aModifyWhoseEntryDNDoesNotParseIsReportedRatherThanThrownOn");
+ logger.error(LocalizableMessage.raw(
+ "Starting replication test : aModifyWhoseEntryDNDoesNotParseIsReportedRatherThanThrownOn"));
+
+ Entry tmp = TestCaseUtils.addEntry(
+ "dn: uid=user.928.1," + baseDN,
+ "objectClass: top",
+ "objectClass: person",
+ "objectClass: organizationalPerson",
+ "objectClass: inetOrgPerson",
+ "uid: user.928.1",
+ "cn: Aaccf Amar",
+ "sn: Amar");
+ final DN dn = tmp.getName();
+ final String uuid = getEntry(dn, 1, true).parseAttribute("entryuuid").asString();
+
+ final LDAPReplicationDomain domain = MultimasterReplication.findDomain(baseDN, null);
+ final long initialFailures = getMonitorAttrValue(baseDN, "replayed-updates-failed");
+ domain.resetUnreplayedChangeAlertThrottle();
+ final int initialAlerts = DummyAlertHandler.getAlertCount(ALERT_TYPE_REPLICATION_UNREPLAYED_CHANGE);
+
+ final CSNGenerator gen = new CSNGenerator(19, TimeThread.getTime());
+ final CSN csn = gen.newCSN();
+ final List<Modification> mods =
+ generatemods("description", "the entry DN of this change does not parse");
+
+ domain.processUpdate(new ModifyMsgWithAnUnparseableOperationDN(csn, dn, mods, uuid));
+
+ TestTimer timer = new TestTimer.Builder()
+ .maxSleep(60, SECONDS)
+ .sleepTimes(200, MILLISECONDS)
+ .toTimer();
+ timer.repeatUntilSuccess(new CallableVoid()
+ {
+ @Override
+ public void call() throws Exception
+ {
+ assertTrue(domain.getServerState().cover(csn),
+ "a change whose entry DN does not parse must be stepped over,"
+ + " not thrown on and asked for again");
+ }
+ });
+ assertMonitorAttrValueEventually(baseDN, "replayed-updates-failed", initialFailures + 1,
+ "a change this replica can not apply must be counted as failed");
+ assertMonitorAttrValueStays(baseDN, "replayed-updates-failed", initialFailures + 1,
+ MONITOR_ATTR_SAMPLES_ACROSS_A_REDELIVERY,
+ "a change which was stepped over must be counted once rather than delivered again");
+ Assertions.assertThat(DummyAlertHandler.getAlertCount(ALERT_TYPE_REPLICATION_UNREPLAYED_CHANGE))
+ .as("the administrator must be told that this replica now diverges")
+ .isGreaterThan(initialAlerts);
+ }
+
+ /**
* Test case for [Issue 908]: a domain being disabled - for an LDIF import, a restore, or
* a backend being taken offline - must not save its ServerState while a replay thread is
* half way through applying one of its changes.
@@ -2986,15 +3057,43 @@
}
/**
- * A ModifyMsg whose operation can not tell which change it carries.
+ * A ModifyMsg whose operation can not be prepared for its replay.
* <p>
* The operation is built - so the replay is past the point where a message is given up
- * on - and its entry DN does not parse, which is what has
- * {@code ModifyOperationBasis.getEntryDN()} return null and the replay throw before
+ * on - and the list of request controls it carries can not be added to, so the
+ * ManageDsaIT control the replay puts on every operation throws before
* {@code OperationContext.getCSN(op)} is reached. Such a message can not travel the
- * protocol: the DN of a ModifyMsg is decoded on the way in and the operation is built
- * from its {@code toString()}, so this one is handed to the domain rather than
- * published.
+ * protocol: {@code ModifyMsg.createOperation()} builds an operation whose controls can
+ * be added to, so this one is handed to the domain rather than published.
+ */
+ private static final class ModifyMsgWhoseOperationRefusesAControl extends ModifyMsg
+ {
+ private ModifyMsgWhoseOperationRefusesAControl(
+ CSN csn, DN dn, List<Modification> mods, String entryUUID)
+ {
+ super(csn, dn, mods, entryUUID);
+ }
+
+ @Override
+ public ModifyOperation createOperation(InternalClientConnection connection, DN newDN)
+ {
+ final ModifyOperation op = new ModifyOperationBasis(connection, nextOperationID(),
+ nextMessageID(), Collections.<Control>emptyList(),
+ ByteString.valueOfUtf8(getDN().toString()), new ArrayList<RawModification>());
+ op.setAttachment(OperationContext.SYNCHROCONTEXT,
+ new ModifyContext(getCSN(), getEntryUUID()));
+ return op;
+ }
+ }
+
+ /**
+ * A ModifyMsg whose operation carries an entry DN which does not parse.
+ * <p>
+ * {@code ModifyOperationBasis.getEntryDN()} reports INVALID_DN_SYNTAX and returns null
+ * for such an operation, which is what the permissive-modify check of the replay reads.
+ * Such a message can not travel the protocol: the DN of a ModifyMsg is parsed on the
+ * way in and the operation is built from its {@code toString()}, so this one is handed
+ * to the domain rather than published.
*/
private static final class ModifyMsgWithAnUnparseableOperationDN extends ModifyMsg
{
--
Gitblit v1.10.0