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:
parent
57862b5771
commit
de76843905
@ -22,6 +22,7 @@ import java.io.InputStream;
|
|||||||
import java.io.OutputStream;
|
import java.io.OutputStream;
|
||||||
import java.nio.ByteBuffer;
|
import java.nio.ByteBuffer;
|
||||||
import java.nio.ByteOrder;
|
import java.nio.ByteOrder;
|
||||||
|
import java.nio.ReadOnlyBufferException;
|
||||||
|
|
||||||
import static io.netty.util.internal.MathUtil.isOutOfBounds;
|
import static io.netty.util.internal.MathUtil.isOutOfBounds;
|
||||||
import static io.netty.util.internal.ObjectUtil.checkNotNull;
|
import static io.netty.util.internal.ObjectUtil.checkNotNull;
|
||||||
@ -315,15 +316,21 @@ final class UnsafeByteBufUtil {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (dst.isDirect()) {
|
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
|
// Copy to direct memory
|
||||||
long dstAddress = PlatformDependent.directBufferAddress(dst);
|
long dstAddress = PlatformDependent.directBufferAddress(dst);
|
||||||
PlatformDependent.copyMemory(addr, dstAddress + dst.position(), bytesToCopy);
|
PlatformDependent.copyMemory(addr, dstAddress + dst.position(), bytesToCopy);
|
||||||
} else {
|
dst.position(dst.position() + bytesToCopy);
|
||||||
|
} else if (dst.hasArray()) {
|
||||||
// Copy to array
|
// Copy to array
|
||||||
PlatformDependent.copyMemory(addr, dst.array(), dst.arrayOffset() + dst.position(), bytesToCopy);
|
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) {
|
static void setBytes(AbstractByteBuf buf, long addr, int index, ByteBuf src, int srcIndex, int length) {
|
||||||
|
@ -27,6 +27,7 @@ import java.io.ByteArrayInputStream;
|
|||||||
import java.io.ByteArrayOutputStream;
|
import java.io.ByteArrayOutputStream;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.nio.ByteBuffer;
|
import java.nio.ByteBuffer;
|
||||||
|
import java.nio.ReadOnlyBufferException;
|
||||||
import java.nio.channels.Channels;
|
import java.nio.channels.Channels;
|
||||||
import java.nio.channels.GatheringByteChannel;
|
import java.nio.channels.GatheringByteChannel;
|
||||||
import java.nio.channels.ScatteringByteChannel;
|
import java.nio.channels.ScatteringByteChannel;
|
||||||
@ -2495,6 +2496,33 @@ public abstract class AbstractByteBufTest {
|
|||||||
assertFalse(nioBuffers[0].hasRemaining());
|
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 {
|
private void testRefCnt0(final boolean parameter) throws Exception {
|
||||||
for (int i = 0; i < 10; i++) {
|
for (int i = 0; i < 10; i++) {
|
||||||
final CountDownLatch latch = new CountDownLatch(1);
|
final CountDownLatch latch = new CountDownLatch(1);
|
||||||
|
@ -102,6 +102,18 @@ public class SlicedByteBufTest extends AbstractByteBufTest {
|
|||||||
super.testWriteZeroAfterRelease();
|
super.testWriteZeroAfterRelease();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test(expected = IndexOutOfBoundsException.class)
|
||||||
|
@Override
|
||||||
|
public void testGetReadOnlyDirectDst() {
|
||||||
|
super.testGetReadOnlyDirectDst();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test(expected = IndexOutOfBoundsException.class)
|
||||||
|
@Override
|
||||||
|
public void testGetReadOnlyHeapDst() {
|
||||||
|
super.testGetReadOnlyHeapDst();
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@Override
|
@Override
|
||||||
public void testLittleEndianWithExpand() {
|
public void testLittleEndianWithExpand() {
|
||||||
|
Loading…
Reference in New Issue
Block a user