[
https://issues.apache.org/jira/browse/COMPRESS-595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17439132#comment-17439132
]
NP commented on COMPRESS-595:
-----------------------------
I was able to make my test case pass with the following code change.
So it was not so much a problem of rewind() being called, but rather a problem
of rewind()
being called when it did not have to be (when `b.hasRemaining()` was still
true):
{code:java}
public static byte[] readRange(final ReadableByteChannel input, final int len)
throws IOException {
final ByteArrayOutputStream output = new ByteArrayOutputStream();
final ByteBuffer b = ByteBuffer.allocate(Math.min(len, COPY_BUF_SIZE));
int read = 0;
while (read < len) {
final int posBefore = b.position();
final int readNow = input.read(b);
if (readNow <= 0) {
break;
}
output.write(b.array(), posBefore, readNow);
read += readNow;
if (! b.hasRemaining()) {
b.rewind();
// Make sure we never read more than len bytes
b.limit(Math.min(len - read, b.capacity()));
}
}
return output.toByteArray();
}
{code}
I've also run the entire test suite, 100% pass. So no regression.
> IOUtils.readRange(ReadableByteChannel input, int len) reads more than len
> when input.read() returns < len (ie. rewind() makes no sense)
> ---------------------------------------------------------------------------------------------------------------------------------------
>
> Key: COMPRESS-595
> URL: https://issues.apache.org/jira/browse/COMPRESS-595
> Project: Commons Compress
> Issue Type: Bug
> Affects Versions: 1.21
> Reporter: NP
> Priority: Major
> Attachments: IOUtilsTest.kt
>
>
> When `input.read(b)` returns `readNow` < `len`, then it means
> `input.read(b)` will need to be called again with the same
> buffer, whose `remaining()` is now the old `remaining()` - `readNow`.
> This way the ReadableByteChannel knows how many bytes are to be
> read in subsequent iterations of the `while (read < len)` loop.
> This is currently not the case, because there is a call to rewind()
> which results in a buffer whose remaining() is reset to `len` if
> `readNow` < `len`.
> I suspect the readRange() method has only been used with channels that never
> do partial reads (such as File Channels), and hence the problem has not been
> experienced until now.
> Here is a test case that exhibits the bug:
>
> ```kotlin
> import org.apache.commons.compress.utils.IOUtils
> import org.junit.jupiter.api.Assertions.assertArrayEquals
> import org.junit.jupiter.api.Assertions.assertEquals
> import org.junit.jupiter.api.Test
> import java.nio.ByteBuffer
> import java.nio.channels.ReadableByteChannel
> class IOUtilsTest {
> private class ReadableBytePerByteChannel: ReadableByteChannel {
> val data = byteArrayOf(0x03, 0x02, 0x01)
> val dataBuffer = ByteBuffer.wrap(data)!!
> override fun close(): Unit = TODO("Not needed")
> override fun isOpen(): Boolean = TODO("Not needed")
> override fun read(dst: ByteBuffer): Int {
> if (! dataBuffer.hasRemaining()) return -1
> val bytesAvailableForReading = dataBuffer.remaining()
> for (bytesReadSoFar in 1..bytesAvailableForReading) {
> val nextByte: Byte = dataBuffer.get()
> dst.put(nextByte)
> // first read() call reads 1 byte, less than requested(2)
> if (0x03.toByte() == nextByte) return bytesReadSoFar
> }
> return bytesAvailableForReading
> }
> }
> @Test
> fun readRangeShouldHandlePartialReads() {
> val channel = ReadableBytePerByteChannel()
> // should result in two calls to read()
> val actual = IOUtils.readRange(channel, 2)
> val expected = byteArrayOf(0x03, 0x02)
> assertArrayEquals(expected, actual)
> assertEquals(1, channel.dataBuffer.remaining())
> }
> }
> ```
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
