[vharseko]

Summary

#134 replaced Class.newInstance() with ReflectionUtil.newInstance(), which goes through MethodHandles.Lookup.findConstructor. That changed how a connector configuration class with a missing dependency fails:

• before: Class.newInstance() threw NoClassDefFoundError. The catch (Exception) in createDefaultAPIConfiguration let it through, and LocalConnectorInfoManagerImpl (catch (NoClassDefFoundError), likewise the OSGi managers) skipped that connector and logged an error;
• now: findConstructor reports the same NoClassDefFoundError as IllegalAccessException: no such constructor. That is an Exception, so it is wrapped in a ConnectorException that nobody catches, and the whole bundle scan fails.

Found through OpenIdentityPlatform/OpenIDM#259 (bump to openicf 2.1.0-SNAPSHOT): ScriptedCRESTConfiguration in groovy-connector refers to httpasyncclient, which is not on the OpenIDM test classpath, and ConnectorInfoProviderServiceTest.beforeTest fails with LocalManager initialisation failed; 126 tests of the module are skipped.

Caused by: ConnectorException: java.lang.IllegalAccessException: no such constructor: org.forgerock.openicf.connectors.scriptedcrest.ScriptedCRESTConfiguration.<init>()v
  at LocalConnectorInfoManagerImpl.createDefaultAPIConfiguration(LocalConnectorInfoManagerImpl.java:339)
Caused by: java.lang.NoClassDefFoundError: org/apache/http/nio/client/HttpAsyncClient

Fix

ReflectionUtil.newInstance rethrows the LinkageError cause of the findConstructor failure, except for the NoSuchMethodError and IllegalAccessError that come with a missing or inaccessible constructor (those are still reported as NoSuchMethodException / IllegalAccessException). This brings back the Class.newInstance() behaviour for every caller, including the OSGi info managers. One case stays different: a link failure that is itself an IllegalAccessError (a dependency that cannot access its superclass) cannot be told from an inaccessible constructor, so it is still reported as IllegalAccessException. Class.newInstance() threw the raw IllegalAccessError. The managers catch neither, so the scan fails in both cases.

Test plan

• New ReflectionUtilTests.newInstanceThrowsLinkageErrorOfClassWithMissingDependency: a class loader that cannot load one dependency, so the class fails to link. Before the fix the test fails with the same IllegalAccessException: no such constructor
• New ReflectionUtilTests.newInstanceFailsOnInaccessibleConstructor: a private constructor seen through publicLookup() still fails with IllegalAccessException, not with a raw IllegalAccessError
• New ReflectionUtilTests.newInstanceFailsOnInaccessibleClass: a package-private class seen through publicLookup(), which findConstructor reports without a cause, still fails with IllegalAccessException, not with a NullPointerException
• New LocalConnectorInfoManagerTests.testSkipsConnectorWithUnlinkableConfiguration: a bundle with two connectors, the configuration class of one refers to a class missing from the bundle; the scan lists the other connector. Without the fix it fails with the IllegalAccessException: no such constructor from OpenIDM#259; without the catch (NoClassDefFoundError) in LocalConnectorInfoManagerImpl it fails with the NoClassDefFoundError. The test closes the bundle class loader before it deletes the temp jar, which Windows cannot delete while it is open
• connector-framework: 205/205 pass (incl. newInstanceFailsWithoutNoArgConstructor, which still gets NoSuchMethodException); connector-framework-internal: 525 run, 0 failures, 2 skipped (testConfigurationUpdate of the remote manager tests)
• End to end: with this connector-framework installed locally, OpenIDM openidm-provisioner-openicf on openicf 2.1.0-SNAPSHOT passes 138/138 (same as on 2.0.4; without the fix beforeTest fails)

[maximthomas]

praise: The fix sits at the one choke point every caller shares, and it keeps the existing contract for missing and inaccessible constructors.

• ReflectionUtil.newInstance (ReflectionUtil.java:162-172) unwraps the LinkageError cause of the findConstructor failure, so every caller gets the link failure back without a change on its side.
• The NoSuchMethodError exclusion keeps newInstanceFailsWithoutNoArgConstructor on NoSuchMethodException.
• newInstanceThrowsLinkageErrorOfClassWithMissingDependency is red at the base (IllegalAccessException: no such constructor ... LinksAgainstHidden.<init>()void) and green at the head.

---

suggestion (non-blocking): No test reaches the !(cause instanceof IllegalAccessError) exclusion.

OpenICF-java-framework/connector-framework/src/main/java/org/identityconnectors/common/ReflectionUtil.java:168-169

Every ReflectionUtilTests fixture is a public nested class created through the test's own MethodHandles.lookup(), a nestmate that resolves even a private constructor. A mutant that drops && !(cause instanceof IllegalAccessError) makes a private constructor seen through a non-nestmate lookup throw a raw IllegalAccessError instead of IllegalAccessException, and the catch (Exception) callers let that Error escape. All 7 ReflectionUtilTests stay green against it on JDK 11.0.25, 17.0.11, 21.0.2 and 26. The case below fails against the mutant and passes at the head on the same four JDKs.

@Test(expectedExceptions = IllegalAccessException.class)
public void newInstanceFailsOnInaccessibleConstructor() throws Exception {
    // publicLookup() is no nestmate: the private constructor fails the access
    // check with an IllegalAccessError cause
    newInstance(MethodHandles.publicLookup(), PrivateConstructor.class);
}

public static class PrivateConstructor {
    private PrivateConstructor() {
    }
}

Pin: at the head it throws IllegalAccessException (cause IllegalAccessError); against the mutant it throws the raw IllegalAccessError.

---

suggestion (non-blocking): contains("Hidden") also matches the class under test, so the assertion does not check that the error names the missing dependency.

OpenICF-java-framework/connector-framework/src/test/java/org/identityconnectors/common/ReflectionUtilTests.java:107

The missing dependency is org/identityconnectors/common/ReflectionUtilTests$Hidden, but ReflectionUtilTests$LinksAgainstHidden contains "Hidden" too. A mutant that throws new NoClassDefFoundError(clazz.getName()) instead of rethrowing the cause passes the current assertion on JDK 11, 17, 21 and 26, and the error the info managers log when they skip the connector would no longer name the missing class. The line below fails against that mutant and passes at the head.

assertTrue(e.getMessage().contains(Hidden.class.getName().replace('.', '/')), e.getMessage());

---

suggestion (non-blocking): The per-connector skip on NoClassDefFoundError, which is the outcome this PR restores, is pinned only at the ReflectionUtil level.

OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/api/local/LocalConnectorInfoManagerImpl.java:303, OpenICF-java-framework/connector-framework-osgi/src/main/java/org/forgerock/openicf/framework/impl/api/osgi/internal/OsgiConnectorInfoManagerImpl.java:320, OpenICF-java-framework/connector-framework-server/src/main/java/org/forgerock/openicf/framework/osgi/internal/AsyncOsgiConnectorInfoManagerImpl.java:220

No test reaches these three catch (final NoClassDefFoundError e) blocks: every configuration class in testbundlev1/v2/v10/v11 links. If createDefaultAPIConfiguration's catch (Exception e) were widened to Exception | Error, or one manager's NoClassDefFoundError catch were dropped, the whole-scan failure from OpenIDM#259 would come back with CI still green. Right now the only end-to-end check is a manual OpenIDM run. This mutant was traced by reading, not run.

Pin: a LocalConnectorInfoManagerTests case with a bundle of two connectors, one whose configuration class refers to a class missing from the bundle; assert that the scan throws nothing and the manager lists the other connector. Widening createDefaultAPIConfiguration to catch Exception | Error turns it red.

---

note (non-blocking): Text that does not match the change.

• PR description (the squash commit message): "brings back the Class.newInstance() behaviour for all eight callers". The fix is inside ReflectionUtil.newInstance, so it reaches every caller, but git grep 'ReflectionUtil.newInstance(' over src/main at the head finds 15-16 production call sites in 13-14 files. Saying "every caller" would be accurate.

[vharseko]

@maximthomas Added all three tests in 20b77d4 and fixed the description ("every caller"); the branch is rebased onto the current master.

• newInstanceFailsOnInaccessibleConstructor (publicLookup() and a private constructor) fails against the mutant without the IllegalAccessError exclusion.
• The missing-dependency test now checks for Hidden.class.getName().replace('.', '/') and fails against new NoClassDefFoundError(clazz.getName()).
• LocalConnectorInfoManagerTests.testSkipsConnectorWithUnlinkableConfiguration builds a bundle with two connectors; the configuration class of one refers to a class that is neither in the bundle nor visible through the parent class loader. The scan lists the other connector. It fails if the fix in ReflectionUtil is reverted (the IllegalAccessException: no such constructor from OpenIDM#259) and if the catch (NoClassDefFoundError) in LocalConnectorInfoManagerImpl is dropped.

One note on the third point: widening createDefaultAPIConfiguration to catch (Exception | Error e) turns out to be an equivalent mutant, because ConnectorException.wrap rethrows any Error unchanged (ConnectorException.java:94-96), so the new test stays green against it by design. The OSGi managers are still not covered by this test.

[maximthomas]

praise: the fix sits in the one place every caller goes through, and the new tests pin it end to end.

• ReflectionUtil.newInstance (:167-171) rethrows the LinkageError cause, and the NoSuchMethodError / IllegalAccessError exclusions keep NoSuchMethodException and IllegalAccessException for a missing or inaccessible constructor.
• LocalConnectorInfoManagerTests.testSkipsConnectorWithUnlinkableConfiguration fails with BASE's ReflectionUtil (IllegalAccessException: no such constructor ...UnlinkableConfiguration) and fails with the NoClassDefFoundError when the catch (NoClassDefFoundError) at LocalConnectorInfoManagerImpl.java:303 is removed.
• ReflectionUtilTests.newInstanceFailsOnInaccessibleConstructor pins the IllegalAccessError exclusion through publicLookup().

---

issue (non-blocking): the new test's temp bundle jar is not deleted on Windows, because the bundle class loader still holds it open.

OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/api/LocalConnectorInfoManagerTests.java:239-245

The manager is built straight from the file: URL, so the jar itself goes on the class path of a child-first BundleClassLoader (LocalConnectorInfoManagerImpl.java:248). That loader opens the jar to define LinkableConnector, and nothing ever closes it. This manager is not in the factory cache either, so clearLocalCache never reaches it. On the windows-latest legs bundle.delete() therefore fails, its result is ignored, and every run leaves an unlinkable-bundle*.jar in java.io.tmpdir. The zip-slip test at :151 has no such leak: its bundle is refused before any loader is built. I did not run this on Windows.

        List<ConnectorInfo> infos = null;
        try {
            infos = new LocalConnectorInfoManagerImpl(Collections.singletonList(bundle.toURI()
                    .toURL()), parent).getConnectorInfos();
            assertEquals(infos.size(), 1, String.valueOf(infos));
            assertEquals(infos.get(0).getConnectorKey().getConnectorName(),
                    LinkableConnector.class.getName());
        } finally {
            // the bundle class loader keeps the jar open, and Windows cannot
            // delete an open file
            if (infos != null && !infos.isEmpty()) {
                ClassLoader loader = ((LocalConnectorInfoImpl) infos.get(0)).getConnectorClass()
                        .getClassLoader();
                if (loader instanceof Closeable) {
                    ((Closeable) loader).close();
                }
            }
            bundle.delete();
        }

This needs two imports: java.io.Closeable and org.identityconnectors.framework.impl.api.local.LocalConnectorInfoImpl.

---

suggestion (non-blocking): no test case has a findConstructor failure with a null cause, so the cause instanceof LinkageError check is not pinned.

OpenICF-java-framework/connector-framework/src/main/java/org/identityconnectors/common/ReflectionUtil.java:168-170

findConstructor gives an IllegalAccessException with no cause for a class the lookup cannot see ("symbolic reference class is not accessible"). It gives a NoSuchMethodException with no cause for an array class. Drop cause instanceof LinkageError && and both inputs become throw (LinkageError) null, a NullPointerException instead of the exception the @throws contract promises. Yet that mutant passes all 8 ReflectionUtilTests cases (measured on JDK 11, 17, 21 and 26), because each of their failure inputs has a non-null cause.

    static class PackagePrivate {
    }

    @Test(expectedExceptions = IllegalAccessException.class)
    public void newInstanceFailsOnInaccessibleClass() throws Exception {
        // findConstructor reports a class the lookup cannot see without a cause
        newInstance(MethodHandles.publicLookup(), PackagePrivate.class);
    }

Pin: with the mutant, this case fails with a NullPointerException.

---

suggestion (non-blocking): the rethrown LinkageError no longer names the class that failed to link.

OpenICF-java-framework/connector-framework/src/main/java/org/identityconnectors/common/ReflectionUtil.java:170

On the PR's own fixture, the error that escapes is NoClassDefFoundError "...$MissingDependency", caused by a ClassNotFoundException. UnlinkableConfiguration appears nowhere in that chain. The dropped IllegalAccessException "no such constructor: <class>.<init>()void" was the only exception that named it. Some callers have no context of their own: ConnectorPoolManager:181 wraps the error and ConnectorDocBuilder:207 logs it. Those callers now report only the missing dependency. This matches what Class.newInstance() did in 2.0.4. A suppressed exception would keep the class name without changing the type or message that the tests and the info managers rely on.

            if (cause instanceof LinkageError && !(cause instanceof NoSuchMethodError)
                    && !(cause instanceof IllegalAccessError)) {
                // keep the name of the class that failed to link; e has cause as
                // its own cause, so record only its message
                cause.addSuppressed(new IllegalAccessException(e.getMessage()));
                throw (LinkageError) cause;
            }

---

nitpick (non-blocking): the Javadoc and the catch comment say that any other LinkageError means the class failed to link, but a class whose link fails with IllegalAccessError still gets "no such constructor".

OpenICF-java-framework/connector-framework/src/main/java/org/identityconnectors/common/ReflectionUtil.java:139-143, :163-166

Verifying a class can itself fail with IllegalAccessError, for example when a dependency cannot access its superclass. The exclusion at :169 then keeps that failure as IllegalAccessException: no such constructor. A probe on JDK 11, 17, 21 and 26 showed Class.newInstance() throwing the raw IllegalAccessError, while BASE and HEAD both wrap it. The scan fails the same way in all three cases, so the code needs no change, only the text.

     * A class that cannot be linked, typically because a class it refers to is
     * missing from the connector bundle, fails with that {@link LinkageError}
     * (e.g. {@link NoClassDefFoundError}), as it did with
     * {@link Class#newInstance()}: the connector info managers skip such a
     * connector instead of failing the whole bundle. A link failure that is an
     * {@link IllegalAccessError} cannot be told from an inaccessible
     * constructor and is reported as {@link IllegalAccessException}.

---

nitpick (non-blocking): the comment on the hiding parent loader gives only one of the two reasons why MissingDependency is unavailable.

OpenICF-java-framework/connector-framework-internal/src/test/java/org/identityconnectors/framework/impl/api/LocalConnectorInfoManagerTests.java:225-226, :227-236

MissingDependency is in org.identityconnectors.framework.impl.api. BundleClassLoader.checkAccessAllowed already refuses every parent-loaded class in that package ("Connector may not reference class ..., it is an internal framework class"). So the test stays green with the hiding loader removed; only the ClassNotFoundException message changes. The loader would start to matter if the fixtures moved to an allowed package.

        // and hidden from the parent class loader, which has it on the test
        // class path, as a dependency missing from the class path would be
        // (BundleClassLoader refuses it anyway, as an internal framework class)

[vharseko]

@maximthomas Round 2 is in bfb989f; the branch is rebased onto the current master.

issue: temp bundle jar on Windows. Applied as you suggested: the finally closes the bundle class loader (reached through LocalConnectorInfoImpl.getConnectorClass()) before bundle.delete(). The bundle has no message catalogs, so the loader's JarFile is the only open handle. I could not run it on Windows either; on macOS the test passes.

suggestion: null cause. Added newInstanceFailsOnInaccessibleClass (publicLookup() and a package-private class). On JDK 11, 17, 21 and 27, findConstructor reports it as IllegalAccessException: symbolic reference class is not accessible, with getCause() == null. With cause instanceof LinkageError && dropped, the test fails with NullPointerException: Cannot throw exception because "cause" is null (JDK 21).

nitpick: IllegalAccessError link failure. Reproduced on JDK 11, 17, 21 and 27 with a class whose dependency cannot access its superclass. Class.newInstance() throws the raw IllegalAccessError, and findConstructor throws IllegalAccessException: no such constructor with that cause. I took your Javadoc text and added the same point to the catch comment. I also narrowed "the connector info managers skip such a connector" to the NoClassDefFoundError case: the managers catch only NoClassDefFoundError and TypeNotPresentException around createDefaultAPIConfiguration.

nitpick: the hiding parent loader. Took your comment. The loader stays, so the test does not depend on the package restriction in BundleClassLoader.

suggestion: class name in the rethrown LinkageError. I would rather leave this one as it is:

• The PR restores what Class.newInstance() did in 2.0.4, and you point out that this matches. The class name was not in that error either.
• The suppressed exception would carry no such constructor: ...<init>()void. That is the text that misled the OpenIDM#259 diagnosis. In a stack trace it reads like a second failure, a missing constructor, not like context.
• On the path this PR is about, the managers already name the connector: Unable to load configuration class of connector class ...LocalConnectorInfoManagerTests$UnlinkableConnector from bundle file:/.../unlinkable-bundle….jar ... java.lang.NoClassDefFoundError: ...$MissingDependency (from the run of the new test).
• ConnectorPoolManager:181 is catch (Exception e), so the LinkageError passes it unwrapped, as in 2.0.4. ConnectorDocBuilder:207 logs the throwable at debug level. If those two callers need the class in their message, it belongs there: they know the connector key, and the shared utility does not.

[maximthomas]

praise: the round closes the test-side gaps from round 2 without touching the fix.

• testSkipsConnectorWithUnlinkableConfiguration now closes the connector's class loader (LocalConnectorInfoImpl.getConnectorClass().getClassLoader(), a URLClassLoader) before bundle.delete(), so the scan no longer holds the temp jar open.
• ReflectionUtilTests.newInstanceFailsOnInaccessibleClass pins the cause-less findConstructor failure (publicLookup() on package-private PackagePrivate) as an IllegalAccessException.
• The unlinkable-configuration case pins the fix on JDK 25.0.2 as well: with ReflectionUtil.java:173-177 deleted it fails with IllegalAccessException: no such constructor: ...LocalConnectorInfoManagerTests$UnlinkableConfiguration.<init>()void (1/1 red).