[#1153] Parse the whole DN string, and build or split DN strings through DN instead of string operations

[vharseko]

Problem

DN.valueOf() silently drops the rest of the string after ;, after a closing quote, or after a hex string followed by a space. The server decodes the DN of every operation with it, so ldapdelete "uid=user.4,ou=People,dc=example,dc=com;uid=nobody" deletes uid=user.4. Around the parser, DN strings are also built or split by hand in many places: one connection handler named LDAP, internal makes every cn=monitor search fail with 80 (Other), a replicated base DN with : breaks the changelog cookie, and referral URLs for non-ASCII DNs are not UTF-8. See #1153 for the full list.

Decisions

The issue left two choices open:

• ; after an RDN is accepted as the RFC 2253 RDN separator: cn=a;dc=b is cn=a,dc=b. RFC 4514 allows other representations, but not dropping input, so any other content after an RDN is now rejected.
• An empty value is accepted wherever it appears, like the RFC 4514 grammar (string may be empty): cn=,dc=x, cn="",dc=x and cn=x+sn=,dc=y parse, as cn= and sn=+cn=x already did. Whether an attribute allows an empty value is left to its syntax. DNTestCase and DistinguishedNameEqualityMatchingRuleTest move these DNs from the illegal ones to the valid ones. A server before this change cannot read such a DN, which matters during a rolling upgrade: see Upgrade notes.

Change

The SDK parser (opendj-core):

• DN.decode() reads one separator (, or ;) after each RDN and throws ERR_DN_TRAILING_GARBAGE on anything else. AVA.valueOf() rejects trailing content (ERR_AVA_TRAILING_GARBAGE), as RDN.valueOf() already did.
• A hex string value ends at + too.
• An empty value is no longer rejected before , or a closing quote.
• A trailing lone \ is rejected (ERR_ATTR_SYNTAX_DN_TRAILING_ESCAPE). The other leniencies of item 5 are kept.
• The hex string is still kept as raw bytes (item 4, record only).

DN strings built or split by hand:

• NameAndOptionalUIDSyntaxImpl / UniqueMemberEqualityMatchingRuleImpl: a #' preceded by an unescaped \ belongs to the DN.
• MultiDomainServerState: the cookie is split at each unescaped ;, and each domain at its last :.
• cli.Utils.getAdministratorDN(): DN.child().
• Monitor DNs. A monitor instance name is a relative DN: the replication monitors and TestMonitorProvider use it to build a tree, so the name cannot simply become a single escaped RDN. Instead:
  • the providers that put a configured name into it escape that part with DN.escapeAttributeValue(): connection handler, client connections, LDAP and HTTP statistics, backend, storage, JE/PDB database, disk space, entry cache, and the replicated base DN of ReplicationMonitor and ReplicationServerDomain;
  • DirectoryServer.getMonitorProviderDN() makes the whole name the value of one RDN when it does not parse, or does not name an entry below cn=monitor (x\ parses as cn=x\,cn=monitor). A third-party provider can therefore no longer break the rest of cn=monitor.
• LDAPURL: percent-encodes the UTF-8 octets, including characters outside the BMP.
• UserAttr: splits at the first #.
• Control panel: the new entry panels and the duplicate entry panel build the DN with DN.child() (AbstractNewEntryPanel.getNewEntryDN()). Utilities.unescapeUtf8() leaves an escaped backslash alone, so cn=a\\41 is no longer shown as cn=a\A.
• JMXMBean.getJmxName() walks the RDNs. A single-valued RDN whose value is made of letters, digits and spaces keeps the name it always had. Any other value is percent-encoded, except that a space is still written as _ (a literal _ becomes %5F), so user-root, userroot and user_root no longer share one name. The SNMP extension finds the connection handlers by Connection_Handler and their statistics by _Statistics in these names, which is why the space stays _. The AVAs of a multi-valued RDN are joined with +, so cn=a+sn=b is now cn-a+sn-b (was cn-asn-b).
  • Compatibility: the JMX name of every monitor entry whose value holds other characters changes, the LDAP/LDAPS connection handlers among them: cn-LDAP_Connection_Handler_0000_port_1389 becomes cn-LDAP_Connection_Handler_0%2E0%2E0%2E0_port_1389. A JMX client that looks such an MBean up by its full name has to be updated.
• Backend and index configuration DNs in the control panel and the installer (DeleteBaseDNAndBackendTask, DeleteIndexTask, InstallerHelper.deleteBackend()) are built with DN.child() (Utilities.getBackendConfigDN(), Utilities.getIndexConfigDN()), so a backend ID or a VLV index name such as monitor,ou=a b names its own entry.
• MakeLDIF <_DN>: joins the RDNs instead of replacing every comma.
• TaskClient.getTaskDN(): DN.child().
• PatternDN (item 8): decodes \HH inside quotes, reads an empty value before ,, ; or +, ends a hex string at +, and rejects a trailing lone \ with the message DN.valueOf() uses.

Found on the way: PatternRDN.matchesRDN() compared the AVAs of a multi-valued RDN, in the order of the DN string, with the pattern types sorted by name, so the pattern sn=x+cn=y,dc=z did not match the DN sn=x+cn=y,dc=z. It now looks each AVA up by its type.

Upgrade

• Stored DNs written with ; (for example member: cn=a;dc=example,dc=com) were indexed under the key of the part before the first ;. They now read as the whole DN, so an indexed equality search misses them until the index is rebuilt. The same goes for a value stored while its syntax was not enforced that only now parses. Such a value cannot be recognized from the index keys (its old key is that of a legitimate cn=a), only by reading the entries. So rather than rebuilding every DN index on every server, a new 5.2.0 upgrade task (verifyAndRebuildDNEqualityIndexes) works as follows:
  • it finds the equality indexes whose matching rule is distinguishedNameMatch or uniqueMemberMatch in each enabled pluggable backend, reading the instance's schema files so that custom attributes count;
  • it runs verify-index --countErrors on them under each base DN;
  • it rebuilds them only where the verification reports errors.
    The verification computes each entry's keys with the new matching rules, so a missing key is an error. A stale old key is harmless, because the filter is evaluated again on the candidates. The task asks first, with yes as the default answer.
    When the indexes of a base DN can be neither verified nor rebuilt, for example in a JDBC or Cassandra backend whose database is down during the upgrade, the task warns, names the indexes and the base DN, and the upgrade goes on: the server works without these keys, and the administrator verifies that backend once it can be read.
• Docker: run.sh now runs upgrade -n --force, as the deb/rpm packages already do. With -n alone, the long tasks took their default answer, which defers index rebuilds that nobody runs afterwards in a container. The README says the upgrade may now take longer than the start period.
• The upgrade chapter of the installation guide (chap-upgrade.adoc) describes the above, the new JMX names of the monitor MBeans, and what is still manual: a stored DN with other trailing content no longer parses.
• ACIs: a DN with ; between RDNs now names the whole DN. An ACI whose DN has other trailing content no longer decodes; the server logs the ACI and enters lockdown mode when it loads it.
• Rolling upgrade: a server before this change closes its replication connection when it receives a change to an entry whose DN holds an empty value, receives the same change again after reconnecting, and so stops replicating. Such entries must not be added or renamed until every server is upgraded; they cannot be imported into an older server either.

Tests

Every new test was first run against the code from master and failed for the reason it targets:

• opendj-core: DNTestCase, AVATestCase, DistinguishedNameEqualityMatchingRuleTest, UniqueMemberEqualityMatchingRuleTest, the new NameAndOptionalUIDSyntaxTest, and TemplateTagTestCase.
• opendj-cli: UtilsTestCase.
• opendj-server-legacy:
  • MultiDomainServerStateTest, LDAPURLTestCase;
  • new: PatternDNTest, UserAttrTest, JMXMBeanNameTest, TaskClientTaskDNTest, NewEntryDNTestCase, UnescapeUtf8TestCase, ConfigDNTestCase, ReplicationMonitorNameTest, ReplicationServerDomainMonitorNameTest;
  • new DNEqualityIndexesUpgradeTestCase: the index lookup on a config and a schema directory (disabled backend, backend ID with ,, Equality in capitals, a custom attribute SUP distinguishedName, a later schema file redefining it), the base DNs selected per backend, verify-then-rebuild on a PDB backend whose trusted member index has its keys removed (IndexKeyRemover, a test helper in the pluggable package): rebuilt once, then only verified, and a warning, not a failure, for a base DN that cannot be verified;
  • new DeleteBackendTestCase: InstallerHelper.deleteBackend("a,b") deletes the configuration of that backend;
  • new SNMPMonitorConnectionHandlerNameTest: registers MBeans under the names getJmxName() gives a connection handler and its statistics, for IPv4, IPv6 and host names, and checks that SNMPMonitor finds both;
  • new MonitorDNTestCase. It adds through cn=config a connection handler named LDAP,ou=internal (legacy LDAP, LDAPConnectionHandler2 and HTTP), JE and PDB backends with the ID monitor,ou=a b, and an entry cache FIFO,ou=a b. It then checks that cn=monitor is searchable and that each provider owns its own entry, not a branch entry.

The round-2 tests (the trailing \ rows of PatternDNTest, the connection handler rows of JMXMBeanNameTest, the IP rows of SNMPMonitorConnectionHandlerNameTest) fail against the round-1 code. Mutants, each caught: no hex flush at the closing quote, no hex flush before an escaped character inside quotes (one PatternDNTest row each), a literal _ kept in an encoded JMX value (distinctDNs), and the backend config DN built by concatenation (ConfigDNTestCase). Upgrade task mutants, each caught by DNEqualityIndexesUpgradeTestCase: no attribute taken as comparing DNs, the instance's schema files not read, disabled backends kept, an inconsistent index not rebuilt, and an index rebuilt without verification.

Round-3 mutants, each caught: the HTTP statistics name not escaped (the HTTP row of MonitorDNTestCase), another result code or another message for a trailing \ (PatternDNTest, four rows each), a verification failure that fails the upgrade again, the backend IDs joined wrongly when the base DNs are selected, the schema files read in reverse order, an earlier attribute definition kept (DNEqualityIndexesUpgradeTestCase), and the installer building the backend config DN by concatenation (DeleteBackendTestCase).

For the monitor changes, each of the 14 guards was removed in turn (one mutant per escape, the scope check and the fallback), and each mutant turned the test red.

A full mvn -Pprecommit verify -pl opendj-server-legacy -am run passed in every module before opendj-server-legacy, including javadoc. In opendj-server-legacy, 33503 tests ran: 461 were skipped and 1 failed, FileChangelogDBTest.replicaDBLosingTheRaceAgainstShutdownIsNotCreated (its 90 s race timeout, while another build was running on the machine). Run on its own, the class is green.

Fixes #1153

[maximthomas]

praise: The parser now refuses what it used to drop, and the monitor-name escaping is proven guard by guard.

• DN.decode() reads exactly one ,/; after each RDN and throws ERR_DN_TRAILING_GARBAGE otherwise, which closes the ldapdelete "uid=user.4,...;uid=nobody" hole at the one place every operation's DN goes through.
• DirectoryServer.getMonitorProviderDN() falls back to a single-RDN name when an instance name does not parse or leaves cn=monitor, so a third-party provider can no longer break the whole cn=monitor subtree; MonitorDNTestCase turned red for each of the 14 removed guards.
• PatternRDN.matchesRDN() now looks each AVA up by type, fixing sn=x+cn=y,dc=z not matching itself.

---

issue (blocking): Percent-encoding the JMX name of every IP-bearing monitor drops all LDAP/LDAPS connection handlers from the SNMP dsApplIfOpsTable.

opendj-server-legacy/src/main/java/org/opends/server/config/JMXMBean.java:145-160, opendj-server-legacy/src/snmp/src/org/opends/server/snmp/DsMIBImpl.java:335-336, SNMPMonitor.java:124-125, :195-197, :231

LDAPConnectionHandler names a handler LDAP Connection Handler <ip> port <port> (:620-634, getHostAddress()), so the value always holds . or : and appendJmxRdn now registers its monitor MBean as Rdn2=cn-LDAP%20Connection%20Handler%200%2E0%2E0%2E0%20port%201389 instead of cn-LDAP_Connection_Handler_0000_port_1389. The SNMP extension finds connection handlers by substring on that name: DsMIBImpl.isAConnectionHandler requires Rdn2.contains("Connection_Handler"), SNMPMonitor the same plus "_Statistics". With an SNMP connection handler enabled, no LDAP/LDAPS handler gets a MIB row and getConnectionHandlersStatistics returns nothing. This ships: the snmp profile activates on opendmk/jdmkrt.jar, which is committed. SNMPSyncManagerV2AccessTest and SNMPTrapManagerTest never read dsApplIfOpsTable, which is why CI is green. Keeping _ for a space in the encoded branch keeps names distinct (a literal _ is %5F there, and a plain value never holds %) and keeps the SNMP matchers and the concat("_Statistics") at SNMPMonitor:231 working.

        for (byte b : ava.getAttributeValue().toByteArray())
        {
          char c = (char) (b & 0xFF);
          if (c == ' ')
          {
            // The SNMP extension matches "Connection_Handler" and "_Statistics" in these names.
            buffer.append('_');
          }
          else if (c < 0x80 && (isAlpha(c) || isDigit(c)))
          {
            buffer.append(c);
          }
          else
          {
            buffer.append('%').append(byteToHex(b));
          }
        }

Pin: a JMXMBeanNameTest row asserting that getJmxName(DN.valueOf("cn=LDAP Connection Handler 0.0.0.0 port 1389,cn=monitor")) contains Rdn2=cn-LDAP_Connection_Handler_0%2E0%2E0%2E0_port_1389; it is red against the current encoding. Or: update the matchers in DsMIBImpl and SNMPMonitor to the new form.

---

issue (non-blocking): PatternDN still accepts an unquoted value that ends in a lone \, which DN.valueOf() now rejects.

opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/PatternDN.java:1151-1156

At the end of input the unquoted branch flushes the hex chars and breaks without checking escaped, so cn=a\ decodes as the pattern cn=a. Before this PR both parsers read it that way; now the SDK throws ERR_ATTR_SYNTAX_DN_TRAILING_ESCAPE and PatternDN does not, against PatternDNTest's premise that a pattern reads like the DN DN.valueOf() reads. An access-log user-dn-equal-to / request-target-dn-* criterion or a wildcard ACI userdn/target with a trailing \ is accepted instead of failing.

        if (pos >= length)
        {
          if (escaped)
          {
            // A trailing lone backslash, which DN.valueOf() rejects too.
            throw illegalCharacter(dnString, pos - 1, '\\');
          }
          // This is the end of the DN and therefore the end of the value.
          // If there are any hex characters, then we need to deal with them accordingly.
          appendHexChars(dnString, valueString, hexChars);
          break;
        }

Pin: @Test(expectedExceptions = DirectoryException.class) on PatternDN.decode("cn=a\\"). It is red today, because decode returns cn=a, and green once the guard is in.

---

issue (non-blocking): Backend config DNs are still concatenated from an unescaped backend ID in the control panel and the installer (pre-existing).

opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/task/DeleteBaseDNAndBackendTask.java:422, :473, opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/task/DeleteIndexTask.java:243, opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/InstallerHelper.java:336

ds-cfg-backend-id has no pattern, and this PR's MonitorDNTestCase uses the legal ID monitor,ou=a b. For that ID these sites build ds-cfg-backend-id=monitor,ou=a b,cn=Backends,cn=config, so the control-panel delete targets an entry that is not the backend. The code predates the PR, and none of these files is in the diff, so a follow-up issue is enough.

    DN dn = DN.valueOf("cn=Backends,cn=config").child("ds-cfg-backend-id", backend.getBackendID());

---

suggestion (non-blocking): Tell operators to rebuild the DN-syntax equality indexes, because stored values written with RFC 2253 ; now normalize differently.

opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java:281-288

Before this PR, DN.decode stopped at the first ;, so a value such as member: cn=a;dc=b passed the DN syntax and was indexed under the key of cn=a. After the upgrade, the value and the filter both normalize to cn=a,dc=b, so an indexed equality search misses the entry until rebuild-index runs. A stored value with trailing content no longer parses. Reach is limited to values that clients wrote with ; or malformed. A release note is enough, or an Upgrade task like the 3.5.0 one (Upgrade.java:403-405, rebuildIndexesNamed(... EMR_DN_NAME, ... EMR_UNIQUE_MEMBER_NAME ...)). Not run: the index miss was traced by reading, and the ACI-load path was not traced.

---

suggestion (non-blocking): Release-note that entries with an empty RDN value cannot be read by older peers.

opendj-core/src/main/java/org/forgerock/opendj/ldap/AVA.java:336-338

Accepting an empty value is the stated decision, and IA5 String allows one, so dc=,dc=example,dc=com can pass schema checking. An older DS/RS decodes the DN of every update message with DN.valueOf (ByteArrayScanner.nextDN), and that call throws on the empty value. During a rolling upgrade such an Add therefore does not replicate to the old servers, and the entry cannot be imported into an older version. Not run: this needs a mixed-version topology.

---

suggestion (non-blocking): Pin the new hex-pair flush at the closing quote in PatternDN.

opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/PatternDNTest.java:54-57, opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/PatternDN.java:1107-1111

Each quoted row ends in a plain character ("a\,b", "a\2Cb", "J\C3\B6rg"), so the regular-character branch always flushes the pending hex pairs before the quote. If the appendHexChars call at :1110 is deleted, all 24 cases still pass, and cn="ab\2C" decodes as cn=ab while DN.valueOf reads ab,.

      { "cn=\"ab\\2C\",dc=x" },
      { "cn=\"\\C3\\B6\\,\",dc=x" },

Pin: these two rows in patternsWithoutWildcard(). The first fails if the flush at :1110 is deleted, the second if the flush at :1096 is deleted.

---

suggestion (non-blocking): The six control-panel call sites of getNewEntryDN are not covered by any test.

opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/NewUserPanel.java:316

NewEntryDNTestCase calls only the static AbstractNewEntryPanel.getNewEntryDN. No test references NewUserPanel, NewGroupPanel, NewOrganizationPanel, NewOrganizationalUnitPanel, NewDomainPanel or DuplicateEntryPanel, so changing one call site back to attr + "=" + value + "," + parent leaves every test green. The tests do cover the escaping, which lives in the helper. Pinning the call sites would need a headless Swing harness that the tree does not have, so this is your call.

---

nitpick (non-blocking): JMX names of a multi-valued RDN with plain values do change (+ between the AVAs), but the description says such values keep their name.

opendj-server-legacy/src/main/java/org/opends/server/config/JMXMBean.java:131

cn=a+sn=b went from cn-asn-b to cn-a+sn-b. The + is right and documented in the appendJmxRdn javadoc, because without it cn=a+sn=b and cn=as+n=b would share a name. Only the description's wording ("keeps the name it always had") needs the qualifier "single-valued RDN". No in-tree producer registers a multi-AVA monitor or config DN, and plainNamesAreUnchanged has single-AVA rows only.

[vharseko]

Thanks for the review. Round 2 is a3c43b4. The branch is rebased onto the current master (92ea60a, which brings #1162, #1163 and #1164); the round-1 commit is unchanged by the rebase, now 80ee6b9.

JMX names vs. SNMP (blocking): fixed. The encoded branch now writes a space as _, and a literal _ stays %5F, so the names stay distinct. cn=LDAP Connection Handler 0.0.0.0 port 1389,cn=monitor is Rdn2=cn-LDAP_Connection_Handler_0%2E0%2E0%2E0_port_1389, and its statistics entry ends in _Statistics.

• JMXMBeanNameTest: your row plus the statistics name, and a new distinctDNs group (a.b c, a.b_c, a.b%20c, a.b%5Fc).
• New SNMPMonitorConnectionHandlerNameTest ties the naming to the real matchers. It registers MBeans under the names getJmxName() gives a handler and its statistics (IPv4, IPv6, host name), then checks that SNMPMonitor.getConnectionHandlers(), getConnectionHandlersStatistics(), getConnectionHandlerStatistics() and getConnectionHandler() find them. Against the round-1 JMXMBean the IPv4, LDAPS and IPv6 rows fail and the localhost row passes. Your two JMXMBeanNameTest rows fail there too.
• Mutant: with a literal _ kept in the encoded branch, distinctDNs fails (user root/user_root, a.b c/a.b_c).
• The description now says what still changes: the handler names go from 0000 to 0%2E0%2E0%2E0, so a JMX client that looks the MBean up by its full name has to follow. That is the cost of injective names.

PatternDN trailing \: fixed. The guard throws with the SDK's ERR_ATTR_SYNTAX_DN_TRAILING_ESCAPE, the same message DN.valueOf() uses. PatternDNTest.patternWithATrailingBackslashIsRejected covers cn=a\, cn=\, cn=a,dc=x\ and the wildcard case cn=a*\. All four rows fail against the round-1 PatternDN.

Backend config DNs: fixed in this PR rather than in a follow-up, because it is the same defect class. Utilities.getBackendConfigDN() and Utilities.getIndexConfigDN() build them with DN.child(), and the five call sites use them: DeleteBaseDNAndBackendTask:422/:473, DeleteIndexTask (the VLV ds-cfg-name and attribute ds-cfg-attribute DNs too) and InstallerHelper.deleteBackend(). ConfigDNTestCase checks the helpers with monitor,ou=a b, a+sn=b, a\b and a. With the backend DN built by concatenation again, 8 of its 10 cases fail; only userRoot passes. As with the new-entry panels, the call sites themselves have no Swing-free test.

Rebuilding DN indexes after ; values: an upgrade task that verifies first. A ; value cannot be told apart by its index key, which is that of a legitimate cn=a; only the entries reveal it. An Upgrade task like the 3.5.0 one would therefore rebuild member/uniqueMember on every server, and the deb/rpm packages run upgrade -n --force. So the new 5.2.0 task verifyAndRebuildDNEqualityIndexes works as follows:

• it looks up the equality indexes with distinguishedNameMatch/uniqueMemberMatch in every enabled pluggable backend, including attributes from the instance's custom schema files;
• it runs verify-index --countErrors on them per base DN;
• it rebuilds only where errors are reported.

Complete-mode verification computes each entry's keys with the new rules, so the missing key of a ; value, or of a value that only now parses, counts as an error. The stale old key is harmless, since candidates are filtered again. The default answer is yes. run.sh in the Docker image now uses upgrade -n --force like the native packages, so the container does not just defer the task.

• DNEqualityIndexesUpgradeTestCase covers the index lookup (disabled backend, a backend ID with ,, a custom SUP distinguishedName attribute). It also covers verify-then-rebuild on a PDB backend whose trusted member index has had its keys removed: rebuilt once, only verified the second time. Five mutants of the task are caught.

Empty RDN values vs. older peers: documented in the upgrade chapter (chap-upgrade.adoc), as an IMPORTANT note in "To Upgrade Replicated Servers", next to the ACI and stored-DN notes. While tracing it:

• The empty-value case is worse than one Add not replicating. An older server decodes the DN inside Session.receive() (LDAPUpdateMsg → ByteArrayScanner.nextDN()), so the DataFormatException closes its replication session. After it reconnects it is sent the same change again, so it stops replicating altogether. The note says so, and says not to add or rename such entries until every server is upgraded. This is traced by reading; I did not run a mixed topology.
• The ACI load path: a DN with ; between RDNs now names the whole DN. An ACI with other trailing content no longer decodes, and AciListenerManager logs it and puts the server in lockdown mode. The note covers that too and links the lockdown section.

Closing-quote flush: pinned with your two rows, cn="ab\2C",dc=x and cn="\C3\B6\,",dc=x. With the appendHexChars call at the closing quote removed, only the first row fails. With the one before an escaped non-hex character removed, only the second row fails.

Control-panel call sites without tests: left as is. There is no headless Swing harness, and the escaping is pinned in getNewEntryDN().

Description wording: fixed. It now says "single-valued RDN" and gives the cn-a+sn-b example.

[maximthomas]

praise: Round 2 closes every round-1 finding where the bug is, and pins each fix with a test that fails against the round-1 code.

• JMXMBean.appendJmxRdn still writes _ for a space in the encoded branch, so the SNMP extension's Connection_Handler / _Statistics match survives; SNMPMonitorConnectionHandlerNameTest pins it for IPv4, IPv6 and host names.
• UpgradeTasks.verifyAndRebuildDNEqualityIndexes runs verify-index first, rebuilds only where it reports errors, and skips when the DN indexes are already queued for a rebuild.
• PatternDN now rejects a trailing \ as DN.valueOf() does, with the patternsWithATrailingBackslash rows.

---

issue (non-blocking): The HTTP connection handler's statistics monitor name is still not escaped.

opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java:477

LDAPConnectionHandler.java:683 and LDAPConnectionHandler2.java:753 now build DN.escapeAttributeValue(handlerName) + " Statistics", and ConnectionHandlerMonitor / ClientConnectionMonitorProvider escape the name for every handler, HTTP included. For an HTTP handler whose cn holds , and = (e.g. HTTP,ou=x), the handler entry lands at cn=HTTP\,ou\=x 0.0.0.0 port 8080,cn=monitor, but the statistics entry parses as cn=HTTP,ou=x 0.0.0.0 port 8080 Statistics,cn=monitor, two levels down, so ConfigFromConnection.isConnectionHandler (parent().equals(monitorDN)) never finds it. This is not a regression: the entry was already misplaced at the base. DN is already imported there.

statTracker = new HTTPStatistics(DN.escapeAttributeValue(handlerName) + " Statistics");

Pin: add HTTPConnectionHandler to MonitorDNTestCase.connectionHandlerClasses if it can start there.

---

question (non-blocking): Is the DN-index verification meant to scan enabled JDBC and Cassandra backends, and should the upgrade fail when their remote store is unreachable?

opendj-server-legacy/src/main/java/org/opends/server/tools/upgrade/UpgradeUtils.java:352

getDNEqualityIndexedAttributesPerBackend selects every enabled ds-cfg-pluggable-backend, and JDBCBackendConfiguration.xml / CASBackendConfiguration.xml extend pluggable-backend. verifyAndRebuildIndexes rebuilds on any nonzero verify result and throws ERR_UPGRADE_PERFORMING_POST_TASKS_FAIL when the rebuild fails. With the database down at upgrade time, the upgrade exits with an error after the version has been bumped, the later post-upgrade tasks are postponed, and the Docker run.sh never marks the container healthy. If that is intended (chap-upgrade says "every enabled backend"), this is fine as is. If remote stores are out of scope, skip them or warn instead of failing. This becomes an issue if the JDBC/CAS storages cannot be opened by an offline verify-index at all, because every such upgrade would then rebuild or fail. Not run: that needs a JDBC/Cassandra instance.

---

suggestion (non-blocking): The upgrade chapter does not say that monitor MBeans get new JMX names.

opendj-server-legacy/src/main/java/org/opends/server/config/JMXMBean.java:135, opendj-doc-generated-ref/src/main/asciidoc/install-guide/chap-upgrade.adoc:300-335

The PR description states the rename, and JMXMBeanNameTest pins it: cn-LDAP_Connection_Handler_0000_port_1389 becomes cn-LDAP_Connection_Handler_0%2E0%2E0%2E0_port_1389, and replication monitor names with ( or : change too. The new upgrade section covers DN indexes, ACIs and empty RDN values, but not this. A JMX consumer that keys on the ObjectName (exporter rules, check_jmx, dashboards) loses these MBeans after the upgrade of a default install. The in-repo SNMP extension keeps working.

The JMX names of monitor MBeans whose RDN value holds characters other than letters, digits and spaces change:
those characters are now percent-encoded instead of dropped. For example,
`cn-LDAP_Connection_Handler_0000_port_1389` becomes `cn-LDAP_Connection_Handler_0%2E0%2E0%2E0_port_1389`.
Update JMX monitoring rules that match these names after the upgrade.

---

suggestion (non-blocking): No test reaches postUpgrade() of verifyAndRebuildDNEqualityIndexes, so the skip, the backend-ID join and the per-base-DN loop are unpinned.

opendj-server-legacy/src/main/java/org/opends/server/tools/upgrade/UpgradeTasks.java:697-730, opendj-server-legacy/src/test/java/org/opends/server/tools/upgrade/DNEqualityIndexesUpgradeTestCase.java:219

DNEqualityIndexesUpgradeTestCase calls only the two static helpers. UpgradeTestCase stops at ERR_UPGRADE_VERSION_UP_TO_DATE before any task runs. A mutant that keys the :718-724 join wrongly, so that every backend hits baseDNs == null -> continue, stays green, which means the task could do nothing on a real upgrade. Moving the selection into a helper makes it testable:

static Map<String, Set<String>> getDNEqualityIndexesToVerify(
    final Map<String, Set<String>> attributesPerBackend, final Map<String, Set<String>> baseDNsPerBackend)
{
  final Map<String, Set<String>> attributesPerBaseDN = new TreeMap<>();
  for (final Map.Entry<String, Set<String>> backend : attributesPerBackend.entrySet())
  {
    final Set<String> baseDNs = baseDNsPerBackend.get(backend.getKey());
    if (baseDNs != null)
    {
      for (final String baseDN : baseDNs)
      {
        attributesPerBaseDN.put(baseDN, backend.getValue());
      }
    }
  }
  return attributesPerBaseDN;
}

Pin: attributes {"a,b": [member], "c": [uniqueMember]} and base DNs {"a,b": [o=a]} give exactly {o=a: [member]}. The :703 skip and the register("5.2.0", …) have no cheap pin.

---

suggestion (non-blocking): The four call sites moved to Utilities.getBackendConfigDN are unpinned; ConfigDNTestCase tests only the helper.

opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/InstallerHelper.java:336, opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/task/DeleteBaseDNAndBackendTask.java:422, :473, opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/task/DeleteIndexTask.java:243

Reverting any caller to DN.valueOf("ds-cfg-backend-id=" + id + ",cn=Backends,cn=config") keeps the suite green. InstallerHelper.deleteBackend needs no GUI because it deletes through the server's configuration handler:

// after adding a disabled backend config entry with ds-cfg-backend-id "a,b"
// (the shape DNEqualityIndexesUpgradeTestCase already adds)
new InstallerHelper().deleteBackend("a,b");
assertFalse(DirectoryServer.entryExists(DN.valueOf("ds-cfg-backend-id=a\\,b,cn=Backends,cn=config")));

The old concatenation turns it red. The two control-panel tasks are Swing-bound.

---

suggestion (non-blocking): The sort order and overwrite flag of readSchemaFiles are unpinned.

opendj-server-legacy/src/test/java/org/opends/server/tools/upgrade/DNEqualityIndexesUpgradeTestCase.java:101

The fixture writes only 99-user.ldif, and its OID is absent from the core schema. Arrays.sort on one file and addSchema(entry, false) with no conflict behave the same as the real code, so dropping either stays green. In production, a later file that redefines an attribute's EQUALITY rule decides whether its index is verified.

# 10-first.ldif, written before 99-user.ldif (which keeps SUP distinguishedName)
dn: cn=schema
objectClass: top
objectClass: ldapSubentry
objectClass: subschema
attributeTypes: ( 1.3.6.1.4.1.26027.1.999.1153 NAME 'myManager' EQUALITY caseIgnoreMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )

Pin: myManager is selected. Swap the two definitions between the files and it is not. overwrite=false turns the first assertion red.

---

suggestion (non-blocking): patternWithATrailingBackslashIsRejected pins the rejection but not the message or result code that commit a3c43b4 says match DN.valueOf().

opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/PatternDNTest.java:118

A mutant that throws another message or result code stays green.

@Test(dataProvider = "patternsWithATrailingBackslash")
public void patternWithATrailingBackslashIsRejected(String pattern) throws Exception
{
  try
  {
    PatternDN.decode(pattern);
    fail("expected a DirectoryException for " + pattern);
  }
  catch (DirectoryException e)
  {
    assertThat(e.getResultCode()).isEqualTo(ResultCode.INVALID_DN_SYNTAX);
    assertThat(e.getMessageObject().toString())
        .isEqualTo(ERR_ATTR_SYNTAX_DN_TRAILING_ESCAPE.get(pattern).toString());
  }
}

---

suggestion (non-blocking): The finally block of the verify-then-rebuild case hides a setup failure behind a NO_SUCH_OBJECT assertion.

opendj-server-legacy/src/test/java/org/opends/server/tools/upgrade/DNEqualityIndexesUpgradeTestCase.java:224-230

If addEntry(indexBranch) or addEntry(memberIndex) fails, deleteEntry(memberIndex.getName()) asserts SUCCESS and throws a new AssertionError. That error replaces the real cause, and the two remaining deletes are skipped, which leaves the backend config entry in place for the rest of the class.

finally
{
  setBackendEnabled(false);
  for (final DN dn : Arrays.asList(memberIndex.getName(), indexBranch.getName(), backend.getName()))
  {
    if (DirectoryServer.entryExists(dn))
    {
      TestCaseUtils.deleteEntry(dn);
    }
  }
}

---

nitpick (non-blocking): The new run.sh comment says --force is needed for verifying indexes, but upgrade -n alone already verifies.

opendj-packages/opendj-docker/run.sh:123-124

The verify task asks confirmYN(summary, YES) (UpgradeTasks.java:692), and chap-upgrade itself says "upgrade --no-prompt performs the verification too". --force only changes the default-no answers.

# --force accepts the tasks whose default answer is no, such as rebuilding indexes:
# nobody is there to run them by hand afterwards, as the native packages do too

[vharseko]

Thanks for the approval and the second round. Round 3 is afd91ea. The branch is rebased onto the current master (85b28b3, which brings #1165, #1167, #1168, #1169 and #1175); the rebase leaves the round-1 and round-2 commits unchanged, now 9b4e723 and aa0a36b.

HTTP statistics monitor name: fixed. HTTPConnectionHandler now builds DN.escapeAttributeValue(handlerName) + " Statistics", as both LDAP handlers do. MonitorDNTestCase.connectionHandlerClasses has an HTTP row: the handler LDAP,ou=internal starts on a free port, and its handler, client connections and statistics entries each sit right below cn=monitor. With the old line back, only the statistics assertion of the HTTP row fails.

DN index verification on JDBC/Cassandra backends: kept in scope, but it no longer fails the upgrade. These backends hold DN values too, and when their database can be read their indexes can miss the same keys, so skipping them would leave the problem unseen. What changes is the failure path. When verify-index fails and the rebuild fails as well, the task no longer throws ERR_UPGRADE_PERFORMING_POST_TASKS_FAIL. It logs the cause, warns with the indexes and the base DN (WARN_UPGRADE_VERIFY_DN_EQUALITY_INDEXES_FAILED), and the upgrade goes on with the next base DN and the remaining post-upgrade tasks. The server works without those keys, so a container whose database is down still finishes its upgrade and turns healthy. The step moved into UpgradeTasks.verifyAndRebuildOrWarn(), so it can be tested. DNEqualityIndexesUpgradeTestCase.indexesThatCannotBeVerifiedOnlyWarn uses a base DN that no backend holds: verifyAndRebuildIndexes throws ClientException there, and verifyAndRebuildOrWarn sends a WARNING notification instead. The verify-then-rebuild case now also goes through verifyAndRebuildOrWarn and checks the "match the entries" notice. The upgrade chapter tells administrators to verify such a backend by hand once it can be read. I did not run this against a real JDBC or Cassandra instance.

JMX names in the upgrade chapter: added, after the DN list, with your example. It also says that the replication monitor names, which hold (, ) and :, change, and that the SNMP connection handler still finds the connection handlers.

Base DN selection: extracted as UpgradeTasks.getDNEqualityIndexesToVerify(), as you suggested. eachBaseDNGetsTheIndexesOfItsBackend gives {"a,b": [member], "c": [uniqueMember]} with base DNs {"a,b": [o=a, o=b], "d": [o=d]} and expects exactly {o=a: [member], o=b: [member]}. The wrong-key mutant fails it.

InstallerHelper.deleteBackend(): pinned by the new DeleteBackendTestCase. It adds a disabled PDB backend with the ID a,b and its cn=Index branch, calls deleteBackend("a,b"), and expects both entries to be gone. With the concatenated DN back, the test fails. The two control-panel tasks are still Swing-bound.

Schema file order and overwrite: pinned by aLaterSchemaFileDecidesTheMatchingRule, using your two definitions of myManager. With caseIgnoreMatch in 10-first.ldif and SUP distinguishedName in 99-user.ldif, myManager is selected; swapped, it is not. The files are written in reverse name order. Both mutants fail it: reading the files in reverse order, and addSchema(entry, false).

Trailing \ result code and message: pinned. patternWithATrailingBackslashIsRejected checks INVALID_DN_SYNTAX and ERR_ATTR_SYNTAX_DN_TRAILING_ESCAPE.get(pattern). For the three rows without a wildcard it also checks that DN.valueOf(pattern) throws the same message. A mutant with another result code fails all four rows, and so does a mutant with another message.

Test cleanup: fixed. The finally block deletes only the entries that exist, so a failed setup surfaces as itself and the backend configuration does not stay behind for the rest of the class.

run.sh comment: fixed. It now says --force performs the tasks whose default answer is no, such as rebuilding indexes.

Tests: MonitorDNTestCase, PatternDNTest, DNEqualityIndexesUpgradeTestCase, DeleteBackendTestCase, ConfigDNTestCase, JMXMBeanNameTest and UpgradeTestCase pass. Eight mutants, one per point above, each turn its test red: the HTTP name, the result code, the message, the rethrow, the backend ID join, the file order, the overwrite flag and the installer DN.