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-net.git
The following commit(s) were added to refs/heads/master by this push:
new 6e387497 Check parser type before instantiating class in
DefaultFTPFileEntryParserFactory.createFileEntryParser() (#403)
6e387497 is described below
commit 6e38749760c0366aac1637c7dc15e4ccf76329c9
Author: Javid Khan <[email protected]>
AuthorDate: Sun Jul 5 21:35:17 2026 +0530
Check parser type before instantiating class in
DefaultFTPFileEntryParserFactory.createFileEntryParser() (#403)
* check parser type before instantiating class in createFileEntryParser
During listing auto-detection the parser key is taken from the server's
SYST reply. A key shaped like a qualified class name was passed to
Class.forName and constructed before the type was checked, so any classpath
class could be instantiated for its side effects. Verify the class implements
FTPFileEntryParser before calling the constructor.
* load parser class without initialising before the type check
Class.forName(key) initialises the class, so an unrelated class named
in the key ran its static initialiser before isAssignableFrom rejected
it. Load with the three-arg forName (initialize=false) so only a real
FTPFileEntryParser is initialised, when it is constructed. Test now
asserts the static initialiser does not run.
Signed-off-by: Javid Khan <[email protected]>
---------
Signed-off-by: Javid Khan <[email protected]>
---
.../parser/DefaultFTPFileEntryParserFactory.java | 11 +++++---
.../DefaultFTPFileEntryParserFactoryTest.java | 29 ++++++++++++++++++++--
2 files changed, 34 insertions(+), 6 deletions(-)
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 e3771798..d544a474 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 @@ public class DefaultFTPFileEntryParserFactory implements
FTPFileEntryParserFacto
// 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 4833d67b..1068b87c 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.apache.commons.net.ftp.FTPFileEntryParser;
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 @@ class DefaultFTPFileEntryParserFactoryTest {
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 @@ class DefaultFTPFileEntryParserFactoryTest {
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 @@ class DefaultFTPFileEntryParserFactoryTest {
// 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");
+ }
}
