You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2019/10/20 15:30:31 UTC

[mina-sshd] 07/07: [SSHD-842] Updated hierarchy of ServerSessionHolder(s)

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 32f9eb6943dd8abc19a23dfd70fd60e8d3f558e2
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Thu Oct 17 13:50:49 2019 +0300

    [SSHD-842] Updated hierarchy of ServerSessionHolder(s)
---
 .../sshd/client/session/ClientUserAuthService.java | 21 ++++++++-------
 .../main/java/org/apache/sshd/common/Service.java  |  5 ++--
 .../java/org/apache/sshd/server/auth/UserAuth.java |  1 -
 .../sshd/server/channel/AbstractServerChannel.java |  7 -----
 .../apache/sshd/server/channel/ServerChannel.java  |  6 ++++-
 .../server/command/AbstractCommandSupport.java     | 31 +++++++++++++++-------
 .../server/kex/AbstractDHServerKeyExchange.java    | 12 ++++++---
 .../apache/sshd/server/shell/InvertedShell.java    | 15 ++++++++++-
 .../org/apache/sshd/server/shell/ProcessShell.java | 23 +++++++++-------
 .../server/shell/InvertedShellWrapperTest.java     | 28 ++++++++++++++++---
 .../apache/sshd/util/test/BogusInvertedShell.java  |  4 +--
 .../subsystem/sftp/SftpSubsystemEnvironment.java   |  9 ++++++-
 12 files changed, 108 insertions(+), 54 deletions(-)

diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java
index cccbc87..5c00c6a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java
@@ -41,7 +41,6 @@ import org.apache.sshd.common.SshConstants;
 import org.apache.sshd.common.SshException;
 import org.apache.sshd.common.auth.UserAuthMethodFactory;
 import org.apache.sshd.common.session.Session;
-import org.apache.sshd.common.session.SessionHolder;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.ValidateUtils;
 import org.apache.sshd.common.util.buffer.Buffer;
@@ -52,10 +51,7 @@ import org.apache.sshd.common.util.closeable.AbstractCloseable;
  *
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
-public class ClientUserAuthService
-        extends AbstractCloseable
-        implements Service, SessionHolder<ClientSession>, ClientSessionHolder {
-
+public class ClientUserAuthService extends AbstractCloseable implements Service, ClientSessionHolder {
     /**
      * The AuthFuture that is being used by the current auth request. This encodes the state.
      * isSuccess -> authenticated, else if isDone -> server waiting for user auth, else authenticating.
@@ -144,7 +140,8 @@ public class ClientUserAuthService
                     log.debug("auth({})[{}] request new authentication", session, service);
                 }
             } else {
-                currentFuture.setException(new InterruptedIOException("New authentication started before previous completed"));
+                currentFuture.setException(
+                    new InterruptedIOException("New authentication started before previous completed"));
             }
         }
 
@@ -240,7 +237,8 @@ public class ClientUserAuthService
             session.setAuthenticated();
             ((ClientSessionImpl) session).switchToNextService();
 
-            AuthFuture authFuture = Objects.requireNonNull(authFutureHolder.get(), "No current future");
+            AuthFuture authFuture =
+                Objects.requireNonNull(authFutureHolder.get(), "No current future");
             // Will wake up anyone sitting in waitFor
             authFuture.setAuthed(true);
             return;
@@ -270,7 +268,8 @@ public class ClientUserAuthService
         }
 
         if (userAuth == null) {
-            throw new IllegalStateException("Received unknown packet: " + SshConstants.getCommandMessageName(cmd));
+            throw new IllegalStateException(
+                "Received unknown packet: " + SshConstants.getCommandMessageName(cmd));
         }
 
         if (log.isDebugEnabled()) {
@@ -328,9 +327,11 @@ public class ClientUserAuthService
                 }
 
                 // also wake up anyone sitting in waitFor
-                AuthFuture authFuture = Objects.requireNonNull(authFutureHolder.get(), "No current future");
+                AuthFuture authFuture =
+                    Objects.requireNonNull(authFutureHolder.get(), "No current future");
                 authFuture.setException(new SshException(
-                    SshConstants.SSH2_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE, "No more authentication methods available"));
+                    SshConstants.SSH2_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE,
+                    "No more authentication methods available"));
                 return;
             }
 
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/Service.java b/sshd-core/src/main/java/org/apache/sshd/common/Service.java
index 69426e9..2ebc41c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/Service.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/Service.java
@@ -19,6 +19,7 @@
 package org.apache.sshd.common;
 
 import org.apache.sshd.common.session.Session;
+import org.apache.sshd.common.session.SessionHolder;
 import org.apache.sshd.common.util.buffer.Buffer;
 
 /**
@@ -27,9 +28,7 @@ import org.apache.sshd.common.util.buffer.Buffer;
  *
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
-public interface Service extends PropertyResolver, Closeable {
-    Session getSession();
-
+public interface Service extends SessionHolder<Session>, PropertyResolver, Closeable {
     @Override
     default PropertyResolver getParentPropertyResolver() {
         return getSession();
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/auth/UserAuth.java b/sshd-core/src/main/java/org/apache/sshd/server/auth/UserAuth.java
index d26da38..21a92ab 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/auth/UserAuth.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/auth/UserAuth.java
@@ -30,7 +30,6 @@ import org.apache.sshd.server.session.ServerSessionHolder;
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
 public interface UserAuth extends ServerSessionHolder, UserAuthInstance<ServerSession>, UsernameHolder {
-
     /**
      * Try to authenticate the user. This methods should return a non {@code null}
      * value indicating if the authentication succeeded. If the authentication is
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java b/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java
index aa42934..250e818 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java
@@ -36,7 +36,6 @@ import org.apache.sshd.common.session.Session;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.buffer.Buffer;
 import org.apache.sshd.common.util.threads.CloseableExecutorService;
-import org.apache.sshd.server.session.ServerSession;
 
 /**
  * TODO Add javadoc
@@ -44,7 +43,6 @@ import org.apache.sshd.server.session.ServerSession;
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
 public abstract class AbstractServerChannel extends AbstractChannel implements ServerChannel {
-
     protected final AtomicBoolean exitStatusSent = new AtomicBoolean(false);
 
     protected AbstractServerChannel(CloseableExecutorService executor) {
@@ -58,11 +56,6 @@ public abstract class AbstractServerChannel extends AbstractChannel implements S
     }
 
     @Override
-    public ServerSession getServerSession() {
-        return (ServerSession) getSession();
-    }
-
-    @Override
     public OpenFuture open(int recipient, long rwSize, long packetSize, Buffer buffer) {
         setRecipient(recipient);
 
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/channel/ServerChannel.java b/sshd-core/src/main/java/org/apache/sshd/server/channel/ServerChannel.java
index 5dc73de..b3e75c4 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/channel/ServerChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/channel/ServerChannel.java
@@ -20,11 +20,15 @@
 package org.apache.sshd.server.channel;
 
 import org.apache.sshd.common.channel.Channel;
+import org.apache.sshd.server.session.ServerSession;
 import org.apache.sshd.server.session.ServerSessionHolder;
 
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
 public interface ServerChannel extends Channel, ServerSessionHolder {
-    // Marker interface
+    @Override
+    default ServerSession getServerSession() {
+        return (ServerSession) getSession();
+    }
 }
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java
index 67a636a..03d1e3f 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java
@@ -47,7 +47,7 @@ import org.apache.sshd.server.session.ServerSessionHolder;
 public abstract class AbstractCommandSupport
         extends AbstractLoggingBean
         implements Command, Runnable, ExecutorServiceCarrier, SessionAware,
-                    SessionHolder<Session>, ServerSessionHolder {
+        SessionHolder<ServerSession>, ServerSessionHolder {
     protected final String command;
     protected InputStream in;
     protected OutputStream out;
@@ -55,7 +55,7 @@ public abstract class AbstractCommandSupport
     protected ExitCallback callback;
     protected Environment environment;
     protected Future<?> cmdFuture;
-    protected Thread cmdRunner;
+    protected volatile Thread cmdRunner;
     protected CloseableExecutorService executorService;
     protected boolean cbCalled;
     protected ServerSession serverSession;
@@ -78,7 +78,7 @@ public abstract class AbstractCommandSupport
     }
 
     @Override
-    public Session getSession() {
+    public ServerSession getSession() {
         return getServerSession();
     }
 
@@ -145,13 +145,19 @@ public abstract class AbstractCommandSupport
     public void start(ChannelSession channel, Environment env) throws IOException {
         environment = env;
         try {
+            if (log.isDebugEnabled()) {
+                log.debug("start({}) starting runner for command={}", channel, command);
+            }
+
             CloseableExecutorService executors = getExecutorService();
             cmdFuture = executors.submit(() -> {
                 cmdRunner = Thread.currentThread();
                 this.run();
             });
         } catch (RuntimeException e) {    // e.g., RejectedExecutionException
-            log.error("Failed (" + e.getClass().getSimpleName() + ") to start command=" + command + ": " + e.getMessage(), e);
+            log.error("start(" + channel + ")"
+                + " Failed (" + e.getClass().getSimpleName() + ")"
+                + " to start command=" + command + ": " + e.getMessage(), e);
             throw new IOException(e);
         }
     }
@@ -160,11 +166,13 @@ public abstract class AbstractCommandSupport
     public void destroy(ChannelSession channel) throws Exception {
         // if thread has not completed, cancel it
         boolean debugEnabled = log.isDebugEnabled();
-        if ((cmdFuture != null) && (!cmdFuture.isDone()) && (cmdRunner != Thread.currentThread())) {
+        if ((cmdFuture != null)
+                && (!cmdFuture.isDone())
+                && (cmdRunner != Thread.currentThread())) {
             boolean result = cmdFuture.cancel(true);
             // TODO consider waiting some reasonable (?) amount of time for cancellation
             if (debugEnabled) {
-                log.debug("destroy() - cancel pending future=" + result);
+                log.debug("destroy({})[{}] - cancel pending future={}", channel, this, result);
             }
         }
 
@@ -174,7 +182,8 @@ public abstract class AbstractCommandSupport
         if ((executors != null) && (!executors.isShutdown())) {
             Collection<Runnable> runners = executors.shutdownNow();
             if (debugEnabled) {
-                log.debug("destroy() - shutdown executor service - runners count=" + runners.size());
+                log.debug("destroy({})[{}] - shutdown executor service - runners count={}",
+                    channel, this, runners.size());
             }
         }
         this.executorService = null;
@@ -185,10 +194,11 @@ public abstract class AbstractCommandSupport
     }
 
     protected void onExit(int exitValue, String exitMessage) {
+        Session session = getSession();
         if (cbCalled) {
             if (log.isTraceEnabled()) {
-                log.trace("onExit({}) ignore exitValue={}, message={} - already called",
-                        this, exitValue, exitMessage);
+                log.trace("onExit({})[{}] ignore exitValue={}, message={} - already called",
+                    session, this, exitValue, exitMessage);
             }
             return;
         }
@@ -196,7 +206,8 @@ public abstract class AbstractCommandSupport
         ExitCallback cb = getExitCallback();
         try {
             if (log.isDebugEnabled()) {
-                log.debug("onExit({}) exiting - value={}, message={}", this, exitValue, exitMessage);
+                log.debug("onExit({})[{}] exiting - value={}, message={}",
+                    session, this, exitValue, exitMessage);
             }
             cb.onExit(exitValue, exitMessage);
         } finally {
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java b/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java
index 7c11ec5..c8a796e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/kex/AbstractDHServerKeyExchange.java
@@ -19,6 +19,7 @@
 
 package org.apache.sshd.server.kex;
 
+import java.security.KeyPair;
 import java.security.PublicKey;
 import java.util.Objects;
 
@@ -31,9 +32,12 @@ import org.apache.sshd.server.session.ServerSessionHolder;
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
-public abstract class AbstractDHServerKeyExchange extends AbstractDHKeyExchange implements ServerSessionHolder {
+public abstract class AbstractDHServerKeyExchange
+        extends AbstractDHKeyExchange
+        implements ServerSessionHolder {
     protected AbstractDHServerKeyExchange(Session s) {
-        super(ValidateUtils.checkInstanceOf(s, ServerSession.class, "Using a client side KeyExchange on a server: %s", s));
+        super(ValidateUtils.checkInstanceOf(s, ServerSession.class,
+            "Using a client side KeyExchange on a server: %s", s));
     }
 
     @Override
@@ -44,6 +48,8 @@ public abstract class AbstractDHServerKeyExchange extends AbstractDHKeyExchange
     @Override
     public PublicKey getServerKey() {
         ServerSession session = getServerSession();
-        return Objects.requireNonNull(session.getHostKey(), "No server key pair available").getPublic();
+        KeyPair kpHost = Objects.requireNonNull(
+            session.getHostKey(), "No server key pair available");
+        return kpHost.getPublic();
     }
 }
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShell.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShell.java
index 0377afa..ade5e69 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShell.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShell.java
@@ -21,9 +21,12 @@ package org.apache.sshd.server.shell;
 import java.io.InputStream;
 import java.io.OutputStream;
 
+import org.apache.sshd.common.session.SessionHolder;
 import org.apache.sshd.server.SessionAware;
 import org.apache.sshd.server.channel.ChannelSession;
 import org.apache.sshd.server.command.CommandLifecycle;
+import org.apache.sshd.server.session.ServerSession;
+import org.apache.sshd.server.session.ServerSessionHolder;
 
 /**
  * This shell have inverted streams, such as the one obtained when launching a
@@ -33,7 +36,17 @@ import org.apache.sshd.server.command.CommandLifecycle;
  *
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
-public interface InvertedShell extends CommandLifecycle, SessionAware {
+public interface InvertedShell
+        extends SessionHolder<ServerSession>,
+        ServerSessionHolder,
+        CommandLifecycle,
+        SessionAware {
+
+    @Override
+    default ServerSession getSession() {
+        return getServerSession();
+    }
+
     /**
      * @return The {@link ChannelSession} instance through which
      * the shell was created - may be {@code null} if shell not started yet
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShell.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShell.java
index a86eeb8..6b62cc1 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShell.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShell.java
@@ -38,14 +38,13 @@ import org.apache.sshd.server.Environment;
 import org.apache.sshd.server.channel.ChannelSession;
 import org.apache.sshd.server.channel.PuttyRequestHandler;
 import org.apache.sshd.server.session.ServerSession;
-import org.apache.sshd.server.session.ServerSessionHolder;
 
 /**
  * Bridges the I/O streams between the SSH command and the process that executes it
  *
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
-public class ProcessShell extends AbstractLoggingBean implements InvertedShell, ServerSessionHolder {
+public class ProcessShell extends AbstractLoggingBean implements InvertedShell {
     private final List<String> command;
     private String cmdValue;
     private ServerSession session;
@@ -65,7 +64,8 @@ public class ProcessShell extends AbstractLoggingBean implements InvertedShell,
 
     public ProcessShell(Collection<String> command) {
         // we copy the original list so as not to change it
-        this.command = new ArrayList<>(ValidateUtils.checkNotNullAndNotEmpty(command, "No process shell command(s)"));
+        this.command = new ArrayList<>(
+            ValidateUtils.checkNotNullAndNotEmpty(command, "No process shell command(s)"));
         this.cmdValue = GenericUtils.join(command, ' ');
     }
 
@@ -105,16 +105,17 @@ public class ProcessShell extends AbstractLoggingBean implements InvertedShell,
                 Map<String, String> procEnv = builder.environment();
                 procEnv.putAll(varsMap);
             } catch (Exception e) {
-                log.warn("start() - Failed ({}) to set environment for command={}: {}",
-                         e.getClass().getSimpleName(), cmdValue, e.getMessage());
+                log.warn("start({}) - Failed ({}) to set environment for command={}: {}",
+                     channel, e.getClass().getSimpleName(), cmdValue, e.getMessage());
                 if (log.isDebugEnabled()) {
-                    log.debug("start(" + cmdValue + ") failure details", e);
+                    log.debug("start(" + channel + ")[" + cmdValue + "] failure details", e);
                 }
             }
         }
 
         if (log.isDebugEnabled()) {
-            log.debug("Starting shell with command: '{}' and env: {}", builder.command(), builder.environment());
+            log.debug("start({}): command='{}', env={}",
+                channel, builder.command(), builder.environment());
         }
 
         process = builder.start();
@@ -177,7 +178,7 @@ public class ProcessShell extends AbstractLoggingBean implements InvertedShell,
         boolean debugEnabled = log.isDebugEnabled();
         if (process != null) {
             if (debugEnabled) {
-                log.debug("destroy({}) Destroy process for {}", channel, cmdValue);
+                log.debug("destroy({}) Destroy process for '{}'", channel, cmdValue);
             }
             process.destroy();
         }
@@ -186,14 +187,16 @@ public class ProcessShell extends AbstractLoggingBean implements InvertedShell,
             IoUtils.closeQuietly(getInputStream(), getOutputStream(), getErrorStream());
         if (e != null) {
             if (debugEnabled) {
-                log.debug(e.getClass().getSimpleName() + " while destroy streams of '" + this + "': " + e.getMessage());
+                log.debug("destroy({}) {} while destroy streams of '{}': {}",
+                    channel, e.getClass().getSimpleName(), this, e.getMessage());
             }
 
             if (log.isTraceEnabled()) {
                 Throwable[] suppressed = e.getSuppressed();
                 if (GenericUtils.length(suppressed) > 0) {
                     for (Throwable t : suppressed) {
-                        log.trace("Suppressed " + t.getClass().getSimpleName() + ") while destroy streams of '" + this + "': " + t.getMessage());
+                        log.trace("destroy({}) Suppressed {} while destroy streams of '{}': {}",
+                            channel, t.getClass().getSimpleName(), this, t.getMessage());
                     }
                 }
             }
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java b/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java
index 7e6aaac..fca8c99 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java
@@ -79,14 +79,20 @@ public class InvertedShellWrapperTest extends BaseTestSupport {
 
     @Test   // see SSHD-570
     public void testExceptionWhilePumpStreams() throws Exception {
-        final BogusInvertedShell bogusShell = newShell("out", "err");
+        BogusInvertedShell bogusShell = newShell("out", "err");
         bogusShell.setAlive(false);
 
         final int destroyedExitValue = 7365;
+        @SuppressWarnings("checkstyle:anoninnerlength")
         InvertedShell shell = new InvertedShell() {
             private boolean destroyed;
 
             @Override
+            public ServerSession getServerSession() {
+                return bogusShell.getServerSession();
+            }
+
+            @Override
             public ChannelSession getChannelSession() {
                 return bogusShell.getChannelSession();
             }
@@ -188,7 +194,9 @@ public class InvertedShellWrapperTest extends BaseTestSupport {
              InputStream shellErr = newDelayedInputStream(Short.SIZE, errorContent);
              ByteArrayOutputStream stderr = new ByteArrayOutputStream(errorContent.length() + Byte.SIZE)) {
 
+            @SuppressWarnings("checkstyle:anoninnerlength")
             InvertedShell shell = new InvertedShell() {
+                private ServerSession session;
                 private ChannelSession channel;
 
                 @Override
@@ -197,13 +205,23 @@ public class InvertedShellWrapperTest extends BaseTestSupport {
                 }
 
                 @Override
+                public ServerSession getServerSession() {
+                    if (session != null) {
+                        return session;
+                    }
+
+                    ChannelSession channel = getChannelSession();
+                    return (channel == null) ? null : channel.getServerSession();
+                }
+
+                @Override
                 public ChannelSession getChannelSession() {
                     return channel;
                 }
 
                 @Override
                 public void setSession(ServerSession session) {
-                    // ignored
+                    this.session = session;
                 }
 
                 @Override
@@ -286,8 +304,10 @@ public class InvertedShellWrapperTest extends BaseTestSupport {
 
     private static BogusInvertedShell newShell(String contentOut, String contentErr) {
         ByteArrayOutputStream in = new ByteArrayOutputStream(20);
-        ByteArrayInputStream out = new ByteArrayInputStream(contentOut.getBytes(StandardCharsets.UTF_8));
-        ByteArrayInputStream err = new ByteArrayInputStream(contentErr.getBytes(StandardCharsets.UTF_8));
+        ByteArrayInputStream out =
+            new ByteArrayInputStream(contentOut.getBytes(StandardCharsets.UTF_8));
+        ByteArrayInputStream err =
+            new ByteArrayInputStream(contentErr.getBytes(StandardCharsets.UTF_8));
         return new BogusInvertedShell(in, out, err);
     }
 }
diff --git a/sshd-core/src/test/java/org/apache/sshd/util/test/BogusInvertedShell.java b/sshd-core/src/test/java/org/apache/sshd/util/test/BogusInvertedShell.java
index 6670529..ce5ccdc 100644
--- a/sshd-core/src/test/java/org/apache/sshd/util/test/BogusInvertedShell.java
+++ b/sshd-core/src/test/java/org/apache/sshd/util/test/BogusInvertedShell.java
@@ -28,11 +28,9 @@ import org.apache.sshd.common.util.io.IoUtils;
 import org.apache.sshd.server.Environment;
 import org.apache.sshd.server.channel.ChannelSession;
 import org.apache.sshd.server.session.ServerSession;
-import org.apache.sshd.server.session.ServerSessionHolder;
 import org.apache.sshd.server.shell.InvertedShell;
 
-public class BogusInvertedShell implements InvertedShell, ServerSessionHolder {
-
+public class BogusInvertedShell implements InvertedShell {
     private final OutputStream in;
     private final InputStream out;
     private final InputStream err;
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemEnvironment.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemEnvironment.java
index a7a12b1..cebe9cf 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemEnvironment.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemEnvironment.java
@@ -23,14 +23,16 @@ import java.nio.file.Path;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
+import org.apache.sshd.common.session.SessionHolder;
 import org.apache.sshd.common.subsystem.sftp.SftpConstants;
 import org.apache.sshd.server.config.SshServerConfigFileReader;
+import org.apache.sshd.server.session.ServerSession;
 import org.apache.sshd.server.session.ServerSessionHolder;
 
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
-public interface SftpSubsystemEnvironment extends ServerSessionHolder {
+public interface SftpSubsystemEnvironment extends SessionHolder<ServerSession>, ServerSessionHolder {
     /**
      * Force the use of a given sftp version
      */
@@ -44,6 +46,11 @@ public interface SftpSubsystemEnvironment extends ServerSessionHolder {
             .mapToObj(Integer::toString)
             .collect(Collectors.joining(","));
 
+    @Override
+    default ServerSession getSession() {
+        return getServerSession();
+    }
+
     /**
      * @return The negotiated version
      */