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 1c127dc976..133d73a235 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.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 Apache MINA SSHD Project */ public class TtyFilterOutputStream extends FilterOutputStream { public static final Set 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 ttyOptions; - private final TtyFilterInputStream echo; - public TtyFilterOutputStream(OutputStream out, TtyFilterInputStream echo, Map modes) { - this(out, echo, PtyMode.resolveEnabledOptions(modes, OUTPUT_OPTIONS)); + public TtyFilterOutputStream(OutputStream out, Map modes) { + this(out, PtyMode.resolveEnabledOptions(modes, OUTPUT_OPTIONS)); } - public TtyFilterOutputStream(OutputStream out, TtyFilterInputStream echo, Collection ttyOptions) { + public TtyFilterOutputStream(OutputStream out, Collection 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 @@ protected void handleLF() throws IOException { 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 void write(byte[] b, int off, int len) throws IOException { 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 f57f6bcb1e..12e49e7c13 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 void write(int b) throws IOException { } }; 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 496ab0a9f6..df57b76fc5 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 void start(ChannelSession channel, Environment env) throws IOException { Map 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 resolveShellEnvironment(Map 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 0000000000..75b9b8b38d --- /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. + *

+ * 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. + *

+ *

+ * 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. + *

+ *

+ * 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. + *

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