gnodet commented on issue #86: [bugfix] fix transfer large file bug(more than1GB) URL: https://github.com/apache/mina-sshd/pull/86#issuecomment-454307190 You're right. Here's a more complete patch. I agree with Lyor than using `compact()` internally may not be wise as it has side effects that could be problematic. The real problem is rather in `TtyFilterInputStream` which has an ever-growing internal buffer. ``` diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/NumberUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/NumberUtils.java index 113e74e0..7c71a3dd 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/NumberUtils.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/NumberUtils.java @@ -60,7 +60,7 @@ public final class NumberUtils { public static long getNextPowerOf2(long value) { long j = 1L; - while (j < value) { + while (j < value && j > 0) { j <<= 1; } return j; @@ -68,7 +68,7 @@ public final class NumberUtils { public static int getNextPowerOf2(int value) { int j = 1; - while (j < value) { + while (j < value && j > 0) { j <<= 1; } return j; diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java index 7fff5d0c..31387db4 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java @@ -518,7 +518,11 @@ public final class BufferUtils { public static int getNextPowerOf2(int value) { // for 0-7 return 8 - return (value < Byte.SIZE) ? Byte.SIZE : NumberUtils.getNextPowerOf2(value); + return (value < Byte.SIZE) + ? Byte.SIZE + : (value > (1 << 30)) + ? value + : NumberUtils.getNextPowerOf2(value); } /** diff --git a/sshd-common/src/test/java/org/apache/sshd/common/util/NumberUtilsTest.java b/sshd-common/src/test/java/org/apache/sshd/common/util/NumberUtilsTest.java index d7c2d320..9ec0937f 100644 --- a/sshd-common/src/test/java/org/apache/sshd/common/util/NumberUtilsTest.java +++ b/sshd-common/src/test/java/org/apache/sshd/common/util/NumberUtilsTest.java @@ -19,6 +19,7 @@ package org.apache.sshd.common.util; +import org.apache.sshd.common.util.buffer.ByteArrayBuffer; import org.apache.sshd.util.test.JUnitTestSupport; import org.apache.sshd.util.test.NoIoTestCase; import org.junit.FixMethodOrder; @@ -60,6 +61,11 @@ public class NumberUtilsTest extends JUnitTestSupport { } } + @Test + public void testNextPowerOf2Max() { + assertTrue(NumberUtils.getNextPowerOf2(1073741872) < 0); + } + @Test public void testToInteger() { assertNull("Unexpected null value", NumberUtils.toInteger(null)); diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/TtyFilterInputStream.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/TtyFilterInputStream.java index 9cde926d..b8948056 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/shell/TtyFilterInputStream.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/TtyFilterInputStream.java @@ -151,6 +151,7 @@ public class TtyFilterInputStream extends FilterInputStream { } if (buffer.available() == 0) { + buffer.compact(); int nb = this.in.read(b, off, len); if (nb == -1) { return nb; diff --git a/sshd-core/src/test/java/org/apache/sshd/server/shell/TtyFilterInputStreamTest.java b/sshd-core/src/test/java/org/apache/sshd/server/shell/TtyFilterInputStreamTest.java index a0226c8d..b5b9a9dd 100644 --- a/sshd-core/src/test/java/org/apache/sshd/server/shell/TtyFilterInputStreamTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/server/shell/TtyFilterInputStreamTest.java @@ -21,7 +21,9 @@ package org.apache.sshd.server.shell; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; @@ -36,6 +38,7 @@ import java.util.stream.Stream; import org.apache.sshd.common.channel.PtyMode; import org.apache.sshd.common.util.GenericUtils; +import org.apache.sshd.common.util.buffer.ByteArrayBuffer; import org.apache.sshd.common.util.io.IoUtils; import org.apache.sshd.util.test.BaseTestSupport; import org.apache.sshd.util.test.JUnit4ClassRunnerWithParametersFactory; @@ -102,6 +105,34 @@ public class TtyFilterInputStreamTest extends BaseTestSupport { } } + @Test + public void testInternalBufferSizeDoesNotGrow() throws Exception { + try (TtyFilterInputStream is = new TtyFilterInputStream(new InputStream() { + int next = 0; + @Override + public int read() { + next = (next + 1) & 0xFF; + return next; + } + }, EnumSet.of(mode))) { + Field f = is.getClass().getDeclaredField("buffer"); + f.setAccessible(true); + ByteArrayBuffer buffer = (ByteArrayBuffer) f.get(is); + + byte[] b = new byte[256]; + for (int i = 0 ; i < 10; i++) { + is.read(b, 0, b.length); + } + + int size = buffer.capacity(); + + for (int i = 0 ; i < 10; i++) { + is.read(b, 0, b.length); + } + assertEquals(size, buffer.capacity()); + } + } + private static void assertCRLFCounts(PtyMode mode, int numLines, int crCount, int lfCount) { switch(mode) { case ECHO: ```
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
