From f8ff834f037e51b7119630bdb7713a29c18b8e31 Mon Sep 17 00:00:00 2001 From: Nikolay Fedorovskikh Date: Tue, 10 Apr 2018 13:41:50 +0500 Subject: [PATCH] Checks accessibility in the #slice and #duplicate methods of ByteBuf (#7846) Motivation: The `ByteBuf#slice` and `ByteBuf#duplicate` methods should check an accessibility to prevent creation slice or duplicate of released buffer. At now this works not in the all scenarios. Modifications: Add missed checks. Result: More correct and consistent behavior of `ByteBuf` methods. --- .../java/io/netty/buffer/AbstractByteBuf.java | 1 + .../buffer/AbstractPooledDerivedByteBuf.java | 8 +- .../io/netty/buffer/AbstractByteBufTest.java | 218 ++++++++++++++++++ 3 files changed, 225 insertions(+), 2 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java index b65cb10d40..4a13d821b9 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java @@ -1172,6 +1172,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public ByteBuf duplicate() { + ensureAccessible(); return new UnpooledDuplicatedByteBuf(this); } diff --git a/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java index 80f33914d7..3388b14190 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java @@ -141,11 +141,13 @@ abstract class AbstractPooledDerivedByteBuf extends AbstractReferenceCountedByte @Override public ByteBuf slice(int index, int length) { + ensureAccessible(); // All reference count methods should be inherited from this object (this is the "parent"). return new PooledNonRetainedSlicedByteBuf(this, unwrap(), index, length); } final ByteBuf duplicate0() { + ensureAccessible(); // All reference count methods should be inherited from this object (this is the "parent"). return new PooledNonRetainedDuplicateByteBuf(this, unwrap()); } @@ -199,6 +201,7 @@ abstract class AbstractPooledDerivedByteBuf extends AbstractReferenceCountedByte @Override public ByteBuf duplicate() { + ensureAccessible(); return new PooledNonRetainedDuplicateByteBuf(referenceCountDelegate, this); } @@ -209,7 +212,7 @@ abstract class AbstractPooledDerivedByteBuf extends AbstractReferenceCountedByte @Override public ByteBuf slice(int index, int length) { - checkIndex0(index, length); + checkIndex(index, length); return new PooledNonRetainedSlicedByteBuf(referenceCountDelegate, unwrap(), index, length); } @@ -275,6 +278,7 @@ abstract class AbstractPooledDerivedByteBuf extends AbstractReferenceCountedByte @Override public ByteBuf duplicate() { + ensureAccessible(); return new PooledNonRetainedDuplicateByteBuf(referenceCountDelegate, unwrap()) .setIndex(idx(readerIndex()), idx(writerIndex())); } @@ -286,7 +290,7 @@ abstract class AbstractPooledDerivedByteBuf extends AbstractReferenceCountedByte @Override public ByteBuf slice(int index, int length) { - checkIndex0(index, length); + checkIndex(index, length); return new PooledNonRetainedSlicedByteBuf(referenceCountDelegate, unwrap(), idx(index), length); } diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java index e307f25baf..1af64a7392 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java @@ -3317,6 +3317,224 @@ public abstract class AbstractByteBufTest { } } + @Test(expected = IllegalReferenceCountException.class) + public void testSliceAfterRelease() { + releasedBuffer().slice(); + } + + @Test(expected = IllegalReferenceCountException.class) + public void testSliceAfterRelease2() { + releasedBuffer().slice(0, 1); + } + + private static void assertSliceFailAfterRelease(ByteBuf... bufs) { + for (ByteBuf buf : bufs) { + if (buf.refCnt() > 0) { + buf.release(); + } + } + for (ByteBuf buf : bufs) { + try { + assertEquals(0, buf.refCnt()); + buf.slice(); + fail(); + } catch (IllegalReferenceCountException ignored) { + // as expected + } + } + } + + @Test + public void testSliceAfterReleaseRetainedSlice() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + assertSliceFailAfterRelease(buf, buf2); + } + + @Test + public void testSliceAfterReleaseRetainedSliceDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + ByteBuf buf3 = buf2.duplicate(); + assertSliceFailAfterRelease(buf, buf2, buf3); + } + + @Test + public void testSliceAfterReleaseRetainedSliceRetainedDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + ByteBuf buf3 = buf2.retainedDuplicate(); + assertSliceFailAfterRelease(buf, buf2, buf3); + } + + @Test + public void testSliceAfterReleaseRetainedDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedDuplicate(); + assertSliceFailAfterRelease(buf, buf2); + } + + @Test + public void testSliceAfterReleaseRetainedDuplicateSlice() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedDuplicate(); + ByteBuf buf3 = buf2.slice(0, 1); + assertSliceFailAfterRelease(buf, buf2, buf3); + } + + @Test(expected = IllegalReferenceCountException.class) + public void testRetainedSliceAfterRelease() { + releasedBuffer().retainedSlice(); + } + + @Test(expected = IllegalReferenceCountException.class) + public void testRetainedSliceAfterRelease2() { + releasedBuffer().retainedSlice(0, 1); + } + + private static void assertRetainedSliceFailAfterRelease(ByteBuf... bufs) { + for (ByteBuf buf : bufs) { + if (buf.refCnt() > 0) { + buf.release(); + } + } + for (ByteBuf buf : bufs) { + try { + assertEquals(0, buf.refCnt()); + buf.retainedSlice(); + fail(); + } catch (IllegalReferenceCountException ignored) { + // as expected + } + } + } + + @Test + public void testRetainedSliceAfterReleaseRetainedSlice() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + assertRetainedSliceFailAfterRelease(buf, buf2); + } + + @Test + public void testRetainedSliceAfterReleaseRetainedSliceDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + ByteBuf buf3 = buf2.duplicate(); + assertRetainedSliceFailAfterRelease(buf, buf2, buf3); + } + + @Test + public void testRetainedSliceAfterReleaseRetainedSliceRetainedDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + ByteBuf buf3 = buf2.retainedDuplicate(); + assertRetainedSliceFailAfterRelease(buf, buf2, buf3); + } + + @Test + public void testRetainedSliceAfterReleaseRetainedDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedDuplicate(); + assertRetainedSliceFailAfterRelease(buf, buf2); + } + + @Test + public void testRetainedSliceAfterReleaseRetainedDuplicateSlice() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedDuplicate(); + ByteBuf buf3 = buf2.slice(0, 1); + assertRetainedSliceFailAfterRelease(buf, buf2, buf3); + } + + @Test(expected = IllegalReferenceCountException.class) + public void testDuplicateAfterRelease() { + releasedBuffer().duplicate(); + } + + @Test(expected = IllegalReferenceCountException.class) + public void testRetainedDuplicateAfterRelease() { + releasedBuffer().retainedDuplicate(); + } + + private static void assertDuplicateFailAfterRelease(ByteBuf... bufs) { + for (ByteBuf buf : bufs) { + if (buf.refCnt() > 0) { + buf.release(); + } + } + for (ByteBuf buf : bufs) { + try { + assertEquals(0, buf.refCnt()); + buf.duplicate(); + fail(); + } catch (IllegalReferenceCountException ignored) { + // as expected + } + } + } + + @Test + public void testDuplicateAfterReleaseRetainedSliceDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + ByteBuf buf3 = buf2.duplicate(); + assertDuplicateFailAfterRelease(buf, buf2, buf3); + } + + @Test + public void testDuplicateAfterReleaseRetainedDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedDuplicate(); + assertDuplicateFailAfterRelease(buf, buf2); + } + + @Test + public void testDuplicateAfterReleaseRetainedDuplicateSlice() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedDuplicate(); + ByteBuf buf3 = buf2.slice(0, 1); + assertDuplicateFailAfterRelease(buf, buf2, buf3); + } + + private static void assertRetainedDuplicateFailAfterRelease(ByteBuf... bufs) { + for (ByteBuf buf : bufs) { + if (buf.refCnt() > 0) { + buf.release(); + } + } + for (ByteBuf buf : bufs) { + try { + assertEquals(0, buf.refCnt()); + buf.retainedDuplicate(); + fail(); + } catch (IllegalReferenceCountException ignored) { + // as expected + } + } + } + + @Test + public void testRetainedDuplicateAfterReleaseRetainedDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedDuplicate(); + assertRetainedDuplicateFailAfterRelease(buf, buf2); + } + + @Test + public void testRetainedDuplicateAfterReleaseDuplicate() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.duplicate(); + assertRetainedDuplicateFailAfterRelease(buf, buf2); + } + + @Test + public void testRetainedDuplicateAfterReleaseRetainedSlice() { + ByteBuf buf = newBuffer(1); + ByteBuf buf2 = buf.retainedSlice(0, 1); + assertRetainedDuplicateFailAfterRelease(buf, buf2); + } + @Test public void testSliceRelease() { ByteBuf buf = newBuffer(8);