Fix incorrect behavior of ReadOnlyByteBufferBuf.getBytes(int,ByteBuffer) (#9125)

* Fix incorrect behavior of ReadOnlyByteBufferBuf.getBytes(int,ByteBuffer)

Motivation

It currently will succeed when the destination is larger than the source
range, but the ByteBuf javadoc states this should be a failure, as is
the case with all the other implementations.

Modifications

- Fix logic to fail the bounds check in this case
- Remove explicit null check which isn't done in any equivalent method
- Add unit test

Result

More correct/consistent behaviour
This commit is contained in:
Nick Hill 2019-05-12 22:00:06 -07:00 committed by Norman Maurer
parent debc1cbc0a
commit 5e786e549e
3 changed files with 16 additions and 18 deletions

View File

@ -15,8 +15,6 @@
*/
package io.netty.buffer;
import static java.util.Objects.requireNonNull;
import io.netty.util.internal.StringUtil;
import java.io.IOException;
@ -209,12 +207,10 @@ class ReadOnlyByteBufferBuf extends AbstractReferenceCountedByteBuf {
@Override
public ByteBuf getBytes(int index, ByteBuffer dst) {
checkIndex(index);
requireNonNull(dst, "dst");
checkIndex(index, dst.remaining());
int bytesToCopy = Math.min(capacity() - index, dst.remaining());
ByteBuffer tmpBuf = internalNioBuffer();
tmpBuf.clear().position(index).limit(index + bytesToCopy);
tmpBuf.clear().position(index).limit(index + dst.remaining());
dst.put(tmpBuf);
return this;
}

View File

@ -94,18 +94,6 @@ final class ReadOnlyUnsafeDirectByteBuf extends ReadOnlyByteBufferBuf {
return this;
}
@Override
public ByteBuf getBytes(int index, ByteBuffer dst) {
checkIndex(index);
requireNonNull(dst, "dst");
int bytesToCopy = Math.min(capacity() - index, dst.remaining());
ByteBuffer tmpBuf = internalNioBuffer();
tmpBuf.clear().position(index).limit(index + bytesToCopy);
dst.put(tmpBuf);
return this;
}
@Override
public ByteBuf copy(int index, int length) {
checkIndex(index, length);

View File

@ -236,6 +236,20 @@ public class ReadOnlyDirectByteBufferBufTest {
buf.release();
}
@Test(expected = IndexOutOfBoundsException.class)
public void testGetBytesByteBuffer() {
byte[] bytes = {'a', 'b', 'c', 'd', 'e', 'f', 'g'};
// Ensure destination buffer is bigger then what is in the ByteBuf.
ByteBuffer nioBuffer = ByteBuffer.allocate(bytes.length + 1);
ByteBuf buffer = buffer(((ByteBuffer) allocate(bytes.length)
.put(bytes).flip()).asReadOnlyBuffer());
try {
buffer.getBytes(buffer.readerIndex(), nioBuffer);
} finally {
buffer.release();
}
}
@Test
public void testCopy() {
ByteBuf buf = buffer(((ByteBuffer) allocate(16).putLong(1).putLong(2).flip()).asReadOnlyBuffer());