diff --git a/src/main/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactory.java b/src/main/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactory.java index e3771798a..d544a474a 100644 --- a/src/main/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactory.java +++ b/src/main/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactory.java @@ -118,12 +118,15 @@ private FTPFileEntryParser createFileEntryParser(final String key, final FTPClie // Is the key a possible class name? if (JAVA_QUALIFIED_NAME_PATTERN.matcher(key).matches()) { try { - final Class parserClass = Class.forName(key); + // Load without initialising, so an unrelated class (e.g. one taken from an untrusted SYST reply) + // does not run its static initialiser before the type is checked. + final Class parserClass = Class.forName(key, false, getClass().getClassLoader()); + if (!FTPFileEntryParser.class.isAssignableFrom(parserClass)) { + throw new ParserInitializationException( + parserClass.getName() + " does not implement the interface " + FTPFileEntryParser.class.getCanonicalName()); + } try { parser = (FTPFileEntryParser) parserClass.getConstructor().newInstance(); - } catch (final ClassCastException e) { - throw new ParserInitializationException( - parserClass.getName() + " does not implement the interface " + FTPFileEntryParser.class.getCanonicalName(), e); } catch (final Exception | LinkageError e) { throw new ParserInitializationException("Error initializing parser", e); } diff --git a/src/test/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactoryTest.java b/src/test/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactoryTest.java index 4833d67b0..1068b87cb 100644 --- a/src/test/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactoryTest.java +++ b/src/test/java/org/apache/commons/net/ftp/parser/DefaultFTPFileEntryParserFactoryTest.java @@ -29,6 +29,21 @@ import org.junit.jupiter.api.Test; class DefaultFTPFileEntryParserFactoryTest { + + /** Set from {@link ConstructorProbe}'s static initialiser; kept here so reading it does not initialise the probe. */ + static boolean probeInitialized; + + /** A non-parser class whose static initialiser records that the class was initialised. */ + public static final class ConstructorProbe { + static { + probeInitialized = true; + } + + public ConstructorProbe() { + // empty + } + } + private void checkParserClass(final FTPFileEntryParserFactory fact, final String key, final Class expected) { final FTPClientConfig config = key == null ? new FTPClientConfig() : new FTPClientConfig(key); final FTPFileEntryParser parser = fact.createFileEntryParser(config); @@ -104,7 +119,7 @@ void testDefaultParserFactory() { factory.createFileEntryParser("org.apache.commons.net.ftp.parser.DefaultFTPFileEntryParserFactory"); fail("Exception should have been thrown. \"DefaultFTPFileEntryParserFactory\" does not implement FTPFileEntryParser"); } catch (final ParserInitializationException pie) { - assertTrue(pie.getCause() instanceof ClassCastException); + assertTrue(pie.getMessage().contains("does not implement the interface"), pie.getMessage()); } try { @@ -112,7 +127,7 @@ void testDefaultParserFactory() { factory.createFileEntryParser("org.apache.commons.net.ftp.parser.FTPFileEntryParserFactory"); fail("ParserInitializationException should have been thrown."); } catch (final ParserInitializationException pie) { - assertTrue(pie.getCause() instanceof ReflectiveOperationException, pie.getCause().toString()); + assertTrue(pie.getMessage().contains("does not implement the interface"), pie.getMessage()); } try { // Class exists, but is abstract @@ -152,4 +167,14 @@ void testDefaultParserFactoryConfig() throws Exception { // Note: exact matching via config is the only way to generate NTFTPEntryParser and OS400FTPEntryParser // using DefaultFTPFileEntryParserFactory } + + @Test + void testNonParserClassIsNotInstantiated() { + final DefaultFTPFileEntryParserFactory factory = new DefaultFTPFileEntryParserFactory(); + probeInitialized = false; + // A qualified class name can arrive from the server's SYST reply during auto-detection. + // Referencing the .class literal does not initialise the probe, so probeInitialized stays false unless the factory triggers it. + assertThrows(ParserInitializationException.class, () -> factory.createFileEntryParser(ConstructorProbe.class.getName())); + assertFalse(probeInitialized, "a class that is not an FTPFileEntryParser must not be loaded with initialisation"); + } }