Throw ReadOnlyBufferException if unsafe buffer is used and dst is direct

Motivation:

We missed to check if the dst is ready only before using unsafe to copy data into it which lead to data-corruption. We need to ensure we respect ready only ByteBuffer.

Modifications:

- Correctly check if the dst is ready only before copy data into it in UnsafeByteBufUtil
- Also make it work for buffers that are not direct and not have an array

Result:

No more data corruption possible if the dst buffer is readonly and unsafe buffer implementation is used.
This commit is contained in:
Norman Maurer 2015-12-12 19:42:45 +01:00
parent dd9fc289fd
commit 4e467f5c6f
3 changed files with 50 additions and 3 deletions

View File

@ -22,6 +22,7 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.ReadOnlyBufferException;
import static io.netty.util.internal.MathUtil.isOutOfBounds;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
@ -514,15 +515,21 @@ final class UnsafeByteBufUtil {
}
if (dst.isDirect()) {
if (dst.isReadOnly()) {
// We need to check if dst is ready-only so we not write something in it by using Unsafe.
throw new ReadOnlyBufferException();
}
// Copy to direct memory
long dstAddress = PlatformDependent.directBufferAddress(dst);
PlatformDependent.copyMemory(addr, dstAddress + dst.position(), bytesToCopy);
} else {
dst.position(dst.position() + bytesToCopy);
} else if (dst.hasArray()) {
// Copy to array
PlatformDependent.copyMemory(addr, dst.array(), dst.arrayOffset() + dst.position(), bytesToCopy);
dst.position(dst.position() + bytesToCopy);
} else {
dst.put(buf.nioBuffer());
}
dst.position(dst.position() + bytesToCopy);
}
static void setBytes(AbstractByteBuf buf, long addr, int index, ByteBuf src, int srcIndex, int length) {

View File

@ -29,6 +29,7 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.ReadOnlyBufferException;
import java.nio.channels.Channels;
import java.nio.channels.GatheringByteChannel;
import java.nio.channels.ScatteringByteChannel;
@ -2866,6 +2867,33 @@ public abstract class AbstractByteBufTest {
assertFalse(nioBuffers[0].hasRemaining());
}
@Test
public void testGetReadOnlyDirectDst() {
testGetReadOnlyDst(true);
}
@Test
public void testGetReadOnlyHeapDst() {
testGetReadOnlyDst(false);
}
private void testGetReadOnlyDst(boolean direct) {
byte[] bytes = { 'a', 'b', 'c', 'd' };
ByteBuf buffer = releaseLater(newBuffer(bytes.length));
buffer.writeBytes(bytes);
ByteBuffer dst = direct ? ByteBuffer.allocateDirect(bytes.length) : ByteBuffer.allocate(bytes.length);
ByteBuffer readOnlyDst = dst.asReadOnlyBuffer();
try {
buffer.getBytes(0, readOnlyDst);
fail();
} catch (ReadOnlyBufferException e) {
// expected
}
assertEquals(0, readOnlyDst.position());
}
private void testRefCnt0(final boolean parameter) throws Exception {
for (int i = 0; i < 10; i++) {
final CountDownLatch latch = new CountDownLatch(1);

View File

@ -102,6 +102,18 @@ public class SlicedByteBufTest extends AbstractByteBufTest {
super.testWriteZeroAfterRelease();
}
@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testGetReadOnlyDirectDst() {
super.testGetReadOnlyDirectDst();
}
@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testGetReadOnlyHeapDst() {
super.testGetReadOnlyHeapDst();
}
@Test
@Override
public void testLittleEndianWithExpand() {