[VFS] Optimize FTP directory existence to use CWD instead of parent LIST

commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java

@@ -64,6 +64,32 @@ protected FTPClientWrapper(final GenericFileName rootFileName, final FileSystemO
         getFtpClient(); // fail-fast
     }
 
+    /** {@inheritDoc} */
+    @Override
+    public boolean isDirectory(final String relPath) throws IOException {
+        try {
+            return isDirectoryWithCwd(relPath);
+        } catch (final IOException e) {
+            disconnect();
+            return isDirectoryWithCwd(relPath);
+        }
+    }
+
+    private boolean isDirectoryWithCwd(final String relPath) throws IOException {
+        // CWD "." checks the current directory without changing it — no save/restore needed.
+        if (".".equals(relPath)) {
+            return getFtpClient().changeWorkingDirectory(relPath);
+        }
+        final String saved = getFtpClient().printWorkingDirectory();
+        try {
+            return getFtpClient().changeWorkingDirectory(relPath);
+        } finally {
+            if (saved != null) {
+                getFtpClient().changeWorkingDirectory(saved);
+            }
+        }
+    }
+
     @Override
     public boolean abort() throws IOException {
         try {

commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java

@@ -70,6 +70,20 @@ default boolean changeDirectory(String relPath) throws IOException {
         return false;
     }
 
+    /**
+     * Tests whether a remote path is an existing directory by attempting to CWD into it.
+     * Implementations must save and restore the working directory to avoid side effects
+     * on subsequent operations that use relative paths.
+     *
+     * @param relPath The pathname to test.
+     * @return true if the path is an existing directory, false otherwise.
+     * @throws IOException If an I/O error occurs.
+     * @since 2.11.0
+     */
+    default boolean isDirectory(String relPath) throws IOException {
+        return false;
+    }
+
     /**
      * Deletes a file on the FTP server.
      *

commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java

@@ -558,7 +558,19 @@ private long getTimestampMillis() throws IOException {
                 mdtmSet = true;
             }
         }
-        return ftpFile.getTimestamp().getTime().getTime();
+        // Synthetic FTPFile from CWD optimization has no timestamp.
+        // Lazily fetch the real FTPFile via parent LIST when needed.
+        if (ftpFile.getTimestamp() == null) {
+            final FtpFileObject parent = (FtpFileObject) FileObjectUtils.getAbstractFileObject(getParent());
+            if (parent != null) {
+                final FTPFile realFile = parent.getChildFile(UriParser.decode(getName().getBaseName()), false);
+                if (realFile != null) {
+                    ftpFile = realFile;
+                }
+            }
+        }
+        final Calendar timestamp = ftpFile.getTimestamp();
+        return timestamp != null ? timestamp.getTime().getTime() : DEFAULT_TIMESTAMP;
     }
 
     /**
@@ -630,34 +642,48 @@ private void setFTPFile(final boolean flush) throws IOException {
             final FtpFileObject parent = (FtpFileObject) FileObjectUtils.getAbstractFileObject(getParent());
             final FTPFile newFileInfo;
             if (parent != null) {
-                newFileInfo = parent.getChildFile(UriParser.decode(getName().getBaseName()), flush);
+                // Try CWD first: if it succeeds, this is a directory and we avoid
+                // the expensive parent LIST which opens a data channel and transfers
+                // the entire directory listing. If CWD fails, fall back to parent
+                // LIST to handle files, symlinks, and non-existent paths.
+                //
+                // Note: CWD follows symlinks transparently, so a symlink-to-directory
+                // is classified as a plain directory here. This is acceptable because
+                // FTP symlink behavior is provider-internal (doGetType resolves symlinks
+                // to their target type) and not exposed to users via isSymbolicLink(),
+                // which always returns false for FTP (doIsSymbolicLink is not overridden).
+                final FTPFile cwdResult = verifyDirectory();
+                if (cwdResult != null) {
+                    newFileInfo = cwdResult;
+                } else {
+                    newFileInfo = parent.getChildFile(UriParser.decode(getName().getBaseName()), flush);
+                }
             } else {
                 // Root-level resource: no parent to query via getChildFile().
-                // Verify the directory exists using CWD, which is a lightweight
-                // control-channel command (no data transfer). Previously this
-                // assumed the root always exists, causing exists() to return true
-                // for non-existent FTP folders.
-                newFileInfo = verifyRootDirectory();
+                newFileInfo = verifyDirectory();
             }
             ftpFile = newFileInfo == null ? UNKNOWN : newFileInfo;
         }
     }
 
     /**
-     * Verifies a root-level FTP directory exists by attempting to CWD into it.
+     * Verifies an FTP directory exists by attempting to CWD into it.
      * Returns an FTPFile with DIRECTORY_TYPE if CWD succeeds, or {@code null} if it fails.
+     * CWD is used instead of LIST because it is a lightweight control-channel command
+     * that does not transfer directory contents.
      * <p>
-     * Uses CWD "." (the current directory) rather than CWD "/" to verify
-     * the logical root. With {@code userDirIsRoot=true}, the logical root
-     * is the user's login directory, not the server root "/".
+     * For root-level files (relPath is null), uses CWD "." to check the current
+     * directory rather than CWD "/". This is correct for both userDirIsRoot
+     * configurations: with userDirIsRoot=true the logical root is the user's login
+     * directory, not the server root "/".
      * </p>
      */
-    private FTPFile verifyRootDirectory() throws IOException {
+    private FTPFile verifyDirectory() throws IOException {
         final FtpClient client = getAbstractFileSystem().getClient();
         try {
-            // relPath is always null for the root (constructor maps "." to null),
-            // so this always resolves to CWD ".". The relPath check is defensive.
-            if (client.changeDirectory(relPath != null ? relPath : ".")) {
+            // relPath is null for the root (constructor maps "." to null).
+            // For non-root paths, isDirectory() saves/restores the working directory.
+            if (client.isDirectory(relPath != null ? relPath : ".")) {
                 final FTPFile result = new FTPFile();
                 result.setType(FTPFile.DIRECTORY_TYPE);
                 return result;

commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpCwdOptimizationTest.java

@@ -0,0 +1,180 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.vfs2.provider.ftp;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.time.Duration;
+
+import org.apache.commons.vfs2.FileObject;
+import org.apache.commons.vfs2.FileSystemOptions;
+import org.apache.commons.vfs2.FileType;
+import org.apache.commons.vfs2.VfsTestUtils;
+import org.apache.commons.vfs2.impl.DefaultFileSystemManager;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests that {@link FtpFileObject} uses CWD to check directory existence before
+ * falling back to the expensive parent LIST command.
+ * <p>
+ * Verifies that:
+ * <ul>
+ *   <li>Directories are correctly detected via CWD (no parent LIST needed)</li>
+ *   <li>Files are correctly detected via parent LIST fallback</li>
+ *   <li>Non-existent paths return {@link FileType#IMAGINARY}</li>
+ *   <li>Working directory is preserved after all operations</li>
+ *   <li>Directory timestamps are lazily fetched via parent LIST when requested</li>
+ * </ul>
+ */
+public class FtpCwdOptimizationTest {
+
+    private File testDir;
+    private File nestedDir;
+    private File fileInRoot;
+    private File fileInNested;
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        FtpProviderTest.setUpClass(VfsTestUtils.getTestDirectory(), null, null);
+
+        testDir = new File(VfsTestUtils.getTestDirectory());
+        final File cwdOptDir = new File(testDir, "cwdOptDir");
+        nestedDir = new File(cwdOptDir, "nested");
+        nestedDir.mkdirs();
+        fileInRoot = new File(testDir, "rootfile.txt");
+        fileInNested = new File(cwdOptDir, "file.txt");
+        try (FileWriter w = new FileWriter(fileInRoot)) {
+            w.write("root content");
+        }
+        try (FileWriter w = new FileWriter(fileInNested)) {
+            w.write("nested content");
+        }
+    }
+
+    @AfterEach
+    public void tearDown() {
+        FtpProviderTest.tearDownClass();
+        if (fileInNested != null) fileInNested.delete();
+        if (fileInRoot != null) fileInRoot.delete();
+        if (nestedDir != null) nestedDir.delete();
+        new File(testDir, "cwdOptDir").delete();
+    }
+
+    private static FileSystemOptions createOptions() {
+        final FileSystemOptions options = new FileSystemOptions();
+        final FtpFileSystemConfigBuilder builder = FtpFileSystemConfigBuilder.getInstance();
+        builder.setPassiveMode(options, true);
+        builder.setConnectTimeout(options, Duration.ofSeconds(10));
+        return options;
+    }
+
+    @Test
+    public void testNestedDirectoryExistsViaOptimization() throws Exception {
+        try (final DefaultFileSystemManager manager = new DefaultFileSystemManager()) {
+            manager.addProvider("ftp", new FtpFileProvider());
+            manager.init();
+            final FileObject dir = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/cwdOptDir/nested", createOptions());
+            assertTrue(dir.exists(), "Nested directory should exist");
+            assertEquals(FileType.FOLDER, dir.getType(), "Nested directory should be FOLDER");
+        }
+    }
+
+    @Test
+    public void testFileDetectedViaListFallback() throws Exception {
+        try (final DefaultFileSystemManager manager = new DefaultFileSystemManager()) {
+            manager.addProvider("ftp", new FtpFileProvider());
+            manager.init();
+            final FileObject file = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/cwdOptDir/file.txt", createOptions());
+            assertTrue(file.exists(), "File should exist");
+            assertEquals(FileType.FILE, file.getType(), "file.txt should be FILE");
+        }
+    }
+
+    @Test
+    public void testNonExistentPathIsImaginary() throws Exception {
+        try (final DefaultFileSystemManager manager = new DefaultFileSystemManager()) {
+            manager.addProvider("ftp", new FtpFileProvider());
+            manager.init();
+            final FileObject ghost = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/cwdOptDir/doesNotExist", createOptions());
+            assertFalse(ghost.exists(), "Non-existent path should not exist");
+            assertEquals(FileType.IMAGINARY, ghost.getType(), "Non-existent path should be IMAGINARY");
+        }
+    }
+
+    @Test
+    public void testWorkingDirectoryPreservedAfterMixedChecks() throws Exception {
+        try (final DefaultFileSystemManager manager = new DefaultFileSystemManager()) {
+            manager.addProvider("ftp", new FtpFileProvider());
+            manager.init();
+            final FileSystemOptions options = createOptions();
+
+            final FileObject rootFile = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/rootfile.txt", options);
+            assertTrue(rootFile.exists(), "rootfile.txt should exist initially");
+
+            final FileObject dir = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/cwdOptDir/nested", options);
+            assertTrue(dir.exists(), "nested dir should exist");
+
+            final FileObject nestedFile = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/cwdOptDir/file.txt", options);
+            assertTrue(nestedFile.exists(), "file.txt should exist");
+
+            final FileObject ghost = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/cwdOptDir/doesNotExist", options);
+            assertFalse(ghost.exists(), "ghost should not exist");
+
+            rootFile.refresh();
+            assertTrue(rootFile.exists(),
+                "rootfile.txt must still be accessible after mixed CWD/LIST operations");
+            assertEquals(FileType.FILE, rootFile.getType());
+        }
+    }
+
+    /**
+     * Tests that requesting the last modified time on a CWD-resolved directory
+     * does not throw NPE and returns a valid timestamp. The CWD optimization
+     * creates a synthetic FTPFile with no timestamp; the timestamp must be lazily
+     * fetched via parent LIST when first requested.
+     */
+    @Test
+    public void testDirectoryTimestampLazilyFetched() throws Exception {
+        try (final DefaultFileSystemManager manager = new DefaultFileSystemManager()) {
+            manager.addProvider("ftp", new FtpFileProvider());
+            manager.init();
+            final FileObject dir = manager.resolveFile(
+                FtpProviderTest.getConnectionUri() + "/cwdOptDir/nested", createOptions());
+            assertTrue(dir.exists(), "Nested directory should exist");
+
+            // Must not throw NPE, and should return a real timestamp from the
+            // lazy parent LIST fallback (not 0/DEFAULT_TIMESTAMP).
+            final long lastModified = dir.getContent().getLastModifiedTime();
+            assertTrue(lastModified > 0,
+                "Directory timestamp should be fetched lazily via parent LIST, not default 0");
+        }
+    }
+}