GH-266: Make ChannelPipedOutputStream.flush() a no-op

ChannelPipedOutputStream passes on data immediately to its sink. Flushing such a stream is thus a no-op, and must not throw an exception even when the stream is already closed. Otherwise there may be spurious failures if the reader of the sink decides to close the whole channel before the channel has flushed the ChannelPipedOutputStream. Fixes #266. Bug: #266
apache · Nov 4, 2022 · f3a3d16 · f3a3d16
1 parent a85d074
commit f3a3d16
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 18 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -32,6 +32,7 @@
 * [SSHD-1307](https://issues.apache.org/jira/browse/SSHD-1307) [NIO2] TCP/IP port forwarding: shut down output stream only after pending writes have been written
 
 * [GH-263](https://github.com/apache/mina-sshd/issues/263)  Race condition in BufferedIoOutputStream
+* [GH-266](https://github.com/apache/mina-sshd/issues/266)  ChannelPipedOutputStream.flush() must be a no-op
 
 ## Major code re-factoring
 

diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelPipedOutputStream.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelPipedOutputStream.java
@@ -60,9 +60,8 @@ public boolean isOpen() {
 
     @Override
     public void flush() throws IOException {
-        if (!isOpen()) {
-            throw new IOException("flush() Stream has been closed");
-        }
+        // Nothing to do since we hand off all data immediately to the sink. Do not throw an exception if we're already
+        // closed; see https://github.com/apache/mina-sshd/issues/266
     }
 
     @Override

diff --git a/sshd-core/src/test/java/org/apache/sshd/common/channel/ChannelPipedOutputStreamTest.java b/sshd-core/src/test/java/org/apache/sshd/common/channel/ChannelPipedOutputStreamTest.java
@@ -72,19 +72,9 @@ public void testNioChannelImplementation() throws IOException {
             assertFalse("Stream still marked as open after close", stream.isOpen());
             assertTrue("Sink EOF not called on close", eofCalled.get());
 
-            try {
-                stream.write(b);
-                fail("Unexpected write success after close");
-            } catch (IOException e) {
-                // expected
-            }
-
-            try {
-                stream.flush();
-                fail("Unexpected flush success after close");
-            } catch (IOException e) {
-                // expected
-            }
+            assertThrows("Unexpected write success after close", IOException.class, () -> stream.write(b));
+            // flush() should not fail
+            stream.flush();
         }
     }
 }
diff --git a/sshd-scp/pom.xml b/sshd-scp/pom.xml
@@ -126,7 +126,6 @@
                     <plugin>
                         <groupId>org.apache.maven.plugins</groupId>
                         <artifactId>maven-surefire-plugin</artifactId>
-                        <version>${surefire.plugin.version}</version>
                         <executions>
                             <execution>
                                 <id>mina</id>
@@ -170,7 +169,6 @@
                     <plugin>
                         <groupId>org.apache.maven.plugins</groupId>
                         <artifactId>maven-surefire-plugin</artifactId>
-                        <version>${surefire.plugin.version}</version>
                         <executions>
                             <execution>
                                 <id>netty</id>