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:
parent
bcea25cc52
commit
ed931b19bb
@ -132,8 +132,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();
|
||||
@ -141,7 +140,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);
|
||||
}
|
||||
|
||||
|
@ -96,8 +96,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;
|
||||
}
|
||||
|
||||
|
@ -316,19 +316,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);
|
||||
}
|
||||
|
||||
|
@ -179,8 +179,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;
|
||||
}
|
||||
|
||||
|
@ -318,9 +318,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;
|
||||
}
|
||||
|
||||
@ -331,12 +330,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());
|
||||
}
|
||||
|
@ -2608,6 +2608,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 void testRefCnt0(final boolean parameter) throws Exception {
|
||||
for (int i = 0; i < 10; i++) {
|
||||
final CountDownLatch latch = new CountDownLatch(1);
|
||||
|
@ -19,6 +19,7 @@ import org.junit.Test;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Random;
|
||||
import java.nio.ByteBuffer;
|
||||
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
@ -184,4 +185,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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user