From 42419d918d26fa33c4a000e927df32b081eb73b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karas=20Luk=C3=A1=C5=A1?= Date: Mon, 21 Mar 2016 18:48:45 +0100 Subject: [PATCH] Fix setBytes when source is read-only ByteBuffer and target is pooled buffer Motivation: The method setBytes creates temporary heap buffer when source buffer is read-only. But this temporary buffer is not used correctly and may lead to data corruption. This problem occurs when target buffer is pooled and temporary buffer arrayOffset() is not zero. Modifications: Use correct arrayOffset when calling PlatformDependent.copyMemory. Unit test was added to test this case. Result: Setting buffer content works correctly when target is pooled buffer and source is read-only ByteBuffer. --- .../io/netty/buffer/UnsafeByteBufUtil.java | 2 +- .../netty/buffer/UnsafeByteBufUtilTest.java | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java index 9cbf06959a..fd2f886f7c 100644 --- a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java +++ b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java @@ -580,7 +580,7 @@ final class UnsafeByteBufUtil { try { byte[] tmp = tmpBuf.array(); src.get(tmp, tmpBuf.arrayOffset(), length); // moves the src position too - PlatformDependent.copyMemory(tmp, 0, addr, length); + PlatformDependent.copyMemory(tmp, tmpBuf.arrayOffset(), addr, length); } finally { tmpBuf.release(); } diff --git a/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java b/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java index 679a661c1d..568700e519 100644 --- a/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java +++ b/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java @@ -21,6 +21,8 @@ import java.nio.ByteBuffer; import static io.netty.util.internal.PlatformDependent.directBufferAddress; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; public class UnsafeByteBufUtilTest { @@ -45,4 +47,40 @@ public class UnsafeByteBufUtilTest { targetBuffer.release(); } } + + @Test + public void testSetBytesOnReadOnlyByteBufferWithPooledAlloc() throws Exception { + byte[] testData = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int length = testData.length; + + ByteBuffer readOnlyBuffer = ByteBuffer.wrap(testData).asReadOnlyBuffer(); + + int pageSize = 4096; + + // create memory pool with one page + ByteBufAllocator alloc = new PooledByteBufAllocator(true, 1, 1, pageSize, 0); + UnpooledDirectByteBuf targetBuffer = new UnpooledDirectByteBuf(alloc, length, length); + + ByteBuf b1 = alloc.heapBuffer(16); + ByteBuf b2 = alloc.heapBuffer(16); + + try { + // just check that two following buffers share same array but different offset + assertEquals(b1.array().length, pageSize); + assertEquals(b1.array(), b2.array()); + assertNotEquals(b1.arrayOffset(), b2.arrayOffset()); + + UnsafeByteBufUtil.setBytes(targetBuffer, directBufferAddress(targetBuffer.nioBuffer()), 0, readOnlyBuffer); + + byte[] check = new byte[length]; + targetBuffer.getBytes(0, check, 0, length); + + assertArrayEquals("The byte array's copy does not equal the original", testData, check); + } finally { + targetBuffer.release(); + b1.release(); + b2.release(); + } + } + }