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

Valery Kharseko
23 hours ago e9e614a4644288148abe6cb6fae3112e25a86d2d
refs
author Valery Kharseko <vharseko@3a-systems.ru>
Tuesday, October 6, 2026 08:58 +0200
committer GitHub <noreply@github.com>
Tuesday, October 6, 2026 08:58 +0200
commite9e614a4644288148abe6cb6fae3112e25a86d2d
tree ca23c4b933030c262585bfab6e6a31cc519d4de8 tree | zip | gz
parent 947c0c9a191cc5b771411e0d0f03d7a56aeedf1e view | diff
[#1159] Keep administrator-typed DNs intact in dsconfig batch lines, tools.properties and the SDK LDAPUrl (#1166)

Fixes #1159

A DN typed by an administrator was changed before it reached the parser
in three places. This PR fixes all three and documents the rules that
stay.

### 1. `dsconfig` batch lines (`DSConfig.toCommandArgs`)

A batch line is now split into arguments the way a POSIX shell splits
it, without expanding variables:

- Outside quotes, a backslash escapes the next char, as before.
- Inside double quotes, a backslash escapes only `"`, `\`, `$` and `` `
``. Before any other char it is kept, so `--set base-dn:"cn=a\,b,dc=x"`
now gives `cn=a\,b,dc=x` instead of `cn=a,b,dc=x`.
- Single quotes are supported and keep everything literally:
`'base-dn:o=My Company'` is one argument.
- Quoted and unquoted parts of one word make one argument (`"a"b` gives
`ab`; before, it gave two arguments), and an empty quoted word (`""`) is
an empty argument.
- A tab separates arguments as a space does.
- A trailing backslash is kept. Before, it threw
`StringIndexOutOfBoundsException`.
- A quote that is not closed is an error, as in a POSIX shell.
`dsconfig` prints `The following batch command has a quote that is not
closed: ...` and stops the batch with exit code 2 (`ERROR_USER_DATA`),
as it stops on a command that fails. Before, an unclosed `"` silently
took the rest of the line.

How `handleBatch` joins lines (`DSConfig.nextBatchCommand`):

- A line continues on the next one only when it ends in an unescaped
backslash, that is, an odd number of backslashes. Before, `pa\\` at the
end of a line was taken as a continuation, so it ran together with the
next command, or was dropped at the end of the file. `--commandFilePath`
writes such lines on UNIX, because it escapes a backslash in a value
that ends a command.
- A command whose last line continues still runs at the end of the file.
Before, it was dropped.
- Only the echoed command is trimmed. Before, the trim ran before the
split, so `description:value\ ` at the end of a line gave
`description:value\` instead of `description:value `.
- A line that holds only blanks is skipped, as an empty line is. Before,
it ran `dsconfig` with no subcommand, which failed with
`ERR_DSCFG_ERROR_MISSING_SUBCOMMAND` and stopped the batch.
- Empty lines and lines that start with `#` stay skipped inside a
continued command, as before, so a line of a long command can be
commented out. A POSIX shell would end the command there instead.

The batch file and standard input are still read in the platform
charset, as `--commandFilePath` writes them (`FileWriter`), so non-ASCII
characters in a batch line depend on the platform as before.

**Compatibility (for the release notes):** a single quote in a batch
line is now a quote character, not a literal. A line such as `--set
description:O'Brien` that worked before is now rejected. Write
`O\'Brien` or `"O'Brien"`.

### 2. Windows quoting in `CommandBuilder.escapeValue`

This differs from the suggested fix in the issue. Doubling every
backslash inside the quotes would break the printed command on the
Windows command line itself: a Windows program splits its command line
by the C runtime rules, which take backslashes literally unless they
precede a double quote. The value is now quoted by those rules.
Backslashes before a `"` or before the closing quote are doubled, and
`"` is escaped as `\"`. Before, a value with `"` (any ACI) was printed
broken, and a value ending in `\` escaped the closing quote.

With the batch tokenizer above, the Windows form now reads back as the
same value in a batch file, including `cn=a\,b,dc=x`. One difference
remains and cannot be removed while the command also stays valid for
`cmd`: a run of two or more backslashes that is not followed by a quote,
and `\$` or `` \` ``, are halved in a batch line but kept on the Windows
command line. The UNIX form is not changed.

The target is the C runtime split of the Java program's command line.
Characters that `cmd.exe` interprets itself (`&`, `|`, `<`, `>`) are not
caret-escaped: `cmd.exe` toggles its quote state at every `"`, including
an escaped one, so after `\"` they are unquoted for it, as on master.
The `dsconfig` description and the admin guide now say so.

### 3. `tools.properties` (`ArgumentParser`, both readers)

- The file is read as UTF-8. If it is not valid UTF-8, it is read as
ISO-8859-1, as before, so files written for earlier releases keep
working. A leading UTF-8 byte order mark is skipped in both cases: it is
removed as bytes before decoding, so a UTF-8 file with a BOM that later
got one Latin-1 byte still has its first key read.
- The backslash keeps its meaning in the properties format. Reading the
file differently would break `\uXXXX` escapes, line continuations, and
values already written with doubled backslashes. The template
`config/tools.properties`, the `Files` section of the tool man pages,
the developer guide, and the production password note now say that a
backslash must be doubled.

### 4. SDK `LDAPUrl`

- A run of `%XX` octets is decoded as UTF-8 (RFC 4516 section 2.1), so
`ldap:///cn=J%C3%B6rg,dc=x` gives `cn=Jörg,dc=x`. A run that is not
valid UTF-8 as a whole (such as `%f6`, which this class used to produce
for `ö`) decodes to one char per octet, as before. That includes valid
UTF-8 octets in the same run: `%C3%B6%F6` gives `öö`.
- A char outside the allowed set is encoded as the octets of its UTF-8
encoding, two hex digits each, including chars outside the BMP. Before,
`Ж` became `%416`, and a char below U+0010 got a single hex digit.

### Documentation

- `man-pages/_description-dsconfig.adoc` and
`admin-guide/chap-admin-tools.adoc`: how a batch line is split, with a
DN example; that a single quote is now a quote character, an unclosed
quote is rejected, an escaped trailing backslash does not continue the
line, and empty lines, lines of blanks and comments are skipped, also
inside a continued command; and that the Windows equivalent command
follows the C runtime rules, not `cmd.exe`.
- `man-pages/_files.adoc`, `server-dev-guide/chap-ldap-operations.adoc`,
and `admin-guide/chap-production.adoc`: the encoding of
`tools.properties` and doubled backslashes.

The changed pages were rendered with AsciidoctorJ to check that the
backslashes and quotes come out as written.

### Tests

- `DSConfigParseTest`: 11 new batch lines with their exact argument
lists, 4 lines with an unclosed quote that must be rejected with its
message, 12 batch files with the commands `nextBatchCommand` reads from
them (continuation, an escaped trailing backslash, an escaped trailing
blank, a continued last line, a line of blanks, an empty line and a
comment inside a continued command), and 2 commands run through
`DSConfig.runBatchCommand`, the body of the `handleBatch` loop: a line
with an unclosed quote gives exit code 2 and the message, and an escaped
trailing blank reaches `dsconfig` untrimmed.
- New `CommandBuilderTestCase`: Windows quoting (ACI quotes, trailing
backslash, backslash before a quote, empty value) and the unchanged UNIX
form, including the escaped single quote that lets a `--commandFilePath`
file be replayed as a batch.
- New `ArgumentParserPropertiesFileTestCase`: UTF-8, ISO-8859-1
fallback, byte order mark (also in the ISO-8859-1 fallback), and doubled
backslash, each through `--propertiesFilePath` and through the
explicit-path `parseArguments`.
- `LDAPUrlTestCase`: UTF-8 decoding of the DN and filter, the non-UTF-8
fallback, UTF-8 encoding with a round trip, and two hex digits for an
octet below 0x10.

Before the fix, 22 of the new cases failed (5 in `LDAPUrlTestCase`, 7 in
opendj-cli, 10 in `DSConfigParseTest`). Each of four mutants turned
exactly its own tests red: no doubling of a trailing backslash, no
ISO-8859-1 fallback in `LDAPUrl`, the same in `ArgumentParser`, and no
BOM skip. With the fix, the full unit suites pass with no failures:
opendj-core 8188, opendj-grizzly 1040, opendj-cli 61,
opendj-ldap-toolkit 291, and opendj-config 562 tests.

Review round 1, rebased on `751a4d2ee8`: seven more mutants each turned
only their own new tests red: an unclosed single quote taking the rest
of the line, an unclosed double quote, the trim before the split placed
in `nextBatchCommand`, any trailing backslash continuing the line, a
continued last line being dropped, the BOM kept in the ISO-8859-1
fallback, and one hex digit for an octet below 0x10. The full unit
suites of the modules this round changes pass: opendj-core 8189,
opendj-cli 65, and opendj-config 574 tests, with javadoc
(`failOnWarnings`) on all three. The changed doc pages were rendered
with AsciidoctorJ again.

Review round 2, rebased on `85b28b3d0f`: seven more mutants each turned
only their own new tests red: a line of blanks run as a command, a
comment ending a continued command, the batch error path returning
success, the batch error path without its message, another message key
for an unclosed quote, the trim put back before the split where
`handleBatch` passes the command to `dsconfig`, and no escape of a
single quote in the UNIX form. The full unit suites pass: opendj-core
8190, opendj-cli 66, and opendj-config 580 tests, with javadoc on all
three. The changed doc pages were rendered with AsciidoctorJ.

### Not changed here

- The `LDAPUrl` constructor writes the filter into `toString()` without
percent-encoding it (`(cn=J\C3\B6rg)`), while `equals()` encodes it.
This is older behavior and not about UTF-8. The new test checks the DN
part and the round trip.
- `ArgumentParser.parseArguments(String[], String, boolean)` matches
property names case-sensitively (the key must be `basedn`, not
`baseDN`). The `--propertiesFilePath` reader lowercases them. Nothing in
the repository calls the former.
13 files modified
2 files added
708 ■■■■■ changed files
opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentParser.java 41 ●●●●● diff | view | raw | blame | history
opendj-cli/src/main/java/com/forgerock/opendj/cli/CommandBuilder.java 33 ●●●●● diff | view | raw | blame | history
opendj-cli/src/test/java/com/forgerock/opendj/cli/ArgumentParserPropertiesFileTestCase.java 103 ●●●●● diff | view | raw | blame | history
opendj-cli/src/test/java/com/forgerock/opendj/cli/CommandBuilderTestCase.java 60 ●●●●● diff | view | raw | blame | history
opendj-config/src/main/java/org/forgerock/opendj/config/dsconfig/DSConfig.java 157 ●●●●● diff | view | raw | blame | history
opendj-config/src/main/resources/com/forgerock/opendj/dsconfig/dsconfig.properties 2 ●●●●● diff | view | raw | blame | history
opendj-config/src/test/java/org/forgerock/opendj/config/dsconfig/DSConfigParseTest.java 138 ●●●●● diff | view | raw | blame | history
opendj-core/src/main/java/org/forgerock/opendj/ldap/LDAPUrl.java 73 ●●●●● diff | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldap/LDAPUrlTestCase.java 65 ●●●●● diff | view | raw | blame | history
opendj-doc-generated-ref/src/main/asciidoc/admin-guide/chap-admin-tools.adoc 11 ●●●●● diff | view | raw | blame | history
opendj-doc-generated-ref/src/main/asciidoc/admin-guide/chap-production.adoc 4 ●●●●● diff | view | raw | blame | history
opendj-doc-generated-ref/src/main/asciidoc/man-pages/_description-dsconfig.adoc 8 ●●●●● diff | view | raw | blame | history
opendj-doc-generated-ref/src/main/asciidoc/man-pages/_files.adoc 4 ●●●●● diff | view | raw | blame | history
opendj-doc-generated-ref/src/main/asciidoc/server-dev-guide/chap-ldap-operations.adoc 4 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/resource/config/tools.properties 5 ●●●●● diff | view | raw | blame | history