Skip to content

Commit

Permalink
Fix setBytes when source is read-only ByteBuffer and target is pooled…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
Karry authored and normanmaurer committed Mar 22, 2016
1 parent 28d03ad commit 42419d9
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ static void setBytes(AbstractByteBuf buf, long addr, int index, ByteBuffer src)
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();
}
Expand Down
38 changes: 38 additions & 0 deletions buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

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 {

Expand All @@ -45,4 +47,40 @@ public void testSetBytesOnReadOnlyByteBuffer() throws Exception {
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();
}
}

}

0 comments on commit 42419d9

Please sign in to comment.