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

Valery Kharseko
14 hours ago 3f4deb91789189521d577457bd6da27de8fd75b1
refs
author Valery Kharseko <vharseko@3a-systems.ru>
Wednesday, October 7, 2026 10:31 +0200
committer GitHub <noreply@github.com>
Wednesday, October 7, 2026 10:31 +0200
commit3f4deb91789189521d577457bd6da27de8fd75b1
tree 7d0cbf5e93905f8660666b7986dacd6973e81f6f tree | zip | gz
parent 58666fd764de42df1615625ea5623dcc1a1a6e15 view | diff
[#1153] Parse the whole DN string, and build or split DN strings through DN instead of string operations (#1171)

## 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](#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 cannot be verified, for example in a JDBC
or Cassandra backend whose database is down during the upgrade, the task
does not rebuild them: it warns, names the indexes and the base DN, and
the upgrade goes on, leaving these indexes as they were for the
administrator to verify once the backend can be read. A rebuild that
fails, after the verification found missing keys, fails the upgrade, as
the other index rebuilds of the upgrade do: it may have left the indexes
untrusted, so that searches cannot use them. The indexes of the base DNs
after it are then neither verified nor rebuilt, since the same cause
would most likely make their rebuild fail too: each of them is named in
a warning. To tell the two cases apart, `VerifyIndex.countIndexErrors()`
returns -1 when the indexes could not be verified; the tool itself, with
`--countErrors`, exits with 1 both then and on a single error, and keeps
doing so.
- **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. A global ACI
(`ds-cfg-global-aci`) of that form stops the server from starting
instead, and has to be corrected in `config.ldif`.
- **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 schema files listed in name order,
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 with its notice,
then only verified, a failed rebuild that fails the upgrade and names
the base DNs after it, and a warning without a rebuild, not a failure,
for a base DN that no backend holds and for a backend whose verification
throws;
- 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`).

Round-4 mutants, each caught by `DNEqualityIndexesUpgradeTestCase`: a
failed rebuild that only warns, indexes that cannot be verified rebuilt
anyway, rebuilt indexes reported as consistent, the schema files not
sorted, and `VerifyIndex.countIndexErrors()` counting errors only with
`--countErrors`.

Round-5 mutants, each caught by `DNEqualityIndexesUpgradeTestCase`: the
base DNs after a failed rebuild verified and rebuilt anyway, the failed
rebuild not rethrown once they are named, and a backend whose
verification throws counted as one error in `VerifyIndex`.

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
55 files modified
15 files added
3277 ■■■■■ changed files
opendj-cli/src/main/java/com/forgerock/opendj/cli/Utils.java 3 ●●●●● diff | view | raw | blame | history
opendj-cli/src/test/java/com/forgerock/opendj/cli/UtilsTestCase.java 21 ●●●●● diff | view | raw | blame | history
opendj-core/src/main/java/org/forgerock/opendj/ldap/AVA.java 27 ●●●●● diff | view | raw | blame | history
opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java 9 ●●●●● diff | view | raw | blame | history
opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/NameAndOptionalUIDSyntaxImpl.java 36 ●●●●● diff | view | raw | blame | history
opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/UniqueMemberEqualityMatchingRuleImpl.java 14 ●●●●● diff | view | raw | blame | history
opendj-core/src/main/java/org/forgerock/opendj/ldif/TemplateTag.java 20 ●●●●● diff | view | raw | blame | history
opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties 7 ●●●●● diff | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldap/AVATestCase.java 37 ●●●●● diff | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java 79 ●●●●● diff | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/DistinguishedNameEqualityMatchingRuleTest.java 15 ●●●●● diff | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/NameAndOptionalUIDSyntaxTest.java 48 ●●●●● diff | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/UniqueMemberEqualityMatchingRuleTest.java 8 ●●●●● diff | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldif/TemplateTagTestCase.java 22 ●●●●● diff | view | raw | blame | history
opendj-doc-generated-ref/src/main/asciidoc/install-guide/chap-upgrade.adoc 22 ●●●●● diff | view | raw | blame | history
opendj-packages/opendj-docker/README.md 6 ●●●●● diff | view | raw | blame | history
opendj-packages/opendj-docker/run.sh 6 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/task/DeleteBaseDNAndBackendTask.java 6 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/task/DeleteIndexTask.java 13 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/AbstractNewEntryPanel.java 44 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/DuplicateEntryPanel.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/NewDomainPanel.java 3 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/NewGroupPanel.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/NewOrganizationPanel.java 3 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/NewOrganizationalUnitPanel.java 3 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ui/NewUserPanel.java 3 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/util/Utilities.java 40 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/quicksetup/installer/InstallerHelper.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/PatternDN.java 38 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/PatternRDN.java 14 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/UserAttr.java 6 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java 3 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java 3 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/config/JMXMBean.java 113 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/core/DirectoryServer.java 23 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/extensions/DiskSpaceMonitor.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/monitors/ClientConnectionMonitorProvider.java 4 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/monitors/ConnectionHandlerMonitor.java 5 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/monitors/EntryCacheMonitorProvider.java 4 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/monitors/LocalBackendMonitor.java 3 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPConnectionHandler.java 2 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/replication/common/MultiDomainServerState.java 58 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/replication/server/ReplicationServerDomain.java 22 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationMonitor.java 25 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/tools/VerifyIndex.java 67 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/tools/tasks/TaskClient.java 11 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/tools/upgrade/Upgrade.java 7 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/tools/upgrade/UpgradeTasks.java 289 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/tools/upgrade/UpgradeUtils.java 143 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/types/LDAPURL.java 38 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/tool.properties 19 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/guitools/controlpanel/ui/NewEntryDNTestCase.java 68 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/guitools/controlpanel/util/ConfigDNTestCase.java 70 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/guitools/controlpanel/util/UnescapeUtf8TestCase.java 60 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/quicksetup/installer/DeleteBackendTestCase.java 85 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/PatternDNTest.java 152 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/UserAttrTest.java 86 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/IndexKeyRemover.java 80 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/config/JMXMBeanNameTest.java 105 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/monitors/MonitorDNTestCase.java 268 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/replication/common/MultiDomainServerStateTest.java 32 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/replication/server/ReplicationServerDomainMonitorNameTest.java 48 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/replication/service/ReplicationMonitorNameTest.java 49 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/snmp/SNMPMonitorConnectionHandlerNameTest.java 91 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/tools/tasks/TaskClientTaskDNTest.java 72 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/tools/upgrade/DNEqualityIndexesUpgradeTestCase.java 563 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/types/LDAPURLTestCase.java 42 ●●●●● diff | view | raw | blame | history