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.
dschobel · Jul 19, 2017 · 4af47f0 · 4af47f0
1 parent ef22e65
commit 4af47f0
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 20 deletions.
diff --git 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 ByteBuf setZero(int index, int length) {
 
     @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 ByteBuf writeZero(int length) {
 
     @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
@@ -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 @@ public ByteBuf setBytes(int index, ByteBuffer src) {
 
     @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
@@ -36,6 +36,7 @@
 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 void testSliceRelease() {
         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
@@ -200,4 +200,28 @@ public void testGetBytesByteBuffer() {
             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
@@ -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 void testLittleEndianWithExpand() {
         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() {