From 458f009de6ccb989ac565da62ac407f20755303e Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Sun, 04 Oct 2026 07:23:04 +0000
Subject: [PATCH] [#1156] Keep colons inside the -J/--control value, and report an unreadable control value file instead of failing with a NullPointerException (#1163)
---
opendj-ldap-toolkit/src/main/java/com/forgerock/opendj/ldap/tools/Utils.java | 32 +++++++---
opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/ControlArgumentTestCase.java | 134 ++++++++++++++++++++++++++++++++++++++++++++
opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/LDAPSearchTestCase.java | 12 ++++
3 files changed, 167 insertions(+), 11 deletions(-)
diff --git a/opendj-ldap-toolkit/src/main/java/com/forgerock/opendj/ldap/tools/Utils.java b/opendj-ldap-toolkit/src/main/java/com/forgerock/opendj/ldap/tools/Utils.java
index 067fa09..f203b4a 100644
--- a/opendj-ldap-toolkit/src/main/java/com/forgerock/opendj/ldap/tools/Utils.java
+++ b/opendj-ldap-toolkit/src/main/java/com/forgerock/opendj/ldap/tools/Utils.java
@@ -18,8 +18,10 @@
*/
package com.forgerock.opendj.ldap.tools;
+import static com.forgerock.opendj.cli.ArgumentConstants.OPTION_LONG_CONTROL;
import static com.forgerock.opendj.cli.ArgumentConstants.USE_SYSTEM_STREAM_TOKEN;
import static com.forgerock.opendj.cli.CliConstants.NO_WRAPPING_BY_DEFAULT;
+import static com.forgerock.opendj.cli.CliMessages.ERR_FILEARG_CANNOT_READ_FILE;
import static com.forgerock.opendj.cli.Utils.filterExitCode;
import static com.forgerock.opendj.cli.Utils.readBytesFromFile;
import static com.forgerock.opendj.cli.Utils.secondsToTimeString;
@@ -313,19 +315,21 @@
/**
* Parse the specified command line argument to create the appropriate
* LDAPControl. The argument string should be in the format
- * controloid[:criticality[:value|::b64value|:<fileurl]]
+ * controloid[:criticality[:value|::b64value|:<filePath]]
+ * <p>
+ * Everything after the second colon is the value, so the value, the
+ * base64 string and the file path may all contain colons.
*
* @param argString
* The argument string containing the encoded control
* information.
- * @return The control decoded from the provided string, or
- * <CODE>null</CODE> if an error occurs while parsing the argument
- * value.
+ * @return The control decoded from the provided string.
* @throws org.forgerock.opendj.ldap.DecodeException
- * If an error occurs.
+ * If the criticality is invalid, the base64 value cannot be
+ * decoded or the file cannot be read.
*/
private static GenericControl getControl(final String argString) throws DecodeException {
- final String[] control = argString.split(":");
+ final String[] control = argString.split(":", 3);
final int nbControlElements = control.length;
final String controlOID = readControlID(control[0]);
@@ -339,14 +343,20 @@
}
final ByteString controlValue;
- if (control[2].isEmpty()) {
- controlValue = ByteString.valueOfBase64(control[3]);
+ if (control[2].startsWith(":")) {
+ try {
+ controlValue = ByteString.valueOfBase64(control[2].substring(1));
+ } catch (final LocalizedIllegalArgumentException e) {
+ throw DecodeException.error(e.getMessageObject(), e);
+ }
} else if (control[2].startsWith("<")) {
// Read data from the file.
+ final String filePath = control[2].substring(1);
try {
- controlValue = ByteString.wrap(readBytesFromFile(control[2].substring(1)));
- } catch (final Exception e) {
- return null;
+ controlValue = ByteString.wrap(readBytesFromFile(filePath));
+ } catch (final IOException e) {
+ throw DecodeException.error(
+ ERR_FILEARG_CANNOT_READ_FILE.get(filePath, OPTION_LONG_CONTROL, e.getMessage()), e);
}
} else {
controlValue = ByteString.valueOfUtf8(control[2]);
diff --git a/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/ControlArgumentTestCase.java b/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/ControlArgumentTestCase.java
new file mode 100644
index 0000000..d6fba64
--- /dev/null
+++ b/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/ControlArgumentTestCase.java
@@ -0,0 +1,134 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions copyright [year] [name of copyright owner]".
+ *
+ * Copyright 2026 3A Systems, LLC.
+ */
+package com.forgerock.opendj.ldap.tools;
+
+import static com.forgerock.opendj.ldap.tools.ToolsMessages.ERR_TOOL_INVALID_CONTROL_STRING;
+import static com.forgerock.opendj.util.OperatingSystem.isWindows;
+import static org.fest.assertions.Assertions.assertThat;
+import static org.testng.Assert.fail;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.List;
+
+import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.ResultCode;
+import org.forgerock.opendj.ldap.controls.Control;
+import org.forgerock.testng.ForgeRockTestCase;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import com.forgerock.opendj.cli.CommonArguments;
+import com.forgerock.opendj.cli.StringArgument;
+
+/** Tests the parsing of the {@code -J/--control} argument by {@link Utils#readControls(StringArgument)}. */
+@Test
+public final class ControlArgumentTestCase extends ForgeRockTestCase {
+
+ @DataProvider
+ public Object[][] validControls() {
+ return new Object[][] {
+ { "1.2.3.4", "1.2.3.4", false, null },
+ { "1.2.3.4:true", "1.2.3.4", true, null },
+ { "1.2.3.4:FALSE:value", "1.2.3.4", false, "value" },
+ { "1.2.3.4:true:", "1.2.3.4", true, "" },
+ { "pwpolicy:true", "1.3.6.1.4.1.42.2.27.8.5.1", true, null },
+ // Everything after the second colon is the value, colons included.
+ { "2.16.840.1.113730.3.4.18:true:dn:uid=bjensen,ou=People,dc=example,dc=com",
+ "2.16.840.1.113730.3.4.18", true, "dn:uid=bjensen,ou=People,dc=example,dc=com" },
+ { "2.16.840.1.113730.3.4.18:true:u:bjensen", "2.16.840.1.113730.3.4.18", true, "u:bjensen" },
+ { "1.2.3.4:false:urn:example:value", "1.2.3.4", false, "urn:example:value" },
+ { "1.2.3.4:false:ldap://host:1389/dc=example", "1.2.3.4", false, "ldap://host:1389/dc=example" },
+ // The base64 form decodes to a value that can itself hold colons.
+ { "1.2.3.4:true::" + base64("dn:uid=bjensen"), "1.2.3.4", true, "dn:uid=bjensen" },
+ { "1.2.3.4:true::", "1.2.3.4", true, "" },
+ };
+ }
+
+ @Test(dataProvider = "validControls")
+ public void testValidControl(final String argument, final String oid, final boolean critical,
+ final String value) throws Exception {
+ final Control control = readSingleControl(argument);
+ assertThat(control.getOID()).isEqualTo(oid);
+ assertThat(control.isCritical()).isEqualTo(critical);
+ if (value == null) {
+ assertThat(control.hasValue()).isFalse();
+ } else {
+ assertThat(control.hasValue()).isTrue();
+ assertThat(control.getValue().toString()).isEqualTo(value);
+ }
+ }
+
+ @Test
+ public void testValueReadFromFileWhosePathHoldsAColon() throws Exception {
+ // On Windows the drive letter puts a colon in every absolute path; elsewhere the file name carries one.
+ final File dir = Files.createTempDirectory("control-value").toFile();
+ final File file = new File(dir, isWindows() ? "control-value.ber" : "control:value.ber");
+ try {
+ final byte[] bytes = { 0x04, 0x03, 'a', ':', 'b' };
+ Files.write(file.toPath(), bytes);
+
+ final Control control = readSingleControl("1.2.3.4:true:<" + file.getAbsolutePath());
+ assertThat(control.getOID()).isEqualTo("1.2.3.4");
+ assertThat(control.isCritical()).isTrue();
+ assertThat(control.getValue()).isEqualTo(ByteString.wrap(bytes));
+ } finally {
+ file.delete();
+ dir.delete();
+ }
+ }
+
+ @DataProvider
+ public Object[][] invalidControls() {
+ final String missingFile = new File(System.getProperty("java.io.tmpdir"), "no-such-dir-1156/value.ber")
+ .getAbsolutePath();
+ return new Object[][] {
+ { "1.2.3.4:invalidcriticality" },
+ { "1.2.3.4:invalidcriticality:value" },
+ { "1.2.3.4:true:<" + missingFile },
+ { "1.2.3.4:true::not*base64" },
+ };
+ }
+
+ @Test(dataProvider = "invalidControls")
+ public void testInvalidControlIsReportedAsAnInvalidControlString(final String argument) throws Exception {
+ try {
+ readControls(argument);
+ fail("Expected the control '" + argument + "' to be rejected");
+ } catch (final LDAPToolException e) {
+ assertThat(e.getResultCode()).isEqualTo(ResultCode.CLIENT_SIDE_PARAM_ERROR.intValue());
+ assertThat(e.getMessage()).isEqualTo(ERR_TOOL_INVALID_CONTROL_STRING.get(argument).toString());
+ }
+ }
+
+ private static Control readSingleControl(final String argument) throws Exception {
+ final List<Control> controls = readControls(argument);
+ assertThat(controls).hasSize(1);
+ return controls.get(0);
+ }
+
+ private static List<Control> readControls(final String argument) throws Exception {
+ final StringArgument controlArg = CommonArguments.controlArgument();
+ controlArg.addValue(argument);
+ controlArg.setPresent(true);
+ return Utils.readControls(controlArg);
+ }
+
+ private static String base64(final String value) {
+ return ByteString.valueOfBytes(value.getBytes(StandardCharsets.UTF_8)).toBase64String();
+ }
+}
diff --git a/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/LDAPSearchTestCase.java b/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/LDAPSearchTestCase.java
index f74170e..5fb16ca 100644
--- a/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/LDAPSearchTestCase.java
+++ b/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/LDAPSearchTestCase.java
@@ -12,6 +12,7 @@
* information: "Portions Copyright [year] [name of copyright owner]".
*
* Copyright 2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package com.forgerock.opendj.ldap.tools;
@@ -142,6 +143,11 @@
argLists.add(args("-b", "", "-J", "1.2.3.4:invalidcriticality", "(objectClass=*)"));
reasonList.add(ERR_TOOL_INVALID_CONTROL_STRING.get("1.2.3.4:invalidcriticality"));
+ argLists.add(args("-b", "", "-J", "1.2.3.4:true:<src/test/resources/no-such-control-value.ber",
+ "(objectClass=*)"));
+ reasonList.add(ERR_TOOL_INVALID_CONTROL_STRING.get(
+ "1.2.3.4:true:<src/test/resources/no-such-control-value.ber"));
+
argLists.add(args("-b", "", "-s", "invalid", "(objectClass=*)"));
reasonList.add(ERR_MCARG_VALUE_NOT_ALLOWED.get("searchScope", "invalid"));
@@ -236,6 +242,12 @@
}
@Test
+ public void testLdapSearchWithControlValueHoldingColons() throws Exception {
+ controls.add(ProxiedAuthV2RequestControl.newControl("dn:uid=bjensen,ou=People,dc=example,dc=com"));
+ runToolOnMockedServer("-J", "2.16.840.1.113730.3.4.18:true:dn:uid=bjensen,ou=People,dc=example,dc=com");
+ }
+
+ @Test
public void testLdapSearchWithSimplePaged() throws Exception {
controls.add(SimplePagedResultsControl.newControl(true, 10, ByteString.empty()));
runToolOnMockedServer("--simplePageSize", "10");
--
Gitblit v1.10.0