From 4144017b7547ed64a02426cc5cb5d88fdd632a82 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Wed, 27 Aug 2008 12:21:04 +0000 Subject: [PATCH] * Fixed a bug where CompositeChannelBuffer.order() returns a wrong order * Fixed a bug where ChannelBuffer.wrappedBuffer() can generate a non-EMPTY_BUFFER when the buffer is actually empty --- .../jboss/netty/buffer/ChannelBuffers.java | 2 +- .../netty/buffer/CompositeChannelBuffer.java | 18 +- .../BigEndianCompositeChannelBufferTest.java | 10 + .../buffer/ChannelBufferIndexFinderTest.java | 23 +- .../netty/buffer/ChannelBuffersTest.java | 205 +++++++++++++++++- ...ittleEndianCompositeChannelBufferTest.java | 10 + 6 files changed, 260 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jboss/netty/buffer/ChannelBuffers.java b/src/main/java/org/jboss/netty/buffer/ChannelBuffers.java index 50031bf5de..b3239b5cb3 100644 --- a/src/main/java/org/jboss/netty/buffer/ChannelBuffers.java +++ b/src/main/java/org/jboss/netty/buffer/ChannelBuffers.java @@ -353,7 +353,7 @@ public class ChannelBuffers { break; default: for (ChannelBuffer b: buffers) { - if (b.capacity() != 0) { + if (b.readable()) { return new CompositeChannelBuffer(buffers); } } diff --git a/src/main/java/org/jboss/netty/buffer/CompositeChannelBuffer.java b/src/main/java/org/jboss/netty/buffer/CompositeChannelBuffer.java index d6f6aa184e..065985ea69 100644 --- a/src/main/java/org/jboss/netty/buffer/CompositeChannelBuffer.java +++ b/src/main/java/org/jboss/netty/buffer/CompositeChannelBuffer.java @@ -47,6 +47,7 @@ import java.util.List; public class CompositeChannelBuffer extends AbstractChannelBuffer { private final ChannelBuffer[] slices; + private final ByteOrder order; private final int[] indices; private int lastSliceId; @@ -66,6 +67,7 @@ public class CompositeChannelBuffer extends AbstractChannelBuffer { throw new IllegalArgumentException("buffers have only empty buffers."); } + order = expectedEndianness; slices = new ChannelBuffer[buffers.length]; for (int i = 0; i < buffers.length; i ++) { if (buffers[i].capacity() != 0 && buffers[i].order() != expectedEndianness) { @@ -82,13 +84,14 @@ public class CompositeChannelBuffer extends AbstractChannelBuffer { } private CompositeChannelBuffer(CompositeChannelBuffer buffer) { + order = buffer.order; slices = buffer.slices.clone(); indices = buffer.indices.clone(); setIndex(buffer.readerIndex(), buffer.writerIndex()); } public ByteOrder order() { - return slices[0].order(); + return order; } public int capacity() { @@ -453,10 +456,19 @@ public class CompositeChannelBuffer extends AbstractChannelBuffer { } public ChannelBuffer slice(int index, int length) { - if (length == 0) { + if (index == 0) { + if (length == 0) { + return ChannelBuffers.EMPTY_BUFFER; + } else { + return new TruncatedChannelBuffer(this, length); + } + } else if (index < 0 || index > capacity() - length) { + throw new IndexOutOfBoundsException(); + } else if (length == 0) { return ChannelBuffers.EMPTY_BUFFER; + } else { + return new SlicedChannelBuffer(this, index, length); } - return new SlicedChannelBuffer(this, index, length); } public ByteBuffer toByteBuffer(int index, int length) { diff --git a/src/test/java/org/jboss/netty/buffer/BigEndianCompositeChannelBufferTest.java b/src/test/java/org/jboss/netty/buffer/BigEndianCompositeChannelBufferTest.java index 3d3e4f1955..52d4a160f3 100644 --- a/src/test/java/org/jboss/netty/buffer/BigEndianCompositeChannelBufferTest.java +++ b/src/test/java/org/jboss/netty/buffer/BigEndianCompositeChannelBufferTest.java @@ -42,15 +42,25 @@ public class BigEndianCompositeChannelBufferTest extends AbstractChannelBufferTe protected ChannelBuffer newBuffer(int length) { buffers = new ArrayList(); for (int i = 0; i < length; i += 10) { + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[1])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[2])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[3])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[4])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[5])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[6])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[7])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[8])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(new byte[9])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); } buffer = ChannelBuffers.wrappedBuffer(buffers.toArray(new ChannelBuffer[buffers.size()])); diff --git a/src/test/java/org/jboss/netty/buffer/ChannelBufferIndexFinderTest.java b/src/test/java/org/jboss/netty/buffer/ChannelBufferIndexFinderTest.java index 8b3c63aca7..8936e3e56c 100644 --- a/src/test/java/org/jboss/netty/buffer/ChannelBufferIndexFinderTest.java +++ b/src/test/java/org/jboss/netty/buffer/ChannelBufferIndexFinderTest.java @@ -37,11 +37,11 @@ import org.junit.Test; public class ChannelBufferIndexFinderTest { @Test - public void testOutOfTheBoxFinders() { + public void testForward() { ChannelBuffer buf = ChannelBuffers.copiedBuffer( "abc\r\n\ndef\r\rghi\n\njkl\0\0mno \t\tx", "ISO-8859-1"); - assertEquals(3, buf.indexOf(0, buf.capacity(), ChannelBufferIndexFinder.CRLF)); + assertEquals(3, buf.indexOf(Integer.MIN_VALUE, buf.capacity(), ChannelBufferIndexFinder.CRLF)); assertEquals(6, buf.indexOf(3, buf.capacity(), ChannelBufferIndexFinder.NOT_CRLF)); assertEquals(9, buf.indexOf(6, buf.capacity(), ChannelBufferIndexFinder.CR)); assertEquals(11, buf.indexOf(9, buf.capacity(), ChannelBufferIndexFinder.NOT_CR)); @@ -51,5 +51,24 @@ public class ChannelBufferIndexFinderTest { assertEquals(21, buf.indexOf(19, buf.capacity(), ChannelBufferIndexFinder.NOT_NUL)); assertEquals(24, buf.indexOf(21, buf.capacity(), ChannelBufferIndexFinder.LINEAR_WHITESPACE)); assertEquals(28, buf.indexOf(24, buf.capacity(), ChannelBufferIndexFinder.NOT_LINEAR_WHITESPACE)); + assertEquals(-1, buf.indexOf(28, buf.capacity(), ChannelBufferIndexFinder.LINEAR_WHITESPACE)); + } + + @Test + public void testBackward() { + ChannelBuffer buf = ChannelBuffers.copiedBuffer( + "abc\r\n\ndef\r\rghi\n\njkl\0\0mno \t\tx", "ISO-8859-1"); + + assertEquals(27, buf.indexOf(Integer.MAX_VALUE, 0, ChannelBufferIndexFinder.LINEAR_WHITESPACE)); + assertEquals(23, buf.indexOf(28, 0, ChannelBufferIndexFinder.NOT_LINEAR_WHITESPACE)); + assertEquals(20, buf.indexOf(24, 0, ChannelBufferIndexFinder.NUL)); + assertEquals(18, buf.indexOf(21, 0, ChannelBufferIndexFinder.NOT_NUL)); + assertEquals(15, buf.indexOf(19, 0, ChannelBufferIndexFinder.LF)); + assertEquals(13, buf.indexOf(16, 0, ChannelBufferIndexFinder.NOT_LF)); + assertEquals(10, buf.indexOf(14, 0, ChannelBufferIndexFinder.CR)); + assertEquals(8, buf.indexOf(11, 0, ChannelBufferIndexFinder.NOT_CR)); + assertEquals(5, buf.indexOf(9, 0, ChannelBufferIndexFinder.CRLF)); + assertEquals(2, buf.indexOf(6, 0, ChannelBufferIndexFinder.NOT_CRLF)); + assertEquals(-1, buf.indexOf(3, 0, ChannelBufferIndexFinder.CRLF)); } } diff --git a/src/test/java/org/jboss/netty/buffer/ChannelBuffersTest.java b/src/test/java/org/jboss/netty/buffer/ChannelBuffersTest.java index e657b40556..50771640e2 100644 --- a/src/test/java/org/jboss/netty/buffer/ChannelBuffersTest.java +++ b/src/test/java/org/jboss/netty/buffer/ChannelBuffersTest.java @@ -25,13 +25,17 @@ package org.jboss.netty.buffer; import static org.jboss.netty.buffer.ChannelBuffers.*; import static org.junit.Assert.*; +import java.io.InputStream; import java.nio.ByteBuffer; +import java.nio.channels.ScatteringByteChannel; +import java.nio.charset.UnsupportedCharsetException; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import org.easymock.classextension.EasyMock; import org.junit.Test; /** @@ -226,8 +230,205 @@ public class ChannelBuffersTest { @Test public void shouldAllowEmptyBufferToCreateCompositeBuffer() { - wrappedBuffer( + ChannelBuffer buf = wrappedBuffer( EMPTY_BUFFER, - buffer(LITTLE_ENDIAN, 16)); + wrappedBuffer(LITTLE_ENDIAN, new byte[16]), + EMPTY_BUFFER); + assertEquals(16, buf.capacity()); + } + + @Test + public void testWrappedBuffer() { + assertEquals(16, wrappedBuffer(ByteBuffer.allocateDirect(16)).capacity()); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + wrappedBuffer(new byte[][] { new byte[] { 1, 2, 3 } })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + wrappedBuffer( + new byte[] { 1 }, + new byte[] { 2 }, + new byte[] { 3 })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + wrappedBuffer(new ChannelBuffer[] { + wrappedBuffer(new byte[] { 1, 2, 3 }) + })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + wrappedBuffer( + wrappedBuffer(new byte[] { 1 }), + wrappedBuffer(new byte[] { 2 }), + wrappedBuffer(new byte[] { 3 }))); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + wrappedBuffer(new ByteBuffer[] { + ByteBuffer.wrap(new byte[] { 1, 2, 3 }) + })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + wrappedBuffer( + ByteBuffer.wrap(new byte[] { 1 }), + ByteBuffer.wrap(new byte[] { 2 }), + ByteBuffer.wrap(new byte[] { 3 }))); + } + + @Test + public void testCopiedBuffer() { + assertEquals(16, copiedBuffer(ByteBuffer.allocateDirect(16)).capacity()); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + copiedBuffer(new byte[][] { new byte[] { 1, 2, 3 } })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + copiedBuffer( + new byte[] { 1 }, + new byte[] { 2 }, + new byte[] { 3 })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + copiedBuffer(new ChannelBuffer[] { + wrappedBuffer(new byte[] { 1, 2, 3 }) + })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + copiedBuffer( + wrappedBuffer(new byte[] { 1 }), + wrappedBuffer(new byte[] { 2 }), + wrappedBuffer(new byte[] { 3 }))); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + copiedBuffer(new ByteBuffer[] { + ByteBuffer.wrap(new byte[] { 1, 2, 3 }) + })); + + assertEquals( + wrappedBuffer(new byte[] { 1, 2, 3 }), + copiedBuffer( + ByteBuffer.wrap(new byte[] { 1 }), + ByteBuffer.wrap(new byte[] { 2 }), + ByteBuffer.wrap(new byte[] { 3 }))); + + try { + copiedBuffer("", "UnsupportedCharset"); + fail(); + } catch (UnsupportedCharsetException e) { + // Expected + } + } + + @Test + public void testHexDump() { + assertEquals("", hexDump(EMPTY_BUFFER)); + + assertEquals("123456", hexDump(wrappedBuffer( + new byte[] { + 0x12, 0x34, 0x56 + }))); + + assertEquals("1234567890abcdef", hexDump(wrappedBuffer( + new byte[] { + 0x12, 0x34, 0x56, 0x78, + (byte) 0x90, (byte) 0xAB, (byte) 0xCD, (byte) 0xEF + }))); + } + + @Test + public void testSwapMedium() { + assertEquals(0x563412, swapMedium(0x123456)); + assertEquals(0x80, swapMedium(0x800000)); + } + + @Test + public void testUnmodifiableBuffer() throws Exception { + ChannelBuffer buf = unmodifiableBuffer(buffer(16)); + + try { + buf.discardReadBytes(); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setByte(0, (byte) 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setBytes(0, EMPTY_BUFFER, 0, 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setBytes(0, new byte[0], 0, 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setBytes(0, ByteBuffer.allocate(0)); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setShort(0, (short) 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setMedium(0, 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setInt(0, 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setLong(0, 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setBytes(0, EasyMock.createMock(InputStream.class), 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } + + try { + buf.setBytes(0, EasyMock.createMock(ScatteringByteChannel.class), 0); + fail(); + } catch (UnsupportedOperationException e) { + // Expected + } } } diff --git a/src/test/java/org/jboss/netty/buffer/LittleEndianCompositeChannelBufferTest.java b/src/test/java/org/jboss/netty/buffer/LittleEndianCompositeChannelBufferTest.java index 4fbd05106b..6fa3503e74 100644 --- a/src/test/java/org/jboss/netty/buffer/LittleEndianCompositeChannelBufferTest.java +++ b/src/test/java/org/jboss/netty/buffer/LittleEndianCompositeChannelBufferTest.java @@ -43,15 +43,25 @@ public class LittleEndianCompositeChannelBufferTest extends AbstractChannelBuffe protected ChannelBuffer newBuffer(int length) { buffers = new ArrayList(); for (int i = 0; i < length; i += 10) { + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[1])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[2])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[3])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[4])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[5])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[6])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[7])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[8])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); buffers.add(ChannelBuffers.wrappedBuffer(ChannelBuffers.LITTLE_ENDIAN, new byte[9])); + buffers.add(ChannelBuffers.EMPTY_BUFFER); } buffer = ChannelBuffers.wrappedBuffer(buffers.toArray(new ChannelBuffer[buffers.size()]));