diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java index 6ae56b31a8..464b793fee 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java +++ b/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 { diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java index a74dc535f0..6bf2793f0e 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java +++ b/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. * diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java index d8228963eb..c63c4c16bb 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java +++ b/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. *

- * 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 "/". *

*/ - 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; diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpCwdOptimizationTest.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpCwdOptimizationTest.java new file mode 100644 index 0000000000..07098f65c9 --- /dev/null +++ b/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. + *

+ * Verifies that: + *

+ */ +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"); + } + } +}