Correctly throw IndexOutOfBoundsException when dst.remaining() is too big.

Motivation:

In some ByteBuf implementations we not correctly implement getBytes(index, ByteBuffer).

Modifications:

Correct code to do what is defined in the javadocs and adding test.

Result:

Implementation works as described.
This commit is contained in:
Norman Maurer 2016-09-27 12:56:56 +02:00
parent 0b7bf49c16
commit 97bf3c0a9b
8 changed files with 69 additions and 20 deletions

View File

@ -158,8 +158,7 @@ final class PooledDirectByteBuf extends PooledByteBuf<ByteBuffer> {
}
private void getBytes(int index, ByteBuffer dst, boolean internal) {
checkIndex(index);
int bytesToCopy = Math.min(capacity() - index, dst.remaining());
checkIndex(index, dst.remaining());
ByteBuffer tmpBuf;
if (internal) {
tmpBuf = internalNioBuffer();
@ -167,7 +166,7 @@ final class PooledDirectByteBuf extends PooledByteBuf<ByteBuffer> {
tmpBuf = memory.duplicate();
}
index = idx(index);
tmpBuf.clear().position(index).limit(index + bytesToCopy);
tmpBuf.clear().position(index).limit(index + dst.remaining());
dst.put(tmpBuf);
}

View File

@ -117,8 +117,8 @@ class PooledHeapByteBuf extends PooledByteBuf<byte[]> {
@Override
public final ByteBuf getBytes(int index, ByteBuffer dst) {
checkIndex(index);
dst.put(memory, idx(index), Math.min(capacity() - index, dst.remaining()));
checkIndex(index, dst.remaining());
dst.put(memory, idx(index), dst.remaining());
return this;
}

View File

@ -341,19 +341,15 @@ public class UnpooledDirectByteBuf extends AbstractReferenceCountedByteBuf {
}
private void getBytes(int index, ByteBuffer dst, boolean internal) {
checkIndex(index);
if (dst == null) {
throw new NullPointerException("dst");
}
checkIndex(index, dst.remaining());
int bytesToCopy = Math.min(capacity() - index, dst.remaining());
ByteBuffer tmpBuf;
if (internal) {
tmpBuf = internalNioBuffer();
} else {
tmpBuf = buffer.duplicate();
}
tmpBuf.clear().position(index).limit(index + bytesToCopy);
tmpBuf.clear().position(index).limit(index + dst.remaining());
dst.put(tmpBuf);
}

View File

@ -180,8 +180,8 @@ public class UnpooledHeapByteBuf extends AbstractReferenceCountedByteBuf {
@Override
public ByteBuf getBytes(int index, ByteBuffer dst) {
ensureAccessible();
dst.put(array, index, Math.min(capacity() - index, dst.remaining()));
checkIndex(index, dst.remaining());
dst.put(array, index, dst.remaining());
return this;
}

View File

@ -517,9 +517,8 @@ final class UnsafeByteBufUtil {
}
static void getBytes(AbstractByteBuf buf, long addr, int index, ByteBuffer dst) {
buf.checkIndex(index);
int bytesToCopy = Math.min(buf.capacity() - index, dst.remaining());
if (bytesToCopy == 0) {
buf.checkIndex(index, dst.remaining());
if (dst.remaining() == 0) {
return;
}
@ -530,12 +529,12 @@ final class UnsafeByteBufUtil {
}
// Copy to direct memory
long dstAddress = PlatformDependent.directBufferAddress(dst);
PlatformDependent.copyMemory(addr, dstAddress + dst.position(), bytesToCopy);
dst.position(dst.position() + bytesToCopy);
PlatformDependent.copyMemory(addr, dstAddress + dst.position(), dst.remaining());
dst.position(dst.position() + dst.remaining());
} else if (dst.hasArray()) {
// Copy to array
PlatformDependent.copyMemory(addr, dst.array(), dst.arrayOffset() + dst.position(), bytesToCopy);
dst.position(dst.position() + bytesToCopy);
PlatformDependent.copyMemory(addr, dst.array(), dst.arrayOffset() + dst.position(), dst.remaining());
dst.position(dst.position() + dst.remaining());
} else {
dst.put(buf.nioBuffer());
}

View File

@ -3249,6 +3249,20 @@ public abstract class AbstractByteBufTest {
}
}
@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 = newBuffer(bytes.length);
try {
buffer.writeBytes(bytes);
buffer.getBytes(buffer.readerIndex(), nioBuffer);
} finally {
buffer.release();
}
}
private static void assertTrueAndRelease(ByteBuf buf, boolean actual) {
try {
assertTrue(actual);

View File

@ -19,6 +19,7 @@ import io.netty.util.internal.ThreadLocalRandom;
import org.junit.Test;
import java.io.IOException;
import java.nio.ByteBuffer;
import static org.junit.Assert.*;
@ -195,4 +196,18 @@ public class SlicedByteBufTest extends AbstractByteBufTest {
assertEquals(0, slice1.refCnt());
assertEquals(0, slice2.refCnt());
}
@Override
@Test(expected = IndexOutOfBoundsException.class)
public void testGetBytesByteBuffer() {
byte[] bytes = {'a', 'b', 'c', 'd', 'e', 'f', 'g'};
// Ensure destination buffer is bigger then what is wrapped in the ByteBuf.
ByteBuffer nioBuffer = ByteBuffer.allocate(bytes.length + 1);
ByteBuf wrappedBuffer = Unpooled.wrappedBuffer(bytes).slice(0, bytes.length - 1);
try {
wrappedBuffer.getBytes(wrappedBuffer.readerIndex(), nioBuffer);
} finally {
wrappedBuffer.release();
}
}
}

View File

@ -646,4 +646,30 @@ public class UnpooledTest {
assertEquals(0, buffer4.refCnt());
assertEquals(0, wrapped.refCnt());
}
@Test(expected = IndexOutOfBoundsException.class)
public void testGetBytesByteBuffer() {
byte[] bytes = {'a', 'b', 'c', 'd', 'e', 'f', 'g'};
// Ensure destination buffer is bigger then what is wrapped in the ByteBuf.
ByteBuffer nioBuffer = ByteBuffer.allocate(bytes.length + 1);
ByteBuf wrappedBuffer = wrappedBuffer(bytes);
try {
wrappedBuffer.getBytes(wrappedBuffer.readerIndex(), nioBuffer);
} finally {
wrappedBuffer.release();
}
}
@Test(expected = IndexOutOfBoundsException.class)
public void testGetBytesByteBuffer2() {
byte[] bytes = {'a', 'b', 'c', 'd', 'e', 'f', 'g'};
// Ensure destination buffer is bigger then what is wrapped in the ByteBuf.
ByteBuffer nioBuffer = ByteBuffer.allocate(bytes.length + 1);
ByteBuf wrappedBuffer = wrappedBuffer(bytes, 0, bytes.length);
try {
wrappedBuffer.getBytes(wrappedBuffer.readerIndex(), nioBuffer);
} finally {
wrappedBuffer.release();
}
}
}