From 3e38fe42c087b1c082d3fe732a0e3cfaefb32b28 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sat, 19 Sep 2026 10:31:55 +0300 Subject: [PATCH 1/2] Wrap malformed numeric values in a domain exception instead of a bare NumberFormatException Active Directory bitmask attributes (userAccountControl, groupType) and sync tokens share one root cause across ADUserAccountControl, ADGroupType and ActiveDirectoryChangeLogSyncStrategy: Integer.parseInt with nothing around it. ADLdapUtil gets parseADInteger/parseADLong, and all three classes route through it, so a corrupted AD attribute or a tampered sync token fails with a ConnectorException naming the value instead of a bare NumberFormatException three frames deep. PagedSearchStrategy's cookie parsing moves into the try/catch that already exists for the other half of the same cookie. The maven plugin's POM property conversion (PropertyBag), the JDBC value conversion for numeric SQL types (SQLUtil.attribute2jdbcValue) and the five primitive decoders of the XML wire format (XmlObjectDecoder) get the same treatment, each in the exception type already used around them (MojoExecutionException, ConnectorException). The proxy port system property and two contract-test configuration parsers get a one-line wrap with the same reasoning. Left alone, and dismissed on GitHub as unreachable: two hex parses built from raw bytes via String.format("%02X..."), a test connector's own revision counter, the JVM's own java.specification.version, and two substrings a regex just validated as exactly 13 digits - none of these can actually fail to parse. --- .../identityconnectors/dbcommon/SQLUtil.java | 9 ++ .../dbcommon/SQLUtilTests.java | 22 +++++ .../test/AuthenticationApiOpTests.java | 8 +- .../contract/test/ConnectorHelper.java | 8 +- .../impl/serializer/xml/XmlObjectDecoder.java | 31 +++++-- .../serializer/xml/XmlObjectDecoderTest.java | 85 +++++++++++++++++++ .../RemoteWSFrameworkConnectionInfo.java | 10 ++- .../RemoteWSFrameworkConnectionInfoTest.java | 54 ++++++++++++ .../identityconnectors/ldap/ADGroupType.java | 13 +-- .../identityconnectors/ldap/ADLdapUtil.java | 31 ++++++- .../ldap/ADUserAccountControl.java | 27 +++--- .../ldap/search/PagedSearchStrategy.java | 3 +- .../ActiveDirectoryChangeLogSyncStrategy.java | 9 +- .../ldap/ADGroupTypeTest.java | 36 ++++++++ .../ldap/ADLdapUtilTest.java | 56 ++++++++++++ .../ldap/ADUserAccountControlTest.java | 43 ++++++++++ .../ldap/search/PagedSearchStrategyTest.java | 37 ++++++++ .../forgerock/openicf/maven/PropertyBag.java | 9 +- 18 files changed, 457 insertions(+), 34 deletions(-) create mode 100644 OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java create mode 100644 OpenICF-java-framework/connector-framework-server/src/test/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfoTest.java create mode 100644 OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADGroupTypeTest.java create mode 100644 OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java create mode 100644 OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADUserAccountControlTest.java create mode 100644 OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java diff --git a/OpenICF-dbcommon/src/main/java/org/identityconnectors/dbcommon/SQLUtil.java b/OpenICF-dbcommon/src/main/java/org/identityconnectors/dbcommon/SQLUtil.java index 6aef2ed80..af21e3837 100644 --- a/OpenICF-dbcommon/src/main/java/org/identityconnectors/dbcommon/SQLUtil.java +++ b/OpenICF-dbcommon/src/main/java/org/identityconnectors/dbcommon/SQLUtil.java @@ -906,6 +906,7 @@ public static Object attribute2jdbcValue(final Object value, int sqlType) throws if (value == null) { return null; } + try { switch (sqlType) { // Known conversions case Types.DECIMAL: @@ -948,6 +949,14 @@ public static Object attribute2jdbcValue(final Object value, int sqlType) throws } else { return Long.valueOf(value.toString()); } + default: + break; + } + } catch (NumberFormatException e) { + throw new ConnectorException("Value '" + value + "' is not valid for SQL type " + + sqlType, e); + } + switch (sqlType) { case Types.TIMESTAMP: if (value instanceof String) { return string2Timestamp((String) value); diff --git a/OpenICF-dbcommon/src/test/java/org/identityconnectors/dbcommon/SQLUtilTests.java b/OpenICF-dbcommon/src/test/java/org/identityconnectors/dbcommon/SQLUtilTests.java index bf8228b97..2f2d9728c 100644 --- a/OpenICF-dbcommon/src/test/java/org/identityconnectors/dbcommon/SQLUtilTests.java +++ b/OpenICF-dbcommon/src/test/java/org/identityconnectors/dbcommon/SQLUtilTests.java @@ -19,6 +19,7 @@ * enclosed by brackets [] replaced by your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" * ==================== + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.dbcommon; @@ -26,6 +27,7 @@ import static org.testng.AssertJUnit.assertNotNull; import static org.testng.AssertJUnit.assertTrue; +import org.identityconnectors.framework.common.exceptions.ConnectorException; import java.io.ByteArrayInputStream; import java.math.BigDecimal; import java.sql.Blob; @@ -597,6 +599,26 @@ public void testAttribute2JdbcValue() throws SQLException { assertEquals(true, actual); } + /** + * A value that does not fit the target SQL type must fail with a + * ConnectorException naming the value and the type, not a bare + * NumberFormatException. + */ + @Test(expectedExceptions = ConnectorException.class) + public void testAttribute2JdbcValueRejectsMalformedDouble() throws SQLException { + SQLUtil.attribute2jdbcValue("not-a-number", Types.DOUBLE); + } + + @Test(expectedExceptions = ConnectorException.class) + public void testAttribute2JdbcValueRejectsMalformedFloat() throws SQLException { + SQLUtil.attribute2jdbcValue("not-a-number", Types.FLOAT); + } + + @Test(expectedExceptions = ConnectorException.class) + public void testAttribute2JdbcValueRejectsMalformedInteger() throws SQLException { + SQLUtil.attribute2jdbcValue("not-a-number", Types.INTEGER); + } + /** * We need this helper class as InitialContextFactory class name value to * Hashtable into InitialContext. We must use instantiable classname and diff --git a/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/AuthenticationApiOpTests.java b/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/AuthenticationApiOpTests.java index e0a502fde..5276ee593 100644 --- a/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/AuthenticationApiOpTests.java +++ b/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/AuthenticationApiOpTests.java @@ -36,6 +36,7 @@ import java.util.Set; import org.identityconnectors.common.security.GuardedString; +import org.identityconnectors.contract.exceptions.ContractException; import org.identityconnectors.contract.exceptions.ObjectNotFoundException; import org.identityconnectors.framework.api.operations.APIOperation; import org.identityconnectors.framework.api.operations.AuthenticationApiOp; @@ -508,7 +509,12 @@ private long getLongTestParam(String name, long defaultValue) { Object valueObject = getDataProvider().getTestSuiteAttribute(name, TEST_NAME); if(valueObject != null) { - longValue = Long.parseLong(valueObject.toString()); + try { + longValue = Long.parseLong(valueObject.toString()); + } catch (NumberFormatException e) { + throw new ContractException("Test suite attribute '" + name + + "' is not a valid number: '" + valueObject + "'", e); + } } } catch (ObjectNotFoundException ex) { } diff --git a/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/ConnectorHelper.java b/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/ConnectorHelper.java index ae9be0048..04f439552 100644 --- a/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/ConnectorHelper.java +++ b/OpenICF-java-framework/connector-framework-contract/src/main/java/org/identityconnectors/contract/test/ConnectorHelper.java @@ -970,7 +970,13 @@ private static ConnectorInfoManager getRemoteManager(final DataProvider dataProv host = System.getProperty("serverHost"); } if (StringUtil.isNotBlank(System.getProperty("serverPort"))) { - port = Integer.parseInt(System.getProperty("serverPort")); + String serverPort = System.getProperty("serverPort"); + try { + port = Integer.parseInt(serverPort); + } catch (NumberFormatException e) { + throw new ContractException("System property serverPort is not a valid port " + + "number: '" + serverPort + "'", e); + } } if (StringUtil.isNotBlank(System.getProperty("serverKey"))) { key = System.getProperty("serverKey"); diff --git a/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java b/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java index 06bca1970..9b19c7cbe 100644 --- a/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java +++ b/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java @@ -19,6 +19,7 @@ * enclosed by brackets [] replaced by your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" * ==================== + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.framework.impl.serializer.xml; @@ -180,7 +181,11 @@ private boolean decodeBoolean(String v) { } private byte decodeByte(String v) { - return Byte.decode(v); + try { + return Byte.decode(v); + } catch (NumberFormatException e) { + throw new ConnectorException("Malformed byte value on the wire: '" + v + "'", e); + } } private byte[] decodeByteArray(String base64) { @@ -204,19 +209,35 @@ private Class decodeClass(String type) { } private double decodeDouble(String val) { - return Double.parseDouble(val); + try { + return Double.parseDouble(val); + } catch (NumberFormatException e) { + throw new ConnectorException("Malformed double value on the wire: '" + val + "'", e); + } } private float decodeFloat(String val) { - return Float.parseFloat(val); + try { + return Float.parseFloat(val); + } catch (NumberFormatException e) { + throw new ConnectorException("Malformed float value on the wire: '" + val + "'", e); + } } private int decodeInt(String val) { - return Integer.parseInt(val); + try { + return Integer.parseInt(val); + } catch (NumberFormatException e) { + throw new ConnectorException("Malformed int value on the wire: '" + val + "'", e); + } } private long decodeLong(String val) { - return Long.parseLong(val); + try { + return Long.parseLong(val); + } catch (NumberFormatException e) { + throw new ConnectorException("Malformed long value on the wire: '" + val + "'", e); + } } private Object readObjectInternal() { diff --git a/OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java b/OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java new file mode 100644 index 000000000..27067b64a --- /dev/null +++ b/OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java @@ -0,0 +1,85 @@ +/* + * 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 org.identityconnectors.framework.impl.serializer.xml; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.fail; + +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.identityconnectors.framework.common.serializer.SerializerUtil; +import org.testng.annotations.Test; + +/** + * A peer sending a corrupted numeric value on the wire must fail with a + * ConnectorException naming what failed to decode, not a bare + * NumberFormatException with no indication of where in the stream it came + * from. + */ +public class XmlObjectDecoderTest { + + @Test + public void decodesAValidInt() { + String xml = SerializerUtil.serializeXmlObject(Integer.valueOf(42), true); + assertEquals(SerializerUtil.deserializeXmlObject(xml, true), Integer.valueOf(42)); + } + + @Test + public void rejectsAMalformedInt() { + String xml = corrupt(SerializerUtil.serializeXmlObject(Integer.valueOf(42), true), "42"); + try { + SerializerUtil.deserializeXmlObject(xml, true); + fail("Expected the malformed value to be rejected"); + } catch (ConnectorException expected) { + // expected: not a bare NumberFormatException + } + } + + @Test + public void rejectsAMalformedLong() { + String xml = corrupt(SerializerUtil.serializeXmlObject(Long.valueOf(42L), true), "42"); + try { + SerializerUtil.deserializeXmlObject(xml, true); + fail("Expected the malformed value to be rejected"); + } catch (ConnectorException expected) { + // expected + } + } + + @Test + public void rejectsAMalformedDouble() { + String xml = corrupt(SerializerUtil.serializeXmlObject(Double.valueOf(4.2), true), "4.2"); + try { + SerializerUtil.deserializeXmlObject(xml, true); + fail("Expected the malformed value to be rejected"); + } catch (ConnectorException expected) { + // expected + } + } + + /** + * Replaces the encoded numeric payload of a valid wire message with a + * non-numeric string, so the rest of the document stays schema-valid and + * only the value under test is corrupted. + */ + private static String corrupt(String validXml, String encodedValue) { + String corrupted = validXml.replace(">" + encodedValue + "<", ">not-a-number<"); + if (corrupted.equals(validXml)) { + throw new IllegalStateException("Could not locate '" + encodedValue + + "' in the serialized XML to corrupt it: " + validXml); + } + return corrupted; + } +} diff --git a/OpenICF-java-framework/connector-framework-server/src/main/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfo.java b/OpenICF-java-framework/connector-framework-server/src/main/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfo.java index c53ebea0f..67fcd8968 100644 --- a/OpenICF-java-framework/connector-framework-server/src/main/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfo.java +++ b/OpenICF-java-framework/connector-framework-server/src/main/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfo.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC */ package org.forgerock.openicf.framework.client; @@ -39,6 +40,7 @@ import org.identityconnectors.common.Assertions; import org.identityconnectors.common.StringUtil; import org.identityconnectors.common.security.GuardedString; +import org.identityconnectors.framework.common.exceptions.ConnectorException; import org.identityconnectors.framework.api.RemoteFrameworkConnectionInfo; public class RemoteWSFrameworkConnectionInfo { @@ -136,7 +138,13 @@ public void loadSystemProxy() { String host = System.getProperty(PROXY_HOST); if (host != null) { proxyHost = host; - proxyPort = Integer.valueOf(System.getProperty(PROXY_PORT, "80")); + String port = System.getProperty(PROXY_PORT, "80"); + try { + proxyPort = Integer.valueOf(port); + } catch (NumberFormatException e) { + throw new ConnectorException("System property " + PROXY_PORT + + " is not a valid port number: '" + port + "'", e); + } } } diff --git a/OpenICF-java-framework/connector-framework-server/src/test/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfoTest.java b/OpenICF-java-framework/connector-framework-server/src/test/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfoTest.java new file mode 100644 index 000000000..1cfbba227 --- /dev/null +++ b/OpenICF-java-framework/connector-framework-server/src/test/java/org/forgerock/openicf/framework/client/RemoteWSFrameworkConnectionInfoTest.java @@ -0,0 +1,54 @@ +/* + * 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 org.forgerock.openicf.framework.client; + +import static org.testng.Assert.fail; + +import java.net.URI; + +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.Test; + +public class RemoteWSFrameworkConnectionInfoTest { + + @AfterMethod + public void clearProxyProperties() { + System.clearProperty(RemoteWSFrameworkConnectionInfo.PROXY_HOST); + System.clearProperty(RemoteWSFrameworkConnectionInfo.PROXY_PORT); + } + + /** + * A typo in -Dhttp.proxyPort must fail with a clear ConnectorException at + * startup rather than a bare NumberFormatException. + */ + @Test + public void loadSystemProxyRejectsAMalformedPort() { + System.setProperty(RemoteWSFrameworkConnectionInfo.PROXY_HOST, "proxy.example.com"); + System.setProperty(RemoteWSFrameworkConnectionInfo.PROXY_PORT, "not-a-number"); + + RemoteWSFrameworkConnectionInfo.Builder builder = + RemoteWSFrameworkConnectionInfo.newBuilder().setRemoteURI( + URI.create("ws://127.0.0.1:8759/openicf")); + RemoteWSFrameworkConnectionInfo info = builder.build(); + try { + info.loadSystemProxy(); + fail("Expected the malformed proxy port to be rejected"); + } catch (ConnectorException expected) { + // expected: not a bare NumberFormatException + } + } +} diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADGroupType.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADGroupType.java index 9c3f6571a..72daa25b1 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADGroupType.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADGroupType.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.ldap; @@ -144,19 +145,19 @@ public javax.naming.directory.Attribute getLdapAttribute() { // Static helpers public static boolean isScopeGlobal(String scope) { - return ((Integer.parseInt(scope) & SCOPE_GLOBAL) == SCOPE_GLOBAL); + return ((ADLdapUtil.parseADInteger(scope) & SCOPE_GLOBAL) == SCOPE_GLOBAL); } public static boolean isScopeDomainLocal(String scope) { - return ((Integer.parseInt(scope) & SCOPE_DOMAIN_LOCAL) == SCOPE_DOMAIN_LOCAL); + return ((ADLdapUtil.parseADInteger(scope) & SCOPE_DOMAIN_LOCAL) == SCOPE_DOMAIN_LOCAL); } public static boolean isScopeUniversal(String scope) { - return ((Integer.parseInt(scope) & SCOPE_UNIVERSAL) == SCOPE_UNIVERSAL); + return ((ADLdapUtil.parseADInteger(scope) & SCOPE_UNIVERSAL) == SCOPE_UNIVERSAL); } public static boolean isTypeSecurity(String type) { - return ((Integer.parseInt(type) & TYPE_SECURITY) == TYPE_SECURITY); + return ((ADLdapUtil.parseADInteger(type) & TYPE_SECURITY) == TYPE_SECURITY); } public static String getType(String type){ @@ -187,13 +188,13 @@ public static ADGroupType createADGroupType(LdapConnection conn, String id) thro NamingEnumeration entries = conn.getInitialContext().search(context, String.format("%s=%s", LdapConstants.MS_GUID_ATTR, guidStringtoByteString(id)), controls); if (entries.hasMore()) { SearchResult res = entries.next(); - int gt = Integer.parseInt(res.getAttributes().get(GROUPTYPE).get().toString()); + int gt = ADLdapUtil.parseADInteger(res.getAttributes().get(GROUPTYPE).get().toString()); return new ADGroupType(gt); } } } else if (isDNAttribute(conn.getConfiguration().getUidAttribute())) { Attributes attrs = conn.getInitialContext().getAttributes(escapeDNValueOfJNDIReservedChars(id), new String[]{GROUPTYPE}); - int gt = Integer.parseInt(attrs.get(GROUPTYPE).get().toString()); + int gt = ADLdapUtil.parseADInteger(attrs.get(GROUPTYPE).get().toString()); return new ADGroupType(gt); } throw new NamingException("Entry not found"); diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java index 0aeb6e2cd..15414e1cd 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.ldap; @@ -65,6 +66,34 @@ public class ADLdapUtil { */ public static final long DIFF_NET_JAVA_FOR_DATE_AND_TIMES = 11644473600000L; + /** + * Parses an integer-valued Active Directory attribute (userAccountControl, + * groupType, ...), which is always numeric per the AD schema, but wraps a + * malformed value in a ConnectorException naming it instead of letting a + * bare NumberFormatException escape with no indication of what failed. + */ + public static int parseADInteger(String value) { + try { + return Integer.parseInt(value); + } catch (NumberFormatException e) { + throw new ConnectorException("Not a valid Active Directory numeric value: '" + + value + "'", e); + } + } + + /** + * As {@link #parseADInteger(String)}, for the numeric AD attributes that + * do not fit in an int (such as the AD epoch time values). + */ + public static long parseADLong(String value) { + try { + return Long.parseLong(value); + } catch (NumberFormatException e) { + throw new ConnectorException("Not a valid Active Directory numeric value: '" + + value + "'", e); + } + } + static String AddLeadingZero(int k) { return (k<=0xF)?"0" + Integer.toHexString(k):Integer.toHexString(k); } @@ -301,7 +330,7 @@ public static List fetchGroupMembersByRange(LdapConnection conn, LdapEntry entry } public static Date getJavaDateFromADTime(String adTime) { - long milliseconds = (Long.parseLong(adTime) / 10000) - DIFF_NET_JAVA_FOR_DATE_AND_TIMES; + long milliseconds = (parseADLong(adTime) / 10000) - DIFF_NET_JAVA_FOR_DATE_AND_TIMES; return new Date(milliseconds); } diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADUserAccountControl.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADUserAccountControl.java index 79c144c61..e3e4bc9af 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADUserAccountControl.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADUserAccountControl.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.ldap; @@ -323,39 +324,39 @@ public BasicAttributes encodeControls(Set attrs) throws ParseExceptio // Static helpers public static boolean isAccountDisabled(String status) { - return ((Integer.parseInt(status) & ACCOUNT_DISABLED) == ACCOUNT_DISABLED); + return ((ADLdapUtil.parseADInteger(status) & ACCOUNT_DISABLED) == ACCOUNT_DISABLED); } public static boolean isAccountLockOut(String status) { - return ((Integer.parseInt(status) & LOCKOUT) == LOCKOUT); + return ((ADLdapUtil.parseADInteger(status) & LOCKOUT) == LOCKOUT); } public static boolean isPasswordNotReq(String status) { - return ((Integer.parseInt(status) & PASSWD_NOTREQD) == PASSWD_NOTREQD); + return ((ADLdapUtil.parseADInteger(status) & PASSWD_NOTREQD) == PASSWD_NOTREQD); } public static boolean isPasswordCantChange(String status) { - return ((Integer.parseInt(status) & PASSWD_CANT_CHANGE) == PASSWD_CANT_CHANGE); + return ((ADLdapUtil.parseADInteger(status) & PASSWD_CANT_CHANGE) == PASSWD_CANT_CHANGE); } public static boolean isNormalAccount(String status) { - return ((Integer.parseInt(status) & NORMAL_ACCOUNT) == NORMAL_ACCOUNT); + return ((ADLdapUtil.parseADInteger(status) & NORMAL_ACCOUNT) == NORMAL_ACCOUNT); } public static boolean isDontExpirePassword(String status) { - return ((Integer.parseInt(status) & DONT_EXPIRE_PASSWORD) == DONT_EXPIRE_PASSWORD); + return ((ADLdapUtil.parseADInteger(status) & DONT_EXPIRE_PASSWORD) == DONT_EXPIRE_PASSWORD); } public static boolean isSmartCardRequired(String status) { - return ((Integer.parseInt(status) & SMARTCARD_REQUIRED) == SMARTCARD_REQUIRED); + return ((ADLdapUtil.parseADInteger(status) & SMARTCARD_REQUIRED) == SMARTCARD_REQUIRED); } public static boolean isPasswordExpired(String status) { - return ((Integer.parseInt(status) & PASSWORD_EXPIRED) == PASSWORD_EXPIRED); + return ((ADLdapUtil.parseADInteger(status) & PASSWORD_EXPIRED) == PASSWORD_EXPIRED); } public static boolean isEncryptedTextPasswordAllowed(String status) { - return ((Integer.parseInt(status) & ENCRYPTED_TEXT_PASSWORD_ALLOWED) == ENCRYPTED_TEXT_PASSWORD_ALLOWED); + return ((ADLdapUtil.parseADInteger(status) & ENCRYPTED_TEXT_PASSWORD_ALLOWED) == ENCRYPTED_TEXT_PASSWORD_ALLOWED); } public static ADUserAccountControl createADUserAccountControl(LdapConnection conn, String id) throws NamingException { @@ -368,15 +369,15 @@ public static ADUserAccountControl createADUserAccountControl(LdapConnection con NamingEnumeration entries = conn.getInitialContext().search(context, String.format("%s=%s", LdapConstants.MS_GUID_ATTR, guidStringtoByteString(id)), controls); if (entries.hasMore()) { SearchResult res = entries.next(); - int uac = Integer.parseInt(res.getAttributes().get(MS_USR_ACCT_CTRL_ATTR).get().toString()); - int msDSUac = Integer.parseInt(res.getAttributes().get(MSDS_USR_ACCT_CTRL_ATTR).get().toString()); + int uac = ADLdapUtil.parseADInteger(res.getAttributes().get(MS_USR_ACCT_CTRL_ATTR).get().toString()); + int msDSUac = ADLdapUtil.parseADInteger(res.getAttributes().get(MSDS_USR_ACCT_CTRL_ATTR).get().toString()); return new ADUserAccountControl(uac, msDSUac); } } } else if (isDNAttribute(conn.getConfiguration().getUidAttribute())) { Attributes attrs = conn.getInitialContext().getAttributes(escapeDNValueOfJNDIReservedChars(id), new String[]{MSDS_USR_ACCT_CTRL_ATTR, MS_USR_ACCT_CTRL_ATTR}); - int uac = Integer.parseInt(attrs.get(MS_USR_ACCT_CTRL_ATTR).get().toString()); - int msDSUac = Integer.parseInt(attrs.get(MSDS_USR_ACCT_CTRL_ATTR).get().toString()); + int uac = ADLdapUtil.parseADInteger(attrs.get(MS_USR_ACCT_CTRL_ATTR).get().toString()); + int msDSUac = ADLdapUtil.parseADInteger(attrs.get(MSDS_USR_ACCT_CTRL_ATTR).get().toString()); return new ADUserAccountControl(uac, msDSUac); } throw new NamingException("Entry not found"); diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java index 012e554f8..05962cadb 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java @@ -16,6 +16,7 @@ * applicable, add the following below the CDDL Header, with the fields enclosed * by brackets [] replaced by your own identifying information: " Portions * Copyrighted [year] [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC * */ package org.identityconnectors.ldap.search; @@ -94,10 +95,10 @@ public void doSearch(LdapContext initCtx, List baseDNs, String query, Se if (split.length == 2) { try { cookie = Base64.decode(split[0]); + context = Integer.parseInt(split[1]); } catch (RuntimeException e) { throw new ConnectorException("PagedResultsCookie is not properly encoded", e); } - context = Integer.valueOf(split[1]); } else { throw new ConnectorException("PagedResultsCookie is not properly formatted"); } diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java index b9b6ccdd4..f8c097059 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC */ package org.identityconnectors.ldap.sync.activedirectory; @@ -118,7 +119,7 @@ public void sync(SyncToken token, final SyncResultsHandler handler, final Operat String waterMark = gethighestCommittedUSN(); if (token != null && logger.isWarning()) { - if (Integer.parseInt(token.getValue().toString()) > Integer.parseInt(waterMark)) { + if (ADLdapUtil.parseADInteger(token.getValue().toString()) > ADLdapUtil.parseADInteger(waterMark)) { //[OPENICF-402] The current SyncToken should never be greater than the highestCommittedUSN on the DC // We log the issue and let the process go logger.warn("The current SyncToken value ({0}) is greater than the highestCommittedUSN value ({1})", token.getValue().toString(), waterMark); @@ -252,7 +253,7 @@ public boolean handle(String baseDN, SearchResult result) throws NamingException syncDeltaBuilder.setUid(uid); syncDeltaBuilder.setObject(cob.build()); - changes.put(Integer.parseInt(usnChanged[0]), syncDeltaBuilder.build()); + changes.put(ADLdapUtil.parseADInteger(usnChanged[0]), syncDeltaBuilder.build()); return true; } }); @@ -291,7 +292,7 @@ public boolean handle(String baseDN, SearchResult result) throws NamingException } else { syncDeltaBuilder.setObjectClass(oclass); } - changes.put(Integer.parseInt(usnChanged[0]), syncDeltaBuilder.build()); + changes.put(ADLdapUtil.parseADInteger(usnChanged[0]), syncDeltaBuilder.build()); } } else if (LdapConstants.ServerType.MSAD_LDS.equals(conn.getServerType())) { logger.error("Active Directory Lightweight Directory Services is used but defaultNamingContext has not been set - impossible to detect deleted objects"); @@ -339,7 +340,7 @@ private String generateUSNChangedFilter(ObjectClass oc, SyncToken token, boolean } filter.append("(uSNChanged>="); - filter.append(Integer.parseInt(token.getValue().toString()) + 1); + filter.append(ADLdapUtil.parseADInteger(token.getValue().toString()) + 1); filter.append(")"); if (isDeleted) { diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADGroupTypeTest.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADGroupTypeTest.java new file mode 100644 index 000000000..67e94936b --- /dev/null +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADGroupTypeTest.java @@ -0,0 +1,36 @@ +/* + * 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 org.identityconnectors.ldap; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.testng.annotations.Test; + +public class ADGroupTypeTest { + + @Test + public void isScopeGlobalReadsTheBitmask() { + assertTrue(ADGroupType.isScopeGlobal(String.valueOf(ADGroupType.SCOPE_GLOBAL))); + assertFalse(ADGroupType.isScopeGlobal(String.valueOf(ADGroupType.SCOPE_UNIVERSAL))); + } + + @Test(expectedExceptions = ConnectorException.class) + public void isScopeGlobalRejectsAMalformedValue() { + ADGroupType.isScopeGlobal("not-a-number"); + } +} diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java new file mode 100644 index 000000000..977161a34 --- /dev/null +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java @@ -0,0 +1,56 @@ +/* + * 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 org.identityconnectors.ldap; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.testng.annotations.Test; + +public class ADLdapUtilTest { + + @Test + public void parseADIntegerParsesAValidValue() { + assertEquals(ADLdapUtil.parseADInteger("512"), 512); + } + + @Test(expectedExceptions = ConnectorException.class) + public void parseADIntegerWrapsAMalformedValue() { + ADLdapUtil.parseADInteger("not-a-number"); + } + + @Test + public void parseADIntegerErrorMessageNamesTheValue() { + try { + ADLdapUtil.parseADInteger("not-a-number"); + } catch (ConnectorException e) { + assertTrue(e.getMessage().contains("not-a-number"), e.getMessage()); + return; + } + throw new AssertionError("Expected a ConnectorException"); + } + + @Test + public void parseADLongParsesAValidValue() { + assertEquals(ADLdapUtil.parseADLong("131425440000000000"), 131425440000000000L); + } + + @Test(expectedExceptions = ConnectorException.class) + public void parseADLongWrapsAMalformedValue() { + ADLdapUtil.parseADLong("not-a-number"); + } +} diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADUserAccountControlTest.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADUserAccountControlTest.java new file mode 100644 index 000000000..91c6855b1 --- /dev/null +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADUserAccountControlTest.java @@ -0,0 +1,43 @@ +/* + * 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 org.identityconnectors.ldap; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.testng.annotations.Test; + +public class ADUserAccountControlTest { + + /** + * userAccountControl bit 0x0002 is ACCOUNTDISABLE. + */ + @Test + public void isAccountDisabledReadsTheBitmask() { + assertTrue(ADUserAccountControl.isAccountDisabled("2")); + assertFalse(ADUserAccountControl.isAccountDisabled("512")); + } + + /** + * A corrupted userAccountControl value must fail with a ConnectorException + * naming it, not a bare NumberFormatException. + */ + @Test(expectedExceptions = ConnectorException.class) + public void isAccountDisabledRejectsAMalformedValue() { + ADUserAccountControl.isAccountDisabled("not-a-number"); + } +} diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java new file mode 100644 index 000000000..99256651f --- /dev/null +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java @@ -0,0 +1,37 @@ +/* + * 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 org.identityconnectors.ldap.search; + +import java.util.Collections; + +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.testng.annotations.Test; + +/** + * The paged-results cookie is round-tripped through the client between + * search calls, so a tampered or corrupted one must fail clearly rather than + * with a bare NumberFormatException. + */ +public class PagedSearchStrategyTest { + + @Test(expectedExceptions = ConnectorException.class) + public void rejectsACookieWithAMalformedContextIndex() throws Exception { + PagedSearchStrategy strategy = new PagedSearchStrategy(10, "AAAA:not-a-number", 0, null, + new org.identityconnectors.framework.common.objects.SortKey[0]); + strategy.doSearch(null, Collections. singletonList("dc=example,dc=com"), "(uid=*)", + new javax.naming.directory.SearchControls(), null); + } +} diff --git a/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java b/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java index 2e6dd3877..ca21f9269 100644 --- a/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java +++ b/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC */ package org.forgerock.openicf.maven; @@ -137,6 +138,7 @@ private Object castValue(String name, PlexusConfiguration value, Class target + "to " + targetType); } + try { if (targetType.equals(Long.class)) { if (StringUtil.isNotBlank(sourceValue)) { targetValue = Long.valueOf(sourceValue); @@ -173,7 +175,12 @@ private Object castValue(String name, PlexusConfiguration value, Class target } } else if (targetType.equals(Boolean.TYPE)) { targetValue = Boolean.valueOf(sourceValue); - } else if (targetType.equals(URI.class)) { + } + } catch (NumberFormatException e) { + throw new MojoExecutionException("Failed to convert value '" + sourceValue + + "' of " + name + " to " + targetType, e); + } + if (targetType.equals(URI.class)) { try { targetValue = new URI(sourceValue); } catch (URISyntaxException e) { From 23c4bdf3a603774beab1ceaac0ad8e9f088a53ef Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 2 Oct 2026 21:54:35 +0300 Subject: [PATCH 2/2] Address review: keep one cast chain, guard empty wire values, parse USNs as long PropertyBag.castValue goes back to a single if/else-if chain inside the try, so a converted numeric, char or boolean property no longer falls through to the "Cast to targetType is not supported" warning. The five numeric XmlObjectDecoder decoders reject an empty element with a ConnectorException instead of an NPE (Double/Float/Byte) or a 'null' message (Integer/Long). PagedSearchStrategy rejects a cookie whose context index is outside baseDNs. ActiveDirectoryChangeLogSyncStrategy parses uSNChanged/highestCommittedUSN as long, since they are 64-bit in AD, and orders changes in a TreeMap. Tests now pin the exact message and the NumberFormatException cause, cover the float and byte decoders, empty elements, out-of-range cookie indexes, a USN past the int range and getJavaDateFromADTime. --- .../impl/serializer/xml/XmlObjectDecoder.java | 15 +++ .../serializer/xml/XmlObjectDecoderTest.java | 94 +++++++++++---- .../identityconnectors/ldap/ADLdapUtil.java | 3 +- .../ldap/search/PagedSearchStrategy.java | 3 + .../ActiveDirectoryChangeLogSyncStrategy.java | 12 +- .../ldap/ADLdapUtilTest.java | 25 +++- .../ldap/search/PagedSearchStrategyTest.java | 16 ++- .../forgerock/openicf/maven/PropertyBag.java | 107 +++++++++--------- 8 files changed, 192 insertions(+), 83 deletions(-) diff --git a/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java b/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java index 9b19c7cbe..053d90c17 100644 --- a/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java +++ b/OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoder.java @@ -181,6 +181,9 @@ private boolean decodeBoolean(String v) { } private byte decodeByte(String v) { + if (v == null) { + throw new ConnectorException("Malformed byte value on the wire: empty element"); + } try { return Byte.decode(v); } catch (NumberFormatException e) { @@ -209,6 +212,9 @@ private Class decodeClass(String type) { } private double decodeDouble(String val) { + if (val == null) { + throw new ConnectorException("Malformed double value on the wire: empty element"); + } try { return Double.parseDouble(val); } catch (NumberFormatException e) { @@ -217,6 +223,9 @@ private double decodeDouble(String val) { } private float decodeFloat(String val) { + if (val == null) { + throw new ConnectorException("Malformed float value on the wire: empty element"); + } try { return Float.parseFloat(val); } catch (NumberFormatException e) { @@ -225,6 +234,9 @@ private float decodeFloat(String val) { } private int decodeInt(String val) { + if (val == null) { + throw new ConnectorException("Malformed int value on the wire: empty element"); + } try { return Integer.parseInt(val); } catch (NumberFormatException e) { @@ -233,6 +245,9 @@ private int decodeInt(String val) { } private long decodeLong(String val) { + if (val == null) { + throw new ConnectorException("Malformed long value on the wire: empty element"); + } try { return Long.parseLong(val); } catch (NumberFormatException e) { diff --git a/OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java b/OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java index 27067b64a..e3aac8227 100644 --- a/OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java +++ b/OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/serializer/xml/XmlObjectDecoderTest.java @@ -16,6 +16,7 @@ package org.identityconnectors.framework.impl.serializer.xml; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; import org.identityconnectors.framework.common.exceptions.ConnectorException; @@ -38,34 +39,75 @@ public void decodesAValidInt() { @Test public void rejectsAMalformedInt() { - String xml = corrupt(SerializerUtil.serializeXmlObject(Integer.valueOf(42), true), "42"); - try { - SerializerUtil.deserializeXmlObject(xml, true); - fail("Expected the malformed value to be rejected"); - } catch (ConnectorException expected) { - // expected: not a bare NumberFormatException - } + assertRejected(corrupt(SerializerUtil.serializeXmlObject(Integer.valueOf(42), true), "42"), "int"); } @Test public void rejectsAMalformedLong() { - String xml = corrupt(SerializerUtil.serializeXmlObject(Long.valueOf(42L), true), "42"); + assertRejected(corrupt(SerializerUtil.serializeXmlObject(Long.valueOf(42L), true), "42"), "long"); + } + + @Test + public void rejectsAMalformedDouble() { + assertRejected(corrupt(SerializerUtil.serializeXmlObject(Double.valueOf(4.2), true), "4.2"), "double"); + } + + @Test + public void rejectsAMalformedFloat() { + assertRejected(corrupt(SerializerUtil.serializeXmlObject(Float.valueOf(4.5f), true), "4.5"), "float"); + } + + @Test + public void rejectsAMalformedByte() { + assertRejected(corrupt(SerializerUtil.serializeXmlObject(Byte.valueOf((byte) 42), true), "42"), "byte"); + } + + /** + * An element with no text node decodes to null, which Double, Float and + * Byte parsing reject with a NullPointerException rather than a + * NumberFormatException. + */ + @Test + public void rejectsAnEmptyDouble() { + assertRejectedEmpty(empty(SerializerUtil.serializeXmlObject(Double.valueOf(4.2), true), "4.2"), "double"); + } + + @Test + public void rejectsAnEmptyFloat() { + assertRejectedEmpty(empty(SerializerUtil.serializeXmlObject(Float.valueOf(4.5f), true), "4.5"), "float"); + } + + @Test + public void rejectsAnEmptyByte() { + assertRejectedEmpty(empty(SerializerUtil.serializeXmlObject(Byte.valueOf((byte) 42), true), "42"), "byte"); + } + + @Test + public void rejectsAnEmptyInt() { + assertRejectedEmpty(empty(SerializerUtil.serializeXmlObject(Integer.valueOf(42), true), "42"), "int"); + } + + @Test + public void rejectsAnEmptyLong() { + assertRejectedEmpty(empty(SerializerUtil.serializeXmlObject(Long.valueOf(42L), true), "42"), "long"); + } + + private static void assertRejected(String xml, String type) { try { SerializerUtil.deserializeXmlObject(xml, true); fail("Expected the malformed value to be rejected"); - } catch (ConnectorException expected) { - // expected + } catch (ConnectorException e) { + assertEquals(e.getMessage(), "Malformed " + type + " value on the wire: 'not-a-number'"); + assertTrue(e.getCause() instanceof NumberFormatException, String.valueOf(e.getCause())); } } - @Test - public void rejectsAMalformedDouble() { - String xml = corrupt(SerializerUtil.serializeXmlObject(Double.valueOf(4.2), true), "4.2"); + private static void assertRejectedEmpty(String xml, String type) { try { SerializerUtil.deserializeXmlObject(xml, true); - fail("Expected the malformed value to be rejected"); - } catch (ConnectorException expected) { - // expected + fail("Expected the empty value to be rejected"); + } catch (ConnectorException e) { + assertEquals(e.getMessage(), "Malformed " + type + " value on the wire: empty element"); } } @@ -75,11 +117,23 @@ public void rejectsAMalformedDouble() { * only the value under test is corrupted. */ private static String corrupt(String validXml, String encodedValue) { - String corrupted = validXml.replace(">" + encodedValue + "<", ">not-a-number<"); - if (corrupted.equals(validXml)) { + return replaceValue(validXml, encodedValue, "not-a-number"); + } + + /** + * Removes the encoded numeric payload of a valid wire message, leaving an + * element with no text node, which the DTD's #PCDATA content allows. + */ + private static String empty(String validXml, String encodedValue) { + return replaceValue(validXml, encodedValue, ""); + } + + private static String replaceValue(String validXml, String encodedValue, String replacement) { + String changed = validXml.replace(">" + encodedValue + "<", ">" + replacement + "<"); + if (changed.equals(validXml)) { throw new IllegalStateException("Could not locate '" + encodedValue - + "' in the serialized XML to corrupt it: " + validXml); + + "' in the serialized XML to replace it: " + validXml); } - return corrupted; + return changed; } } diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java index 15414e1cd..2a3e6fa95 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/ADLdapUtil.java @@ -83,7 +83,8 @@ public static int parseADInteger(String value) { /** * As {@link #parseADInteger(String)}, for the numeric AD attributes that - * do not fit in an int (such as the AD epoch time values). + * do not fit in an int (such as the AD epoch time values and the 64-bit + * update sequence numbers uSNChanged and highestCommittedUSN). */ public static long parseADLong(String value) { try { diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java index 05962cadb..1c63001fa 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/search/PagedSearchStrategy.java @@ -99,6 +99,9 @@ public void doSearch(LdapContext initCtx, List baseDNs, String query, Se } catch (RuntimeException e) { throw new ConnectorException("PagedResultsCookie is not properly encoded", e); } + if (context < 0 || context >= baseDNs.size()) { + throw new ConnectorException("PagedResultsCookie is not properly encoded"); + } } else { throw new ConnectorException("PagedResultsCookie is not properly formatted"); } diff --git a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java index f8c097059..fd1cb2c07 100644 --- a/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java +++ b/OpenICF-ldap-connector/src/main/java/org/identityconnectors/ldap/sync/activedirectory/ActiveDirectoryChangeLogSyncStrategy.java @@ -114,12 +114,12 @@ public void sync(SyncToken token, final SyncResultsHandler handler, final Operat // We have to detect deleted entries as well. To do so, we use the filter (isDeleted==TRUE) to detect // the tombstones in the cn=delete objects, container. - final TreeMap changes = new TreeMap(); + final TreeMap changes = new TreeMap(); final String[] usnChanged = {""}; String waterMark = gethighestCommittedUSN(); if (token != null && logger.isWarning()) { - if (ADLdapUtil.parseADInteger(token.getValue().toString()) > ADLdapUtil.parseADInteger(waterMark)) { + if (ADLdapUtil.parseADLong(token.getValue().toString()) > ADLdapUtil.parseADLong(waterMark)) { //[OPENICF-402] The current SyncToken should never be greater than the highestCommittedUSN on the DC // We log the issue and let the process go logger.warn("The current SyncToken value ({0}) is greater than the highestCommittedUSN value ({1})", token.getValue().toString(), waterMark); @@ -253,7 +253,7 @@ public boolean handle(String baseDN, SearchResult result) throws NamingException syncDeltaBuilder.setUid(uid); syncDeltaBuilder.setObject(cob.build()); - changes.put(ADLdapUtil.parseADInteger(usnChanged[0]), syncDeltaBuilder.build()); + changes.put(ADLdapUtil.parseADLong(usnChanged[0]), syncDeltaBuilder.build()); return true; } }); @@ -292,7 +292,7 @@ public boolean handle(String baseDN, SearchResult result) throws NamingException } else { syncDeltaBuilder.setObjectClass(oclass); } - changes.put(ADLdapUtil.parseADInteger(usnChanged[0]), syncDeltaBuilder.build()); + changes.put(ADLdapUtil.parseADLong(usnChanged[0]), syncDeltaBuilder.build()); } } else if (LdapConstants.ServerType.MSAD_LDS.equals(conn.getServerType())) { logger.error("Active Directory Lightweight Directory Services is used but defaultNamingContext has not been set - impossible to detect deleted objects"); @@ -304,7 +304,7 @@ public boolean handle(String baseDN, SearchResult result) throws NamingException logger.info("The server does not support the control to search for deleted entries"); } // Changes are now ordered in the TreeMap according to usnChanged. - for (Map.Entry entry : changes.entrySet()) { + for (Map.Entry entry : changes.entrySet()) { if (!handler.handle(entry.getValue())) { break; } else { @@ -340,7 +340,7 @@ private String generateUSNChangedFilter(ObjectClass oc, SyncToken token, boolean } filter.append("(uSNChanged>="); - filter.append(ADLdapUtil.parseADInteger(token.getValue().toString()) + 1); + filter.append(ADLdapUtil.parseADLong(token.getValue().toString()) + 1); filter.append(")"); if (isDeleted) { diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java index 977161a34..7aed460d4 100644 --- a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/ADLdapUtilTest.java @@ -38,7 +38,8 @@ public void parseADIntegerErrorMessageNamesTheValue() { try { ADLdapUtil.parseADInteger("not-a-number"); } catch (ConnectorException e) { - assertTrue(e.getMessage().contains("not-a-number"), e.getMessage()); + assertEquals(e.getMessage(), "Not a valid Active Directory numeric value: 'not-a-number'"); + assertTrue(e.getCause() instanceof NumberFormatException, String.valueOf(e.getCause())); return; } throw new AssertionError("Expected a ConnectorException"); @@ -49,8 +50,30 @@ public void parseADLongParsesAValidValue() { assertEquals(ADLdapUtil.parseADLong("131425440000000000"), 131425440000000000L); } + @Test + public void parseADLongParsesAUSNPastTheIntRange() { + assertEquals(ADLdapUtil.parseADLong("2147483648"), 2147483648L); + } + @Test(expectedExceptions = ConnectorException.class) public void parseADLongWrapsAMalformedValue() { ADLdapUtil.parseADLong("not-a-number"); } + + @Test + public void parseADLongErrorMessageNamesTheValue() { + try { + ADLdapUtil.parseADLong("not-a-number"); + } catch (ConnectorException e) { + assertEquals(e.getMessage(), "Not a valid Active Directory numeric value: 'not-a-number'"); + assertTrue(e.getCause() instanceof NumberFormatException, String.valueOf(e.getCause())); + return; + } + throw new AssertionError("Expected a ConnectorException"); + } + + @Test(expectedExceptions = ConnectorException.class) + public void getJavaDateFromADTimeWrapsAMalformedValue() { + ADLdapUtil.getJavaDateFromADTime("not-a-number"); + } } diff --git a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java index 99256651f..cce309681 100644 --- a/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java +++ b/OpenICF-ldap-connector/src/test/java/org/identityconnectors/ldap/search/PagedSearchStrategyTest.java @@ -29,7 +29,21 @@ public class PagedSearchStrategyTest { @Test(expectedExceptions = ConnectorException.class) public void rejectsACookieWithAMalformedContextIndex() throws Exception { - PagedSearchStrategy strategy = new PagedSearchStrategy(10, "AAAA:not-a-number", 0, null, + search("AAAA:not-a-number"); + } + + @Test(expectedExceptions = ConnectorException.class) + public void rejectsACookieWithAContextIndexPastTheBaseDNs() throws Exception { + search("AAAA:5"); + } + + @Test(expectedExceptions = ConnectorException.class) + public void rejectsACookieWithANegativeContextIndex() throws Exception { + search("AAAA:-1"); + } + + private static void search(String pagedResultsCookie) throws Exception { + PagedSearchStrategy strategy = new PagedSearchStrategy(10, pagedResultsCookie, 0, null, new org.identityconnectors.framework.common.objects.SortKey[0]); strategy.doSearch(null, Collections. singletonList("dc=example,dc=com"), "(uid=*)", new javax.naming.directory.SearchControls(), null); diff --git a/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java b/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java index ca21f9269..e89a25001 100644 --- a/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java +++ b/OpenICF-maven-plugin/src/main/java/org/forgerock/openicf/maven/PropertyBag.java @@ -139,71 +139,70 @@ private Object castValue(String name, PlexusConfiguration value, Class target } try { - if (targetType.equals(Long.class)) { - if (StringUtil.isNotBlank(sourceValue)) { + if (targetType.equals(Long.class)) { + if (StringUtil.isNotBlank(sourceValue)) { + targetValue = Long.valueOf(sourceValue); + } + } else if (targetType.equals(Long.TYPE)) { targetValue = Long.valueOf(sourceValue); - } - } else if (targetType.equals(Long.TYPE)) { - targetValue = Long.valueOf(sourceValue); - } else if (targetType.equals(Character.class)) { - if (StringUtil.isNotBlank(sourceValue)) { + } else if (targetType.equals(Character.class)) { + if (StringUtil.isNotBlank(sourceValue)) { + targetValue = sourceValue.charAt(0); + } + } else if (targetType.equals(Character.TYPE)) { targetValue = sourceValue.charAt(0); - } - } else if (targetType.equals(Character.TYPE)) { - targetValue = sourceValue.charAt(0); - } else if (targetType.equals(Double.class)) { - if (StringUtil.isNotBlank(sourceValue)) { + } else if (targetType.equals(Double.class)) { + if (StringUtil.isNotBlank(sourceValue)) { + targetValue = Double.valueOf(sourceValue); + } + } else if (targetType.equals(Double.TYPE)) { targetValue = Double.valueOf(sourceValue); - } - } else if (targetType.equals(Double.TYPE)) { - targetValue = Double.valueOf(sourceValue); - } else if (targetType.equals(Float.class)) { - if (StringUtil.isNotBlank(sourceValue)) { + } else if (targetType.equals(Float.class)) { + if (StringUtil.isNotBlank(sourceValue)) { + targetValue = Float.valueOf(sourceValue); + } + } else if (targetType.equals(Float.TYPE)) { targetValue = Float.valueOf(sourceValue); - } - } else if (targetType.equals(Float.TYPE)) { - targetValue = Float.valueOf(sourceValue); - } else if (targetType.equals(Integer.class)) { - if (StringUtil.isNotBlank(sourceValue)) { + } else if (targetType.equals(Integer.class)) { + if (StringUtil.isNotBlank(sourceValue)) { + targetValue = Integer.valueOf(sourceValue); + } + } else if (targetType.equals(Integer.TYPE)) { targetValue = Integer.valueOf(sourceValue); - } - } else if (targetType.equals(Integer.TYPE)) { - targetValue = Integer.valueOf(sourceValue); - } else if (targetType.equals(Boolean.class)) { - if (StringUtil.isNotBlank(sourceValue)) { + } else if (targetType.equals(Boolean.class)) { + if (StringUtil.isNotBlank(sourceValue)) { + targetValue = Boolean.valueOf(sourceValue); + } + } else if (targetType.equals(Boolean.TYPE)) { targetValue = Boolean.valueOf(sourceValue); + } else if (targetType.equals(URI.class)) { + try { + targetValue = new URI(sourceValue); + } catch (URISyntaxException e) { + throw new MojoExecutionException(e.getMessage(), e); + } + } else if (targetType.equals(File.class)) { + targetValue = new File(sourceValue); + } else if (targetType.equals(GuardedByteArray.class)) { + targetValue = new GuardedByteArray(sourceValue.getBytes()); + } else if (targetType.equals(GuardedString.class)) { + targetValue = new GuardedString(sourceValue.toCharArray()); + } else if (targetType.equals(Script.class)) { + String scriptLanguage = value.getAttribute("scriptLanguage"); + if (StringUtil.isBlank(scriptLanguage)) { + throw new MojoExecutionException( + "Missing argument 'scriptLanguage' for Script attribute: " + name); + } + targetValue = + new ScriptBuilder().setScriptLanguage(scriptLanguage) + .setScriptText(sourceValue).build(); + } else { + log.warn("Cast to targetType: '" + targetType + "' is not supported"); } - } else if (targetType.equals(Boolean.TYPE)) { - targetValue = Boolean.valueOf(sourceValue); - } } catch (NumberFormatException e) { throw new MojoExecutionException("Failed to convert value '" + sourceValue + "' of " + name + " to " + targetType, e); } - if (targetType.equals(URI.class)) { - try { - targetValue = new URI(sourceValue); - } catch (URISyntaxException e) { - throw new MojoExecutionException(e.getMessage(), e); - } - } else if (targetType.equals(File.class)) { - targetValue = new File(sourceValue); - } else if (targetType.equals(GuardedByteArray.class)) { - targetValue = new GuardedByteArray(sourceValue.getBytes()); - } else if (targetType.equals(GuardedString.class)) { - targetValue = new GuardedString(sourceValue.toCharArray()); - } else if (targetType.equals(Script.class)) { - String scriptLanguage = value.getAttribute("scriptLanguage"); - if (StringUtil.isBlank(scriptLanguage)) { - throw new MojoExecutionException( - "Missing argument 'scriptLanguage' for Script attribute: " + name); - } - targetValue = - new ScriptBuilder().setScriptLanguage(scriptLanguage) - .setScriptText(sourceValue).build(); - } else { - log.warn("Cast to targetType: '" + targetType + "' is not supported"); - } return targetValue; }