From bb0b86ce50073a7de7f8f5918774b7c76bf4f99f Mon Sep 17 00:00:00 2001 From: Louis Ryan Date: Fri, 26 Jun 2015 14:43:16 -0700 Subject: [PATCH] Fix FixedCompositeByteBuf handling when copying to direct buffers and streams Motivation: FixedCompositeByteBuf does not properly implement a number of methods for copying its content to direct buffers and output streams Modifications: Replace improper use of capacity() with readableBytes() when computing offesets during writes Result: Copying works correctly --- .../netty/buffer/FixedCompositeByteBuf.java | 8 +-- .../buffer/FixedCompositeByteBufTest.java | 65 ++++++++++++++++++- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/FixedCompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/FixedCompositeByteBuf.java index 816c0b41de..af28d682d8 100644 --- a/buffer/src/main/java/io/netty/buffer/FixedCompositeByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/FixedCompositeByteBuf.java @@ -343,7 +343,7 @@ final class FixedCompositeByteBuf extends AbstractReferenceCountedByteBuf { int adjustment = c.offset; ByteBuf s = c.buf; for (;;) { - int localLength = Math.min(length, s.capacity() - (index - adjustment)); + int localLength = Math.min(length, s.readableBytes() - (index - adjustment)); dst.limit(dst.position() + localLength); s.getBytes(index - adjustment, dst); index += localLength; @@ -372,7 +372,7 @@ final class FixedCompositeByteBuf extends AbstractReferenceCountedByteBuf { int adjustment = c.offset; ByteBuf s = c.buf; for (;;) { - int localLength = Math.min(length, s.capacity() - (index - adjustment)); + int localLength = Math.min(length, s.readableBytes() - (index - adjustment)); s.getBytes(index - adjustment, dst, dstIndex, localLength); index += localLength; dstIndex += localLength; @@ -414,7 +414,7 @@ final class FixedCompositeByteBuf extends AbstractReferenceCountedByteBuf { int adjustment = c.offset; ByteBuf s = c.buf; for (;;) { - int localLength = Math.min(length, s.capacity() - (index - adjustment)); + int localLength = Math.min(length, s.readableBytes() - (index - adjustment)); s.getBytes(index - adjustment, out, localLength); index += localLength; length -= localLength; @@ -491,7 +491,7 @@ final class FixedCompositeByteBuf extends AbstractReferenceCountedByteBuf { int adjustment = c.offset; ByteBuf s = c.buf; for (;;) { - int localLength = Math.min(length, s.capacity() - (index - adjustment)); + int localLength = Math.min(length, s.readableBytes() - (index - adjustment)); switch (s.nioBufferCount()) { case 0: throw new UnsupportedOperationException(); diff --git a/buffer/src/test/java/io/netty/buffer/FixedCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/FixedCompositeByteBufTest.java index 03857551de..83396883d1 100644 --- a/buffer/src/test/java/io/netty/buffer/FixedCompositeByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/FixedCompositeByteBufTest.java @@ -26,11 +26,14 @@ import java.nio.ReadOnlyBufferException; import java.nio.channels.ScatteringByteChannel; import java.util.ArrayDeque; import java.util.Queue; +import java.nio.charset.Charset; import static io.netty.buffer.Unpooled.buffer; import static io.netty.buffer.Unpooled.compositeBuffer; import static io.netty.buffer.Unpooled.directBuffer; -import static org.junit.Assert.assertArrayEquals; +import static io.netty.buffer.Unpooled.unmodifiableBuffer; + +import static org.junit.Assert.*; public class FixedCompositeByteBufTest { @@ -276,4 +279,64 @@ public class FixedCompositeByteBufTest { buf.release(); } + @Test + public void testCopyingToOtherBuffer() { + ByteBuf buf1 = directBuffer(10); + ByteBuf buf2 = buffer(10); + ByteBuf buf3 = directBuffer(10); + buf1.writeBytes("a".getBytes(Charset.defaultCharset())); + buf2.writeBytes("b".getBytes(Charset.defaultCharset())); + buf3.writeBytes("c".getBytes(Charset.defaultCharset())); + ByteBuf composite = unmodifiableBuffer(buf1, buf2, buf3); + ByteBuf copy = directBuffer(3); + ByteBuf copy2 = buffer(3); + copy.setBytes(0, composite, 0, 3); + copy2.setBytes(0, composite, 0, 3); + copy.writerIndex(3); + copy2.writerIndex(3); + assertEquals(0, ByteBufUtil.compare(copy, composite)); + assertEquals(0, ByteBufUtil.compare(copy2, composite)); + assertEquals(0, ByteBufUtil.compare(copy, copy2)); + copy.release(); + copy2.release(); + composite.release(); + } + + @Test + public void testCopyingToOutputStream() throws IOException { + ByteBuf buf1 = directBuffer(10); + ByteBuf buf2 = buffer(10); + ByteBuf buf3 = directBuffer(10); + buf1.writeBytes("a".getBytes(Charset.defaultCharset())); + buf2.writeBytes("b".getBytes(Charset.defaultCharset())); + buf3.writeBytes("c".getBytes(Charset.defaultCharset())); + ByteBuf composite = unmodifiableBuffer(buf1, buf2, buf3); + ByteBuf copy = directBuffer(3); + ByteBuf copy2 = buffer(3); + composite.getBytes(0, new ByteBufOutputStream(copy), 3); + composite.getBytes(0, new ByteBufOutputStream(copy2), 3); + assertEquals(0, ByteBufUtil.compare(copy, composite)); + assertEquals(0, ByteBufUtil.compare(copy2, composite)); + assertEquals(0, ByteBufUtil.compare(copy, copy2)); + copy.release(); + copy2.release(); + composite.release(); + } + + @Test + public void testExtractNioBuffers() { + ByteBuf buf1 = directBuffer(10); + ByteBuf buf2 = buffer(10); + ByteBuf buf3 = directBuffer(10); + buf1.writeBytes("a".getBytes(Charset.defaultCharset())); + buf2.writeBytes("b".getBytes(Charset.defaultCharset())); + buf3.writeBytes("c".getBytes(Charset.defaultCharset())); + ByteBuf composite = unmodifiableBuffer(buf1, buf2, buf3); + ByteBuffer[] byteBuffers = composite.nioBuffers(0, 3); + assertEquals(3, byteBuffers.length); + assertEquals(1, byteBuffers[0].limit()); + assertEquals(1, byteBuffers[1].limit()); + assertEquals(1, byteBuffers[2].limit()); + composite.release(); + } }