[#1195] Fix Unpooled.wrappedBuffer(..) with non-direct read-only ByteBuffer

buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java

@@ -28,19 +28,19 @@ import java.nio.channels.ScatteringByteChannel;
 
 
 /**
- * Read-only ByteBuf which wraps a read-only direct ByteBuffer.
+ * Read-only ByteBuf which wraps a read-only ByteBuffer.
  */
-class ReadOnlyDirectByteBuf extends AbstractReferenceCountedByteBuf {
+class ReadOnlyByteBufferBuf extends AbstractReferenceCountedByteBuf {
     private final ResourceLeak leak = leakDetector.open(this);
 
     protected final ByteBuffer buffer;
     private final ByteBufAllocator allocator;
     private ByteBuffer tmpNioBuf;
 
-    public ReadOnlyDirectByteBuf(ByteBufAllocator allocator, ByteBuffer buffer) {
+    public ReadOnlyByteBufferBuf(ByteBufAllocator allocator, ByteBuffer buffer) {
         super(buffer.remaining());
-        if (!buffer.isDirect()) {
+        if (!buffer.isReadOnly()) {
-            throw new IllegalArgumentException("must be a direct buffer: " + buffer.getClass().getSimpleName());
+            throw new IllegalArgumentException("must be a readonly buffer: " + buffer.getClass().getSimpleName());
         }
 
         this.allocator = allocator;
@@ -208,7 +208,7 @@ class ReadOnlyDirectByteBuf extends AbstractReferenceCountedByteBuf {
 
     @Override
     public boolean isDirect() {
-        return true;
+        return buffer.isDirect();
     }
 
     @Override
@@ -293,17 +293,17 @@ class ReadOnlyDirectByteBuf extends AbstractReferenceCountedByteBuf {
 
     @Override
     public boolean hasArray() {
-        return false;
+        return buffer.hasArray();
     }
 
     @Override
     public byte[] array() {
-        throw new UnsupportedOperationException("direct buffer");
+        return buffer.array();
     }
 
     @Override
     public int arrayOffset() {
-        throw new UnsupportedOperationException("direct buffer");
+        return buffer.arrayOffset();
     }
 
     @Override

buffer/src/main/java/io/netty/buffer/ReadOnlyUnsafeDirectByteBuf.java

@@ -26,7 +26,7 @@ import java.nio.ByteOrder;
 /**
  * Read-only ByteBuf which wraps a read-only direct ByteBuffer and use unsafe for best performance.
  */
-final class ReadOnlyUnsafeDirectByteBuf extends ReadOnlyDirectByteBuf {
+final class ReadOnlyUnsafeDirectByteBuf extends ReadOnlyByteBufferBuf {
     private static final boolean NATIVE_ORDER = ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN;
     private final long memoryAddress;
 

buffer/src/main/java/io/netty/buffer/Unpooled.java

@@ -224,13 +224,17 @@ public final class Unpooled {
                     buffer.remaining()).order(buffer.order());
         } else if (PlatformDependent.hasUnsafe()) {
             if (buffer.isReadOnly()) {
-                return new ReadOnlyUnsafeDirectByteBuf(ALLOC, buffer);
+                if (buffer.isDirect()) {
+                    return new ReadOnlyUnsafeDirectByteBuf(ALLOC, buffer);
+                } else {
+                    return new ReadOnlyByteBufferBuf(ALLOC, buffer);
+                }
             } else {
                 return new UnpooledUnsafeDirectByteBuf(ALLOC, buffer, buffer.remaining());
             }
         } else {
             if (buffer.isReadOnly()) {
-                return new ReadOnlyDirectByteBuf(ALLOC, buffer);
+                return new ReadOnlyByteBufferBuf(ALLOC, buffer);
             }  else {
                 return new UnpooledDirectByteBuf(ALLOC, buffer, buffer.remaining());
             }

buffer/src/test/java/io/netty/buffer/ReadOnlyByteBufferBufTest.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright 2013 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.buffer;
+
+import java.nio.ByteBuffer;
+
+public class ReadOnlyByteBufferBufTest extends ReadOnlyDirectByteBufferBufTest {
+    @Override
+    protected ByteBuffer allocate(int size) {
+        return ByteBuffer.allocate(size);
+    }
+}

buffer/src/test/java/io/netty/buffer/ReadOnlyDirectByteBufferBufTest.java

@@ -23,57 +23,61 @@ import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.ReadOnlyBufferException;
 
-public class ReadOnlyDirectByteBufTest {
+public class ReadOnlyDirectByteBufferBufTest {
 
     protected ByteBuf buffer(ByteBuffer buffer) {
-        return new ReadOnlyDirectByteBuf(UnpooledByteBufAllocator.DEFAULT, buffer.asReadOnlyBuffer());
+        return new ReadOnlyByteBufferBuf(UnpooledByteBufAllocator.DEFAULT, buffer);
+    }
+
+    protected ByteBuffer allocate(int size) {
+        return ByteBuffer.allocateDirect(size);
     }
 
     @Test(expected = IllegalArgumentException.class)
-    public void testConstructWithNotDirectBuffer() {
+    public void testConstructWithWritable() {
-        buffer(ByteBuffer.allocate(1));
+        buffer(allocate(1));
     }
 
     @Test(expected = ReadOnlyBufferException.class)
     public void testSetByte() {
-        ByteBuf buf = buffer(ByteBuffer.allocateDirect(8).asReadOnlyBuffer());
+        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
         buf.setByte(0, 1);
     }
 
 
     @Test(expected = ReadOnlyBufferException.class)
     public void testSetInt() {
-        ByteBuf buf = buffer(ByteBuffer.allocateDirect(8).asReadOnlyBuffer());
+        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
         buf.setInt(0, 1);
     }
 
     @Test(expected = ReadOnlyBufferException.class)
     public void testSetShort() {
-        ByteBuf buf = buffer(ByteBuffer.allocateDirect(8).asReadOnlyBuffer());
+        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
         buf.setShort(0, 1);
     }
 
     @Test(expected = ReadOnlyBufferException.class)
     public void testSetMedium() {
-        ByteBuf buf = buffer(ByteBuffer.allocateDirect(8).asReadOnlyBuffer());
+        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
         buf.setMedium(0, 1);
     }
 
     @Test(expected = ReadOnlyBufferException.class)
     public void testSetLong() {
-        ByteBuf buf = buffer(ByteBuffer.allocateDirect(8).asReadOnlyBuffer());
+        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
         buf.setLong(0, 1);
     }
 
     @Test(expected = ReadOnlyBufferException.class)
     public void testSetBytesViaArray() {
-        ByteBuf buf = buffer(ByteBuffer.allocateDirect(8).asReadOnlyBuffer());
+        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
         buf.setBytes(0, "test".getBytes());
     }
 
     @Test(expected = ReadOnlyBufferException.class)
     public void testSetBytesViaBuffer() {
-        ByteBuf buf = buffer(ByteBuffer.allocateDirect(8).asReadOnlyBuffer());
+        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
         buf.setBytes(0, Unpooled.copyInt(1));
     }
 
@@ -85,7 +89,7 @@ public class ReadOnlyDirectByteBufTest {
 
     @Test
     public void testGetReadByte() {
-        ByteBuf buf = buffer(((ByteBuffer) ByteBuffer.allocateDirect(2).put(new byte[]{(byte) 1, (byte) 2}).flip()).asReadOnlyBuffer());
+        ByteBuf buf = buffer(((ByteBuffer) allocate(2).put(new byte[]{(byte) 1, (byte) 2}).flip()).asReadOnlyBuffer());
         Assert.assertEquals(1, buf.getByte(0));
         Assert.assertEquals(2, buf.getByte(1));
 
@@ -96,7 +100,7 @@ public class ReadOnlyDirectByteBufTest {
 
     @Test
     public void testGetReadInt() {
-        ByteBuf buf = buffer(((ByteBuffer) ByteBuffer.allocateDirect(8).putInt(1).putInt(2).flip()).asReadOnlyBuffer());
+        ByteBuf buf = buffer(((ByteBuffer) allocate(8).putInt(1).putInt(2).flip()).asReadOnlyBuffer());
         Assert.assertEquals(1, buf.getInt(0));
         Assert.assertEquals(2, buf.getInt(4));
 
@@ -108,7 +112,7 @@ public class ReadOnlyDirectByteBufTest {
 
     @Test
     public void testGetReadShort() {
-        ByteBuf buf = buffer(((ByteBuffer) ByteBuffer.allocateDirect(8).putShort((short) 1).putShort((short) 2).flip()).asReadOnlyBuffer());
+        ByteBuf buf = buffer(((ByteBuffer) allocate(8).putShort((short) 1).putShort((short) 2).flip()).asReadOnlyBuffer());
         Assert.assertEquals(1, buf.getShort(0));
         Assert.assertEquals(2, buf.getShort(2));
 
@@ -119,7 +123,7 @@ public class ReadOnlyDirectByteBufTest {
 
     @Test
     public void testGetReadLong() {
-        ByteBuf buf = buffer(((ByteBuffer) ByteBuffer.allocateDirect(16).putLong(1).putLong(2).flip()).asReadOnlyBuffer());
+        ByteBuf buf = buffer(((ByteBuffer) allocate(16).putLong(1).putLong(2).flip()).asReadOnlyBuffer());
         Assert.assertEquals(1, buf.getLong(0));
         Assert.assertEquals(2, buf.getLong(8));
 
@@ -130,13 +134,13 @@ public class ReadOnlyDirectByteBufTest {
 
     @Test
     public void testCopy() {
-        ByteBuf buf = buffer(((ByteBuffer) ByteBuffer.allocateDirect(16).putLong(1).putLong(2).flip()).asReadOnlyBuffer());
+        ByteBuf buf = buffer(((ByteBuffer) allocate(16).putLong(1).putLong(2).flip()).asReadOnlyBuffer());
         Assert.assertEquals(buf, buf.copy());
     }
 
     @Test
     public void testCopyWithOffset() {
-        ByteBuf buf = buffer(((ByteBuffer) ByteBuffer.allocateDirect(16).putLong(1).putLong(2).flip()).asReadOnlyBuffer());
+        ByteBuf buf = buffer(((ByteBuffer) allocate(16).putLong(1).putLong(2).flip()).asReadOnlyBuffer());
         Assert.assertEquals(buf.slice(1, 9), buf.copy(1, 9));
     }
 }

buffer/src/test/java/io/netty/buffer/ReadOnlyUnsafeDirectByteBufferBufTest.java

@@ -22,7 +22,7 @@ import java.nio.ByteBuffer;
 
 import static org.junit.Assume.assumeTrue;
 
-public class ReadOnlyUnsafeDirectByteBufTest extends ReadOnlyDirectByteBufTest {
+public class ReadOnlyUnsafeDirectByteBufferBufTest extends ReadOnlyDirectByteBufferBufTest {
 
     /**
      * Needs unsafe to run