This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push:
new 5ee81f47e Fix duplicate character echoing in ProcessShell
5ee81f47e is described below
commit 5ee81f47e148649131c91127f29ff2dc26cb61f7
Author: Denys Horman <[email protected]>
AuthorDate: Sun Jan 11 18:29:04 2026 +0200
Fix duplicate character echoing in ProcessShell
- Remove redundant manual ECHO handling in TtyFilterOutputStream
- Update ProcessShell to stop passing the echo stream to
TtyFilterOutputStream
- Update TtyFilterOutputStream constructors to remove the unused echo
parameter
- Update tests to match the new TtyFilterOutputStream API
This fix prevents characters from being duplicated in interactive shell
sessions
where the underlying shell process already handles echoing.
---
.../sshd/server/shell/TtyFilterOutputStream.java | 19 +---
.../server/shell/TtyFilterOutputStreamTest.java | 2 +-
.../org/apache/sshd/server/shell/ProcessShell.java | 2 +-
.../apache/sshd/server/shell/ProcessShellTest.java | 116 +++++++++++++++++++++
4 files changed, 123 insertions(+), 16 deletions(-)
diff --git
a/sshd-common/src/main/java/org/apache/sshd/server/shell/TtyFilterOutputStream.java
b/sshd-common/src/main/java/org/apache/sshd/server/shell/TtyFilterOutputStream.java
index 1c127dc97..133d73a23 100644
---
a/sshd-common/src/main/java/org/apache/sshd/server/shell/TtyFilterOutputStream.java
+++
b/sshd-common/src/main/java/org/apache/sshd/server/shell/TtyFilterOutputStream.java
@@ -25,33 +25,30 @@ import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.Map;
-import java.util.Objects;
import java.util.Set;
import org.apache.sshd.common.channel.PtyMode;
import org.apache.sshd.common.util.GenericUtils;
/**
- * Handles the output stream while taking care of the {@link PtyMode} for CR /
LF and ECHO settings
+ * Handles the output stream while taking care of the {@link PtyMode} for CR /
LF settings
*
* @author <a href="mailto:[email protected]";>Apache MINA SSHD Project</a>
*/
public class TtyFilterOutputStream extends FilterOutputStream {
public static final Set<PtyMode> OUTPUT_OPTIONS
- = Collections.unmodifiableSet(EnumSet.of(PtyMode.ECHO,
PtyMode.INLCR, PtyMode.ICRNL, PtyMode.IGNCR));
+ = Collections.unmodifiableSet(EnumSet.of(PtyMode.INLCR,
PtyMode.ICRNL, PtyMode.IGNCR));
private final Set<PtyMode> ttyOptions;
- private final TtyFilterInputStream echo;
- public TtyFilterOutputStream(OutputStream out, TtyFilterInputStream echo,
Map<PtyMode, ?> modes) {
- this(out, echo, PtyMode.resolveEnabledOptions(modes, OUTPUT_OPTIONS));
+ public TtyFilterOutputStream(OutputStream out, Map<PtyMode, ?> modes) {
+ this(out, PtyMode.resolveEnabledOptions(modes, OUTPUT_OPTIONS));
}
- public TtyFilterOutputStream(OutputStream out, TtyFilterInputStream echo,
Collection<PtyMode> ttyOptions) {
+ public TtyFilterOutputStream(OutputStream out, Collection<PtyMode>
ttyOptions) {
super(out);
// we create a copy of the options so as to avoid concurrent
modifications
this.ttyOptions = GenericUtils.of(ttyOptions); // TODO validate
non-conflicting options
- this.echo = this.ttyOptions.contains(PtyMode.ECHO) ?
Objects.requireNonNull(echo, "No echo stream") : echo;
}
@Override
@@ -86,9 +83,6 @@ public class TtyFilterOutputStream extends FilterOutputStream
{
protected void writeRawOutput(int c) throws IOException {
this.out.write(c);
- if (ttyOptions.contains(PtyMode.ECHO)) {
- echo.write(c);
- }
}
@Override
@@ -119,8 +113,5 @@ public class TtyFilterOutputStream extends
FilterOutputStream {
protected void writeRawOutput(byte[] b, int off, int len) throws
IOException {
this.out.write(b, off, len);
- if (ttyOptions.contains(PtyMode.ECHO)) {
- echo.write(b, off, len);
- }
}
}
diff --git
a/sshd-common/src/test/java/org/apache/sshd/server/shell/TtyFilterOutputStreamTest.java
b/sshd-common/src/test/java/org/apache/sshd/server/shell/TtyFilterOutputStreamTest.java
index f57f6bcb1..12e49e7c1 100644
---
a/sshd-common/src/test/java/org/apache/sshd/server/shell/TtyFilterOutputStreamTest.java
+++
b/sshd-common/src/test/java/org/apache/sshd/server/shell/TtyFilterOutputStreamTest.java
@@ -84,7 +84,7 @@ public class TtyFilterOutputStreamTest extends
JUnitTestSupport {
}
};
TtyFilterOutputStream ttyOut = new TtyFilterOutputStream(
- output, null, PtyMode.ECHO.equals(mode) ?
Collections.emptySet() : EnumSet.of(mode));
+ output, PtyMode.ECHO.equals(mode) ?
Collections.emptySet() : EnumSet.of(mode));
Writer writer = new OutputStreamWriter(ttyOut,
StandardCharsets.UTF_8)) {
for (String l : lines) {
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 496ab0a9f..df57b76fc 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
@@ -121,7 +121,7 @@ public class ProcessShell extends AbstractLoggingBean
implements InvertedShell {
Map<PtyMode, ?> modes = resolveShellTtyOptions(env.getPtyModes());
out = new TtyFilterInputStream(process.getInputStream(), modes);
err = new TtyFilterInputStream(process.getErrorStream(), modes);
- in = new TtyFilterOutputStream(process.getOutputStream(), err, modes);
+ in = new TtyFilterOutputStream(process.getOutputStream(), modes);
}
protected Map<String, String> resolveShellEnvironment(Map<String, String>
env) {
diff --git
a/sshd-core/src/test/java/org/apache/sshd/server/shell/ProcessShellTest.java
b/sshd-core/src/test/java/org/apache/sshd/server/shell/ProcessShellTest.java
new file mode 100644
index 000000000..75b9b8b38
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/server/shell/ProcessShellTest.java
@@ -0,0 +1,116 @@
+/*
+ * 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
+ *
+ * http://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.sshd.server.shell;
+
+import java.io.ByteArrayOutputStream;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.channel.ChannelShell;
+import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.util.OsUtils;
+import org.apache.sshd.server.SshServer;
+import org.apache.sshd.util.test.BaseTestSupport;
+import org.apache.sshd.util.test.CoreTestSupportUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class ProcessShellTest extends BaseTestSupport {
+ private SshClient client;
+
+ public ProcessShellTest() {
+ super();
+ }
+
+ @BeforeEach
+ public void setUp() throws Exception {
+ client = CoreTestSupportUtils.setupTestClient(getClass());
+ client.start();
+ }
+
+ @AfterEach
+ public void tearDown() throws Exception {
+ if (client != null) {
+ client.stop();
+ }
+ }
+
+ /**
+ * Verifies that characters sent to the shell are returned exactly as they
were sent, without any modifications or
+ * redundant echoes from the server.
+ * <p>
+ * This test ensures that the SSH server does not inject extra characters
(like manual echoes) into the data stream.
+ * It verifies that characters sent to the shell are returned without any
modifications. If the server were to
+ * return modified characters or extra echoes, it would indicate a failure
in the I/O bridging logic.
+ * </p>
+ * <p>
+ * The test relies on {@code cat} as a shell because it is a simple,
standard Linux utility that acts as a
+ * transparent bridge, reading from STDIN and writing to STDOUT without
any internal processing, echoing, or
+ * modifications to the character stream. This allows the test to isolate
and verify the server's behavior without
+ * interference from the shell process itself.
+ * </p>
+ * <p>
+ * Note: This test is skipped on Windows because {@code cat} is not
natively available. Since the core logic being
+ * tested is platform-independent Java, verifying it on Unix-like systems
is sufficient.
+ * </p>
+ *
+ * @throws Exception If failed
+ */
+ @Test
+ void testNoRedundantEchoOnStderr() throws Exception {
+ if (OsUtils.isWin32()) {
+ return;
+ }
+
+ try (SshServer localSshd =
CoreTestSupportUtils.setupTestServer(getClass())) {
+ localSshd.setShellFactory(new ProcessShellFactory("cat", "cat"));
+ localSshd.start();
+
+ try (ClientSession session = client.connect(getCurrentTestName(),
"localhost", localSshd.getPort())
+ .verify(10_000).getSession()) {
+ session.addPasswordIdentity(getCurrentTestName());
+ session.auth().verify(10_000);
+
+ try (ChannelShell channel = session.createShellChannel();
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ ByteArrayOutputStream err = new ByteArrayOutputStream()) {
+ channel.setOut(out);
+ channel.setErr(err);
+ channel.open().verify(10_000);
+
+ OutputStream pipe = channel.getInvertedIn();
+ pipe.write("hello\n".getBytes(StandardCharsets.UTF_8));
+ pipe.flush();
+
+ Thread.sleep(1000);
+
+ String stderr =
err.toString(StandardCharsets.UTF_8.name());
+ assertFalse(stderr.contains("hello"), "Redundant echo
detected on stderr: " + stderr);
+
+ String stdout =
out.toString(StandardCharsets.UTF_8.name());
+ assertTrue(stdout.contains("hello"), "Output should
contain 'hello': " + stdout);
+ }
+ } finally {
+ localSshd.stop(true);
+ }
+ }
+ }
+}
