Merge pull request #16762 from iterate-ch/bugfix/MD-22844

Make sure required feature is available regardless of connection state.
iterate-ch · Jan 11, 2025 · 237e5e2 · 237e5e2
2 parents 4f3226f + 1374aac
commit 237e5e2
Show file tree

Hide file tree

Showing 16 changed files with 104 additions and 82 deletions.
diff --git a/core/src/main/java/ch/cyberduck/core/FeatureFactory.java b/core/src/main/java/ch/cyberduck/core/FeatureFactory.java
@@ -0,0 +1,28 @@
+package ch.cyberduck.core;
+
+/*
+ * Copyright (c) 2002-2025 iterate GmbH. All rights reserved.
+ * https://cyberduck.io/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+public interface FeatureFactory {
+    @SuppressWarnings("unchecked")
+    <T> T getFeature(Class<T> type);
+
+    FeatureFactory disabled = new FeatureFactory() {
+        @Override
+        public <T> T getFeature(final Class<T> type) {
+            return null;
+        }
+    };
+}
diff --git a/core/src/main/java/ch/cyberduck/core/Protocol.java b/core/src/main/java/ch/cyberduck/core/Protocol.java
@@ -25,7 +25,7 @@
 import java.util.Map;
 import java.util.Set;
 
-public interface Protocol extends Comparable<Protocol>, Serializable {
+public interface Protocol extends FeatureFactory, Comparable<Protocol>, Serializable {
 
     /**
      * Check login credentials for validity for this protocol.
@@ -365,5 +365,6 @@ enum VersioningMode {
     }
 
     @SuppressWarnings("unchecked")
+    @Override
     <T> T getFeature(final Class<T> type);
 }
diff --git a/core/src/main/java/ch/cyberduck/core/Session.java b/core/src/main/java/ch/cyberduck/core/Session.java
@@ -47,7 +47,7 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
-public abstract class Session<C> implements TranscriptListener {
+public abstract class Session<C> implements FeatureFactory, TranscriptListener {
     private static final Logger log = LogManager.getLogger(Session.class);
 
     /**
@@ -255,8 +255,8 @@ public void log(final Type request, final String message) {
      * @return Feature implementation or null when not supported
      */
     @SuppressWarnings("unchecked")
+    @Override
     public <T> T getFeature(final Class<T> type) {
-        metrics.increment(type);
         return this.getFeature(type, this._getFeature(type));
     }
 
@@ -269,6 +269,7 @@ public <T> T getFeature(final Class<T> type) {
      */
     @SuppressWarnings("unchecked")
     public <T> T getFeature(final Class<T> type, final T feature) {
+        metrics.increment(type);
         return registry.getFeature(this, type, feature);
     }
 

diff --git a/core/src/main/java/ch/cyberduck/core/pool/DefaultSessionPool.java b/core/src/main/java/ch/cyberduck/core/pool/DefaultSessionPool.java
@@ -18,12 +18,9 @@
 import ch.cyberduck.core.ConnectionService;
 import ch.cyberduck.core.Host;
 import ch.cyberduck.core.Session;
-import ch.cyberduck.core.SessionFactory;
 import ch.cyberduck.core.TranscriptListener;
 import ch.cyberduck.core.exception.BackgroundException;
 import ch.cyberduck.core.exception.ConnectionCanceledException;
-import ch.cyberduck.core.ssl.DefaultX509KeyManager;
-import ch.cyberduck.core.ssl.DisabledX509TrustManager;
 import ch.cyberduck.core.ssl.X509KeyManager;
 import ch.cyberduck.core.ssl.X509TrustManager;
 import ch.cyberduck.core.threading.BackgroundActionState;
@@ -53,37 +50,36 @@ public class DefaultSessionPool implements SessionPool {
     private final FailureDiagnostics<BackgroundException> diagnostics
             = new DefaultFailureDiagnostics();
 
-    private final ConnectionService connect;
     private final TranscriptListener transcript;
     private final Host bookmark;
 
     private final VaultRegistry registry;
 
     private final GenericObjectPool<Session> pool;
 
-    private SessionPool features = SessionPool.DISCONNECTED;
+    private static final GenericObjectPoolConfig<Session> configuration = new GenericObjectPoolConfig<>();
 
-    public DefaultSessionPool(final ConnectionService connect, final X509TrustManager trust, final X509KeyManager key,
-                              final VaultRegistry registry, final TranscriptListener transcript,
-                              final Host bookmark) {
-        this.connect = connect;
-        this.registry = registry;
-        this.bookmark = bookmark;
-        this.transcript = transcript;
-        final GenericObjectPoolConfig<Session> configuration = new GenericObjectPoolConfig<>();
+    static {
         configuration.setJmxEnabled(false);
         configuration.setEvictionPolicyClassName(CustomPoolEvictionPolicy.class.getName());
         configuration.setBlockWhenExhausted(true);
         configuration.setMaxWait(Duration.ofMillis(BORROW_MAX_WAIT_INTERVAL));
-        this.pool = new GenericObjectPool<>(new PooledSessionFactory(connect, trust, key, bookmark, registry), configuration);
-        final AbandonedConfig abandon = new AbandonedConfig();
+    }
+
+    private static final AbandonedConfig abandon = new AbandonedConfig();
+
+    {
         abandon.setUseUsageTracking(true);
-        this.pool.setAbandonedConfig(abandon);
     }
 
-    public DefaultSessionPool(final ConnectionService connect, final VaultRegistry registry,
-                              final TranscriptListener transcript, final Host bookmark, final GenericObjectPool<Session> pool) {
-        this.connect = connect;
+    public DefaultSessionPool(final ConnectionService connect, final X509TrustManager trust, final X509KeyManager key,
+                              final VaultRegistry registry, final TranscriptListener transcript, final Host bookmark) {
+        this(connect, registry, transcript, bookmark,
+                new GenericObjectPool<>(new PooledSessionFactory(connect, trust, key, bookmark, registry), configuration, abandon));
+    }
+
+    public DefaultSessionPool(final ConnectionService connect, final VaultRegistry registry, final TranscriptListener transcript,
+                              final Host bookmark, final GenericObjectPool<Session> pool) {
         this.transcript = transcript;
         this.bookmark = bookmark;
         this.registry = registry;
@@ -132,9 +128,6 @@ public Session<?> borrow(final BackgroundActionState callback) throws Background
                     log.info("Borrow session from pool {}", this);
                     final Session<?> session = pool.borrowObject();
                     log.info("Borrowed session {} from pool {}", session, this);
-                    if(DISCONNECTED == features) {
-                        features = new StatelessSessionPool(connect, session, transcript, registry);
-                    }
                     return session.withListener(transcript);
                 }
                 catch(IllegalStateException e) {
@@ -259,10 +252,16 @@ public Session.State getState() {
 
     @Override
     public <T> T getFeature(final Class<T> type) {
-        if(DISCONNECTED == features) {
-            return SessionFactory.create(bookmark, new DisabledX509TrustManager(), new DefaultX509KeyManager()).getFeature(type);
+        try {
+            final Session<?> session = this.borrow(BackgroundActionState.running);
+            final T feature = session.getFeature(type);
+            this.release(session, null);
+            return feature;
+        }
+        catch(BackgroundException e) {
+            log.warn("Failure {} obtaining feature from {}", e.toString(), this);
+            return null;
         }
-        return features.getFeature(type);
     }
 
     @Override

diff --git a/core/src/main/java/ch/cyberduck/core/pool/SessionPool.java b/core/src/main/java/ch/cyberduck/core/pool/SessionPool.java
@@ -18,6 +18,7 @@
  */
 
 import ch.cyberduck.core.AbstractProtocol;
+import ch.cyberduck.core.FeatureFactory;
 import ch.cyberduck.core.Host;
 import ch.cyberduck.core.Scheme;
 import ch.cyberduck.core.Session;
@@ -27,7 +28,7 @@
 
 import org.apache.commons.lang3.StringUtils;
 
-public interface SessionPool {
+public interface SessionPool extends FeatureFactory {
     SessionPool DISCONNECTED = new DisconnectedSessionPool();
 
     /**
@@ -70,6 +71,7 @@ public interface SessionPool {
     /**
      * Obtain feature from connection type
      */
+    @Override
     <T> T getFeature(final Class<T> type);
 
     /**
@@ -116,7 +118,7 @@ public Session.State getState() {
 
         @Override
         public <T> T getFeature(final Class<T> type) {
-            return null;
+            return FeatureFactory.disabled.getFeature(type);
         }
 
         @Override

diff --git a/ftp/src/main/java/ch/cyberduck/core/ftp/FTPReadFeature.java b/ftp/src/main/java/ch/cyberduck/core/ftp/FTPReadFeature.java
@@ -41,15 +41,15 @@ public class FTPReadFeature implements Read {
     /**
      * Server process supports RESTart in STREAM mode
      */
-    private final boolean rest;
+    private final EnumSet<Flags> flags = EnumSet.noneOf(Flags.class);
 
     public FTPReadFeature(final FTPSession session) {
-        this(session, true);
+        this.session = session;
     }
 
-    public FTPReadFeature(final FTPSession session, final boolean rest) {
-        this.session = session;
-        this.rest = rest;
+    public void configure(final EnumSet<Flags> flags) {
+        this.flags.clear();
+        this.flags.addAll(flags);
     }
 
     @Override
@@ -81,7 +81,7 @@ public InputStream execute() throws BackgroundException {
 
     @Override
     public EnumSet<Flags> features(final Path file) {
-        return rest ? EnumSet.of(Flags.offset) : EnumSet.noneOf(Flags.class);
+        return flags;
     }
 
     private final class ReadReplyInputStream extends ProxyInputStream {

diff --git a/ftp/src/main/java/ch/cyberduck/core/ftp/FTPSession.java b/ftp/src/main/java/ch/cyberduck/core/ftp/FTPSession.java
@@ -35,7 +35,6 @@
 import ch.cyberduck.core.ftp.list.FTPListService;
 import ch.cyberduck.core.idna.PunycodeConverter;
 import ch.cyberduck.core.preferences.HostPreferences;
-import ch.cyberduck.core.preferences.PreferencesFactory;
 import ch.cyberduck.core.preferences.PreferencesReader;
 import ch.cyberduck.core.proxy.ProxyFinder;
 import ch.cyberduck.core.proxy.ProxySocketFactory;
@@ -62,20 +61,21 @@
 import java.nio.charset.StandardCharsets;
 import java.text.MessageFormat;
 import java.time.Duration;
+import java.util.EnumSet;
 import java.util.Locale;
-import java.util.TimeZone;
 
 public class FTPSession extends SSLSession<FTPClient> {
     private static final Logger log = LogManager.getLogger(FTPSession.class);
 
     private final PreferencesReader preferences
             = new HostPreferences(host);
 
-    private Read read;
+    private final FTPListService list = new FTPListService(this);
+    private final FTPReadFeature read = new FTPReadFeature(this);
+
     private Timestamp timestamp;
     private UnixPermission permission;
     private Symlink symlink;
-    private FTPListService listService;
     private Protocol.Case casesensitivity = Protocol.Case.sensitive;
 
     public FTPSession(final Host h) {
@@ -246,14 +246,6 @@ public void login(final LoginCallback prompt, final CancelCallback cancel) throw
                         }
                     }
                 }
-                final TimeZone zone;
-                if(null == host.getTimezone()) {
-                    zone = TimeZone.getTimeZone(PreferencesFactory.get().getProperty("ftp.timezone.default"));
-                }
-                else {
-                    zone = host.getTimezone();
-                }
-                log.info("Reset parser to timezone {}", zone);
                 String system = StringUtils.EMPTY; //Unknown
                 try {
                     system = client.getSystemType();
@@ -264,8 +256,9 @@ public void login(final LoginCallback prompt, final CancelCallback cancel) throw
                 catch(IOException e) {
                     log.warn("SYST command failed {}", e.getMessage());
                 }
-                read = new FTPReadFeature(this, client.hasFeature("REST", "STREAM"));
-                listService = new FTPListService(this, system, zone);
+                read.configure(client.hasFeature("REST", "STREAM") ?
+                        EnumSet.of(Read.Flags.offset) : EnumSet.noneOf(Read.Flags.class));
+                list.configure(system);
                 if(client.hasFeature(FTPCmd.MFMT.getCommand())) {
                     timestamp = new FTPMFMTTimestampFeature(this);
                 }
@@ -295,7 +288,7 @@ public void login(final LoginCallback prompt, final CancelCallback cancel) throw
     @SuppressWarnings("unchecked")
     public <T> T _getFeature(final Class<T> type) {
         if(type == ListService.class) {
-            return (T) listService;
+            return (T) list;
         }
         if(type == Directory.class) {
             return (T) new FTPDirectoryFeature(this);

diff --git a/ftp/src/main/java/ch/cyberduck/core/ftp/list/FTPListService.java b/ftp/src/main/java/ch/cyberduck/core/ftp/list/FTPListService.java
@@ -28,6 +28,7 @@
 import ch.cyberduck.core.ftp.FTPSession;
 import ch.cyberduck.core.ftp.parser.CompositeFileEntryParser;
 import ch.cyberduck.core.preferences.HostPreferences;
+import ch.cyberduck.core.preferences.PreferencesFactory;
 import ch.cyberduck.core.preferences.PreferencesReader;
 
 import org.apache.commons.lang3.StringUtils;
@@ -91,28 +92,35 @@ public String toString() {
         }
     }
 
-    public FTPListService(final FTPSession session, final String system, final TimeZone zone) {
+    public FTPListService(final FTPSession session) {
         this.session = session;
+        this.configure(StringUtils.EMPTY);
+    }
+
+    public void configure(final String system) {
+        implementations.clear();
         // Directory listing parser depending on response for SYST command
-        final CompositeFileEntryParser parser = new FTPParserSelector().getParser(system, zone);
-        this.implementations.put(Command.list, new FTPDefaultListService(session, parser, Command.list));
+        final TimeZone tz = null == session.getHost().getTimezone() ? TimeZone.getTimeZone(PreferencesFactory.get().getProperty("ftp.timezone.default")) : session.getHost().getTimezone();
+        log.info("Reset parser to timezone {}", tz);
+        final CompositeFileEntryParser parser = new FTPParserSelector().getParser(system, tz);
+        implementations.put(Command.list, new FTPDefaultListService(session, parser, Command.list));
         final PreferencesReader preferences = new HostPreferences(session.getHost());
         if(preferences.getBoolean("ftp.command.stat")) {
             if(StringUtils.isNotBlank(system)) {
                 if(!system.toUpperCase(Locale.ROOT).contains(FTPClientConfig.SYST_NT)) {
                     // Workaround for #5572.
-                    this.implementations.put(Command.stat, new FTPStatListService(session, parser));
+                    implementations.put(Command.stat, new FTPStatListService(session, parser));
                 }
             }
             else {
-                this.implementations.put(Command.stat, new FTPStatListService(session, parser));
+                implementations.put(Command.stat, new FTPStatListService(session, parser));
             }
         }
         if(preferences.getBoolean("ftp.command.mlsd")) {
-            this.implementations.put(Command.mlsd, new FTPMlsdListService(session));
+            implementations.put(Command.mlsd, new FTPMlsdListService(session));
         }
         if(preferences.getBoolean("ftp.command.lista")) {
-            this.implementations.put(Command.lista, new FTPDefaultListService(session, parser, Command.lista));
+            implementations.put(Command.lista, new FTPDefaultListService(session, parser, Command.lista));
         }
     }
 

diff --git a/ftp/src/test/java/ch/cyberduck/core/cryptomator/FTPListServiceTest.java b/ftp/src/test/java/ch/cyberduck/core/cryptomator/FTPListServiceTest.java
@@ -42,7 +42,6 @@
 
 import java.util.Arrays;
 import java.util.EnumSet;
-import java.util.TimeZone;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -59,12 +58,10 @@ public void testListCryptomator() throws Exception {
         final CryptoVault cryptomator = new CryptoVault(vault);
         cryptomator.create(session, new VaultCredentials("test"), vaultVersion);
         session.withRegistry(new DefaultVaultRegistry(new DisabledPasswordStore(), new DisabledPasswordCallback(), cryptomator));
-        assertTrue(new CryptoListService(session, new FTPListService(session, null,
-                TimeZone.getDefault()), cryptomator).list(vault, new DisabledListProgressListener()).isEmpty());
+        assertTrue(new CryptoListService(session, new FTPListService(session), cryptomator).list(vault, new DisabledListProgressListener()).isEmpty());
         new CryptoTouchFeature<>(session, new DefaultTouchFeature<>(new FTPWriteFeature(session)
         ), new FTPWriteFeature(session), cryptomator).touch(test, new TransferStatus());
-        assertEquals(test, new CryptoListService(session, new FTPListService(session, null,
-                TimeZone.getDefault()), cryptomator).list(vault, new DisabledListProgressListener()).get(0));
+        assertEquals(test, new CryptoListService(session, new FTPListService(session), cryptomator).list(vault, new DisabledListProgressListener()).get(0));
         cryptomator.getFeature(session, Delete.class, new FTPDeleteFeature(session)).delete(Arrays.asList(test, vault), new DisabledLoginCallback(), new Delete.DisabledCallback());
     }
 }
diff --git a/ftp/src/test/java/ch/cyberduck/core/cryptomator/FTPWriteFeatureTest.java b/ftp/src/test/java/ch/cyberduck/core/cryptomator/FTPWriteFeatureTest.java
@@ -57,7 +57,6 @@
 import java.io.OutputStream;
 import java.util.Arrays;
 import java.util.EnumSet;
-import java.util.TimeZone;
 
 import static org.junit.Assert.*;
 
@@ -90,7 +89,7 @@ public void testWrite() throws Exception {
         assertEquals(TransferStatus.UNKNOWN_LENGTH, status.getResponse().getSize());
         assertTrue(cryptomator.getFeature(session, Find.class, new DefaultFindFeature(session)).find(test));
         final PathAttributes attr = cryptomator.getFeature(session, AttributesFinder.class, new FTPAttributesFinderFeature(session)).find(test);
-        assertEquals(content.length, new CryptoListService(session, new FTPListService(session, null, TimeZone.getDefault()), cryptomator).list(test.getParent(), new DisabledListProgressListener()).get(test).attributes().getSize());
+        assertEquals(content.length, new CryptoListService(session, new FTPListService(session), cryptomator).list(test.getParent(), new DisabledListProgressListener()).get(test).attributes().getSize());
         final ByteArrayOutputStream buffer = new ByteArrayOutputStream(content.length);
         final InputStream in = new CryptoReadFeature(session, new FTPReadFeature(session), cryptomator).read(test, new TransferStatus(), new DisabledConnectionCallback());
         new StreamCopier(status, status).transfer(in, buffer);