Skip to content

Commit

Permalink
Fix incorrect behavior of ReadOnlyByteBufferBuf.getBytes(int,ByteBuff…
Browse files Browse the repository at this point in the history
…er) (netty#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
  • Loading branch information
njhill authored and normanmaurer committed May 13, 2019
1 parent 6ee8b65 commit 60de092
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,10 @@ public ByteBuf getBytes(int index, byte[] dst, int dstIndex, int length) {

@Override
public ByteBuf getBytes(int index, ByteBuffer dst) {
checkIndex(index);
if (dst == null) {
throw new NullPointerException("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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,6 @@ public ByteBuf getBytes(int index, byte[] dst, int dstIndex, int length) {
return this;
}

@Override
public ByteBuf getBytes(int index, ByteBuffer dst) {
checkIndex(index);
if (dst == null) {
throw new NullPointerException("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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,20 @@ public void testGetReadLong() {
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());
Expand Down

0 comments on commit 60de092

Please sign in to comment.