This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-cli.git
commit 03960a4d9a0a8473b3ea0ad11cbd5891fafa2716 Author: Gary Gregory <[email protected]> AuthorDate: Sat Aug 29 07:55:52 2026 -0400 Load classes without initializing them in Converter.CLASS (#432). --- src/changes/changes.xml | 1 + .../java/org/apache/commons/cli/Converter.java | 5 +- .../org/apache/commons/cli/ConverterTests.java | 78 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 0bc73da0..74e1ec2a 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -36,6 +36,7 @@ <action type="fix" dev="ggregory" due-to="Naveed Khan, Gary Gregory">Fix integer overflow in TextHelpAppendable.indexOfWrap (#437).</action> <action type="fix" dev="ggregory" due-to="Naveed Khan, Gary Gregory">Reject UTF-16 surrogate values in Character converter (#438).</action> <action type="fix" dev="ggregory" due-to="dev_Hakaze, Gary Gregory" issue="CLI-354">Fix HelpFormatter wrapped description indent (#439).</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory, farkhalit rida">Load classes without initializing them in Converter.CLASS (#432).</action> <!-- ADD --> <action type="fix" dev="ggregory" due-to="Elric, Gary Gregory">Add AbstractHelpFormatter.printHelp(String, Options).</action> <!-- UPDATE --> diff --git a/src/main/java/org/apache/commons/cli/Converter.java b/src/main/java/org/apache/commons/cli/Converter.java index 1faf7817..d655205e 100644 --- a/src/main/java/org/apache/commons/cli/Converter.java +++ b/src/main/java/org/apache/commons/cli/Converter.java @@ -44,9 +44,10 @@ public interface Converter<T, E extends Exception> { Converter<?, RuntimeException> DEFAULT = s -> s; /** - * Converts a String to a {@link Class}. Calls {@link Class#forName(String)}. + * Converts a String to a {@link Class}. Calls {@link Class#forName(String, boolean, ClassLoader)} with {@code initialize} set to {@code false} so that + * resolving the name does not by itself trigger the class's static initializer. */ - Converter<Class<?>, ClassNotFoundException> CLASS = Class::forName; + Converter<Class<?>, ClassNotFoundException> CLASS = s -> Class.forName(s, false, Converter.class.getClassLoader()); /** * Converts a String to a {@link File}. Calls {@link File#File(String)}. diff --git a/src/test/java/org/apache/commons/cli/ConverterTests.java b/src/test/java/org/apache/commons/cli/ConverterTests.java index 57cb63f9..9c4c8278 100644 --- a/src/test/java/org/apache/commons/cli/ConverterTests.java +++ b/src/test/java/org/apache/commons/cli/ConverterTests.java @@ -18,8 +18,10 @@ package org.apache.commons.cli; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.net.URL; import java.text.DateFormat; @@ -41,6 +43,14 @@ import org.junitpioneer.jupiter.DefaultLocale; */ public class ConverterTests { + // A class whose static initializer has an observable side effect. + public static class AClassWithAStaticInitializer { + + static { + classInitializerRan = true; + } + } + // A class without a default constructor. public class AClassWithoutADefaultConstructor { @@ -48,6 +58,35 @@ public class ConverterTests { } } + // A Plugin used by the end-to-end public-API test; kept separate so it is not initialized by another test first. + public static class ApiPluginImpl implements Plugin { + + static { + apiPluginInitializerRan = true; + } + } + + // Marker type a caller might validate a resolved Class against before touching it. + public interface Plugin { + } + + // A Plugin whose static initializer has an observable side effect. + public static class PluginImpl implements Plugin { + + static { + pluginInitializerRan = true; + } + } + + // Set by the static initializer of AClassWithAStaticInitializer; readable without initializing that class. + private static boolean classInitializerRan; + + // Set by the static initializer of PluginImpl; readable without initializing that class. + private static boolean pluginInitializerRan; + + // Set by the static initializer of ApiPluginImpl; readable without initializing that class. + private static boolean apiPluginInitializerRan; + private static Stream<Arguments> numberTestParameters() { final List<Arguments> lst = new ArrayList<>(); lst.add(Arguments.of("123", Long.valueOf("123"))); @@ -72,6 +111,45 @@ public class ConverterTests { assertNotNull(Converter.CLASS.apply(AClassWithoutADefaultConstructor.class.getName())); } + @Test + void testClassDoesNotInitialize() throws Exception { + final Class<?> cls = Converter.CLASS.apply(AClassWithAStaticInitializer.class.getName()); + assertFalse(classInitializerRan); + assertEquals(AClassWithAStaticInitializer.class, cls); + cls.getConstructor().newInstance(); + assertTrue(classInitializerRan); + } + + @Test + void testClassNotInitializedThroughPublicApi() throws Exception { + // What an application using the public API actually does: declare a Class-typed option via a pattern, + // parse argv, query the value, then gate it. Parsing and querying must not run the named class's + // static initializer; only the application deciding to instantiate it should. + final Options options = PatternOptionBuilder.parsePattern("c+"); + final CommandLine line = new DefaultParser().parse(options, new String[] {"-c", ApiPluginImpl.class.getName()}); + final Class<?> cls = line.getParsedOptionValue("c"); + assertEquals(ApiPluginImpl.class, cls); + // The isAssignableFrom gate a caller uses to reject non-plugins runs without initializing the class. + assertTrue(Plugin.class.isAssignableFrom(cls)); + assertFalse(apiPluginInitializerRan); + // Only when the application instantiates the class does its static initializer run. + final Object instance = cls.getConstructor().newInstance(); + assertTrue(instance instanceof Plugin); + assertTrue(apiPluginInitializerRan); + } + + @Test + void testClassValidatedBeforeInitialization() throws Exception { + // The caller pattern this enables: resolve the option, gate it with isAssignableFrom, then instantiate. + // The assignability check must not run the class's static initializer; only newInstance() should. + final Class<?> cls = Converter.CLASS.apply(PluginImpl.class.getName()); + assertTrue(Plugin.class.isAssignableFrom(cls)); + assertFalse(pluginInitializerRan); + final Object instance = cls.getConstructor().newInstance(); + assertTrue(instance instanceof Plugin); + assertTrue(pluginInitializerRan); + } + @Test void testDate() throws Exception { assertThrows(java.text.ParseException.class, () -> Converter.DATE.apply("whatever"));
