[ 
https://issues.apache.org/jira/browse/COMPRESS-595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17439073#comment-17439073
 ] 

NP edited comment on COMPRESS-595 at 11/5/21, 8:26 AM:
-------------------------------------------------------

I could, but I won't spend anytime producing a PR unless I get a hint that the 
bug is confirmed.

Partial reads are not handled correctly. And the test suite did not catch it 
because the In Memory byte channel implementation never does partial reads.

If the description of the bug is not enough for anyone to understand what I'm 
going on about, I need to fix that first...

 


was (Author: nervy_protozoan):
I could, but I won't spend anytime producing a PR unless I get a hint that the 
bug is confirmed.

Partial reads are not handled correctly. And the test suite did not catch it 
because the In Memory byte channel implementation never does partial reads.

> 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)

Reply via email to