From 4af47f0ced39d86a1ef6a644e7c1506d81c0ea1b Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 10 Jul 2017 11:06:19 +0200 Subject: [PATCH] AbstractByteBuf.setCharSequence(...) must not expand buffer Motivation: AbstractByteBuf.setCharSequence(...) must not expand the buffer if not enough writable space is present in the buffer to be consistent with all the other set operations. Modifications: - Ensure we only exand the buffer on writeCharSequence(...) but not on setCharSequence(...) - Add unit tests. Result: Consistent and correct behavior. --- .../java/io/netty/buffer/AbstractByteBuf.java | 30 +++++++-- .../buffer/AbstractUnpooledSlicedByteBuf.java | 15 +---- .../io/netty/buffer/AbstractByteBufTest.java | 61 +++++++++++++++++++ .../io/netty/buffer/SlicedByteBufTest.java | 24 ++++++++ .../WrappedUnpooledUnsafeByteBufTest.java | 25 ++++++++ 5 files changed, 135 insertions(+), 20 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java index 87a512741d..add15b1890 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java @@ -669,17 +669,35 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public int setCharSequence(int index, CharSequence sequence, Charset charset) { + return setCharSequence0(index, sequence, charset, false); + } + + private int setCharSequence0(int index, CharSequence sequence, Charset charset, boolean expand) { if (charset.equals(CharsetUtil.UTF_8)) { - ensureWritable0(ByteBufUtil.utf8MaxBytes(sequence)); + int length = ByteBufUtil.utf8MaxBytes(sequence); + if (expand) { + ensureWritable0(length); + checkIndex0(index, length); + } else { + checkIndex(index, length); + } return ByteBufUtil.writeUtf8(this, index, sequence, sequence.length()); } if (charset.equals(CharsetUtil.US_ASCII) || charset.equals(CharsetUtil.ISO_8859_1)) { - int len = sequence.length(); - ensureWritable0(len); - return ByteBufUtil.writeAscii(this, index, sequence, len); + int length = sequence.length(); + if (expand) { + ensureWritable0(length); + checkIndex0(index, length); + } else { + checkIndex(index, length); + } + return ByteBufUtil.writeAscii(this, index, sequence, length); } byte[] bytes = sequence.toString().getBytes(charset); - ensureWritable0(bytes.length); + if (expand) { + ensureWritable0(bytes.length); + // setBytes(...) will take care of checking the indices. + } setBytes(index, bytes); return bytes.length; } @@ -1140,7 +1158,7 @@ public abstract class AbstractByteBuf extends ByteBuf { @Override public int writeCharSequence(CharSequence sequence, Charset charset) { - int written = setCharSequence(writerIndex, sequence, charset); + int written = setCharSequence0(writerIndex, sequence, charset, true); writerIndex += written; return written; } diff --git a/buffer/src/main/java/io/netty/buffer/AbstractUnpooledSlicedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractUnpooledSlicedByteBuf.java index 6897c18359..5c718f3446 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractUnpooledSlicedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractUnpooledSlicedByteBuf.java @@ -16,7 +16,6 @@ package io.netty.buffer; import io.netty.util.ByteProcessor; -import io.netty.util.CharsetUtil; import java.io.IOException; import java.io.InputStream; @@ -389,19 +388,7 @@ abstract class AbstractUnpooledSlicedByteBuf extends AbstractDerivedByteBuf { @Override public int setCharSequence(int index, CharSequence sequence, Charset charset) { - if (charset.equals(CharsetUtil.UTF_8)) { - checkIndex0(index, ByteBufUtil.utf8MaxBytes(sequence)); - return ByteBufUtil.writeUtf8(this, idx(index), sequence, sequence.length()); - } - if (charset.equals(CharsetUtil.US_ASCII) || charset.equals(CharsetUtil.ISO_8859_1)) { - int len = sequence.length(); - checkIndex0(index, len); - return ByteBufUtil.writeAscii(this, idx(index), sequence, len); - } - byte[] bytes = sequence.toString().getBytes(charset); - checkIndex0(index, bytes.length); - buffer.setBytes(idx(index), bytes); - return bytes.length; + return super.setCharSequence(idx(index), sequence, charset); } @Override diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java index ef9d729553..b28781fd90 100644 --- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java @@ -36,6 +36,7 @@ import java.nio.channels.FileChannel; import java.nio.channels.GatheringByteChannel; import java.nio.channels.ScatteringByteChannel; import java.nio.channels.WritableByteChannel; +import java.nio.charset.Charset; import java.util.Arrays; import java.util.HashSet; import java.util.Random; @@ -3175,6 +3176,66 @@ public abstract class AbstractByteBufTest { assertEquals(0, buf.refCnt()); } + @Test + public void testWriteUsAsciiCharSequenceExpand() { + testWriteCharSequenceExpand(CharsetUtil.US_ASCII); + } + + @Test + public void testWriteUtf8CharSequenceExpand() { + testWriteCharSequenceExpand(CharsetUtil.UTF_8); + } + + @Test + public void testWriteIso88591CharSequenceExpand() { + testWriteCharSequenceExpand(CharsetUtil.ISO_8859_1); + } + @Test + public void testWriteUtf16CharSequenceExpand() { + testWriteCharSequenceExpand(CharsetUtil.UTF_16); + } + + private void testWriteCharSequenceExpand(Charset charset) { + ByteBuf buf = newBuffer(1); + try { + int writerIndex = buf.capacity() - 1; + buf.writerIndex(writerIndex); + int written = buf.writeCharSequence("AB", charset); + assertEquals(writerIndex, buf.writerIndex() - written); + } finally { + buf.release(); + } + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testSetUsAsciiCharSequenceNoExpand() { + testSetCharSequenceNoExpand(CharsetUtil.US_ASCII); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testSetUtf8CharSequenceNoExpand() { + testSetCharSequenceNoExpand(CharsetUtil.UTF_8); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testSetIso88591CharSequenceNoExpand() { + testSetCharSequenceNoExpand(CharsetUtil.ISO_8859_1); + } + + @Test(expected = IndexOutOfBoundsException.class) + public void testSetUtf16CharSequenceNoExpand() { + testSetCharSequenceNoExpand(CharsetUtil.UTF_16); + } + + private void testSetCharSequenceNoExpand(Charset charset) { + ByteBuf buf = newBuffer(1); + try { + buf.setCharSequence(0, "AB", charset); + } finally { + buf.release(); + } + } + @Test(expected = IndexOutOfBoundsException.class) public void testRetainedSliceIndexOutOfBounds() { testSliceOutOfBounds(true, true, true); diff --git a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java index 410f21aad3..3677ee4847 100644 --- a/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java @@ -200,4 +200,28 @@ public class SlicedByteBufTest extends AbstractByteBufTest { wrappedBuffer.release(); } } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteUsAsciiCharSequenceExpand() { + super.testWriteUsAsciiCharSequenceExpand(); + } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteUtf8CharSequenceExpand() { + super.testWriteUtf8CharSequenceExpand(); + } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteIso88591CharSequenceExpand() { + super.testWriteIso88591CharSequenceExpand(); + } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteUtf16CharSequenceExpand() { + super.testWriteUtf16CharSequenceExpand(); + } } diff --git a/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java index e8c0ed0746..dec9ef32ab 100644 --- a/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java +++ b/buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java @@ -15,6 +15,7 @@ */ package io.netty.buffer; +import io.netty.util.CharsetUtil; import io.netty.util.internal.PlatformDependent; import org.junit.Assume; import org.junit.Before; @@ -124,6 +125,30 @@ public class WrappedUnpooledUnsafeByteBufTest extends BigEndianUnsafeDirectByteB super.testLittleEndianWithExpand(); } + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteUsAsciiCharSequenceExpand() { + super.testWriteUsAsciiCharSequenceExpand(); + } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteUtf8CharSequenceExpand() { + super.testWriteUtf8CharSequenceExpand(); + } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteIso88591CharSequenceExpand() { + super.testWriteIso88591CharSequenceExpand(); + } + + @Test(expected = IndexOutOfBoundsException.class) + @Override + public void testWriteUtf16CharSequenceExpand() { + super.testWriteUtf16CharSequenceExpand(); + } + @Test @Override public void testForEachByteDesc2() {