diff --git a/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java b/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java index 7ab31681f1..c87c3a2bfe 100644 --- a/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java @@ -28,6 +28,8 @@ import java.nio.channels.GatheringByteChannel; import java.nio.channels.ScatteringByteChannel; import java.nio.charset.Charset; +import static io.netty.util.internal.MathUtil.isOutOfBounds; + /** * A derived buffer which exposes its parent's sub-region only. It is * recommended to use {@link ByteBuf#slice()} and @@ -45,7 +47,7 @@ public class SlicedByteBuf extends AbstractDerivedByteBuf { public SlicedByteBuf(ByteBuf buffer, int index, int length) { super(length); - if (index < 0 || index > buffer.capacity() - length) { + if (isOutOfBounds(index, length, buffer.capacity())) { throw new IndexOutOfBoundsException(buffer + ".slice(" + index + ", " + length + ')'); } diff --git a/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java b/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java index 31b8bfb784..b1f752ed19 100644 --- a/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java @@ -896,7 +896,7 @@ public class SwappedByteBuf extends ByteBuf { @Override public ByteBuf retainedSlice() { - return buf.slice().order(order); + return buf.retainedSlice().order(order); } @Override @@ -906,7 +906,7 @@ public class SwappedByteBuf extends ByteBuf { @Override public ByteBuf retainedSlice(int index, int length) { - return buf.slice(index, length).order(order); + return buf.retainedSlice(index, length).order(order); } @Override diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java index 6946b9d151..37c60c91c9 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java @@ -1745,6 +1745,22 @@ public abstract class AbstractByteBufTest { assertEquals(buffer.capacity() - 2, buffer.slice(1, buffer.capacity() - 2).writerIndex()); } + @Test + public void testRetainedSliceIndex() throws Exception { + assertEqualsAndRelease(buffer, 0, buffer.retainedSlice(0, buffer.capacity()).readerIndex()); + assertEqualsAndRelease(buffer, 0, buffer.retainedSlice(0, buffer.capacity() - 1).readerIndex()); + assertEqualsAndRelease(buffer, 0, buffer.retainedSlice(1, buffer.capacity() - 1).readerIndex()); + assertEqualsAndRelease(buffer, 0, buffer.retainedSlice(1, buffer.capacity() - 2).readerIndex()); + + assertEqualsAndRelease(buffer, buffer.capacity(), buffer.retainedSlice(0, buffer.capacity()).writerIndex()); + assertEqualsAndRelease(buffer, + buffer.capacity() - 1, buffer.retainedSlice(0, buffer.capacity() - 1).writerIndex()); + assertEqualsAndRelease(buffer, + buffer.capacity() - 1, buffer.retainedSlice(1, buffer.capacity() - 1).writerIndex()); + assertEqualsAndRelease(buffer, + buffer.capacity() - 2, buffer.retainedSlice(1, buffer.capacity() - 2).writerIndex()); + } + @Test @SuppressWarnings("ObjectEqualsNull") public void testEquals() { @@ -1801,6 +1817,9 @@ public abstract class AbstractByteBufTest { assertTrue(buffer.compareTo(wrappedBuffer(value, 0, 31).order(LITTLE_ENDIAN)) > 0); assertTrue(buffer.slice(0, 31).compareTo(wrappedBuffer(value)) < 0); assertTrue(buffer.slice(0, 31).compareTo(wrappedBuffer(value).order(LITTLE_ENDIAN)) < 0); + assertTrueAndRelease(buffer, buffer.retainedSlice(0, 31).compareTo(wrappedBuffer(value)) < 0); + assertTrueAndRelease(buffer, + buffer.retainedSlice(0, 31).compareTo(wrappedBuffer(value).order(LITTLE_ENDIAN)) < 0); } @Test @@ -2839,6 +2858,123 @@ public abstract class AbstractByteBufTest { assertEquals(0, buf.refCnt()); } + @Test(expected = IndexOutOfBoundsException.class) + public void testRetainedSliceIndexOutOfBounds() { + testSliceOutOfBounds(true, true, true); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testRetainedSliceLengthOutOfBounds() { + testSliceOutOfBounds(true, true, false); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testMixedSliceAIndexOutOfBounds() { + testSliceOutOfBounds(true, false, true); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testMixedSliceALengthOutOfBounds() { + testSliceOutOfBounds(true, false, false); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testMixedSliceBIndexOutOfBounds() { + testSliceOutOfBounds(false, true, true); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testMixedSliceBLengthOutOfBounds() { + testSliceOutOfBounds(false, true, false); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testSliceIndexOutOfBounds() { + testSliceOutOfBounds(false, false, true); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testSliceLengthOutOfBounds() { + testSliceOutOfBounds(false, false, false); + } + + @Test + public void testRetainedSliceContents() { + testSliceContents(true); + } + + @Test + public void testSliceContents() { + testSliceContents(false); + } + + @Test + public void testRetainedDuplicateContents() { + testDuplicateContents(true); + } + + @Test + public void testDuplicateContents() { + testDuplicateContents(false); + } + + private void testSliceOutOfBounds(boolean initRetainedSlice, boolean finalRetainedSlice, boolean indexOutOfBounds) { + ByteBuf buf = releaseLater(newBuffer(8)); + ByteBuf slice = initRetainedSlice ? buf.retainedSlice(buf.readerIndex() + 1, 2) + : buf.slice(buf.readerIndex() + 1, 2); + try { + assertEquals(2, slice.capacity()); + assertEquals(2, slice.maxCapacity()); + final int index = indexOutOfBounds ? 3 : 0; + final int length = indexOutOfBounds ? 0 : 3; + if (finalRetainedSlice) { + // This is expected to fail ... so no need to release. + slice.retainedSlice(index, length); + } else { + slice.slice(index, length); + } + } finally { + if (initRetainedSlice) { + slice.release(); + } + } + } + + private void testSliceContents(boolean retainedSlice) { + ByteBuf buf = releaseLater(newBuffer(8)).resetWriterIndex(); + ByteBuf expected = releaseLater(newBuffer(3)).resetWriterIndex(); + buf.writeBytes(new byte[] {1, 2, 3, 4, 5, 6, 7, 8}); + expected.writeBytes(new byte[] {4, 5, 6}); + ByteBuf slice = retainedSlice ? buf.retainedSlice(buf.readerIndex() + 3, 3) + : buf.slice(buf.readerIndex() + 3, 3); + try { + assertEquals(0, slice.compareTo(expected)); + assertEquals(0, slice.compareTo(slice.duplicate())); + assertEquals(0, slice.compareTo(releaseLater(slice.retainedDuplicate()))); + assertEquals(0, slice.compareTo(slice.slice(0, slice.capacity()))); + } finally { + if (retainedSlice) { + slice.release(); + } + } + } + + private void testDuplicateContents(boolean retainedDuplicate) { + ByteBuf buf = releaseLater(newBuffer(8)).resetWriterIndex(); + buf.writeBytes(new byte[] {1, 2, 3, 4, 5, 6, 7, 8}); + ByteBuf dup = retainedDuplicate ? buf.retainedDuplicate() : buf.duplicate(); + try { + assertEquals(0, dup.compareTo(buf)); + assertEquals(0, dup.compareTo(dup.duplicate())); + assertEquals(0, dup.compareTo(releaseLater(dup.retainedDuplicate()))); + assertEquals(0, dup.compareTo(dup.slice(0, dup.capacity()))); + } finally { + if (retainedDuplicate) { + dup.release(); + } + } + } + @Test public void testDuplicateRelease() { ByteBuf buf = newBuffer(8); @@ -2996,6 +3132,22 @@ public abstract class AbstractByteBufTest { assertEquals(0, buffer2.refCnt()); } + private static void assertTrueAndRelease(ByteBuf buf, boolean actual) { + try { + assertTrue(actual); + } finally { + buf.release(); + } + } + + private static void assertEqualsAndRelease(ByteBuf buf, int expected, int actual) { + try { + assertEquals(expected, actual); + } finally { + buf.release(); + } + } + private void testRefCnt0(final boolean parameter) throws Exception { for (int i = 0; i < 10; i++) { final CountDownLatch latch = new CountDownLatch(1);