From 3cbb41a4781375b1a714f2d4b1b6e91be00db4b4 Mon Sep 17 00:00:00 2001
From: Chris Vest <christianvest_hansen@apple.com>
Date: Wed, 8 Sep 2021 09:04:16 +0200
Subject: [PATCH] Make ByteCursor implementations static final inner classes
 (#11662)

Motivation:
People might be tempted to use mocking tools like Mockito.spy() on the ByteCursors.
By returning instances where the concrete classes are final, we will be forcing integrators to use stub-like wrappers instead.
Such stubs are more well-behaved since they are implemented in terms of the real instance.
This prevents the mocked objects from (easily) producing behaviour that violates the API specification.

Modification:
All ByteCursor implementations have changed from using anonymous inner classes, to using static-final named inner classes.

Result:
The concrete ByteCursor classes can no longer be extended via byte code generation, such as from mocking tools.
---
 .../io/netty/buffer/api/CompositeBuffer.java  | 224 ++++++++++--------
 .../buffer/api/bytebuffer/NioBuffer.java      | 144 ++++++-----
 .../netty/buffer/api/unsafe/UnsafeBuffer.java | 170 +++++++------
 .../buffer/AbstractByteBufAllocatorTest.java  |   2 -
 4 files changed, 303 insertions(+), 237 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/api/CompositeBuffer.java b/buffer/src/main/java/io/netty/buffer/api/CompositeBuffer.java
index 26033a6cb2..6c123cf8fc 100644
--- a/buffer/src/main/java/io/netty/buffer/api/CompositeBuffer.java
+++ b/buffer/src/main/java/io/netty/buffer/api/CompositeBuffer.java
@@ -518,54 +518,7 @@ public final class CompositeBuffer extends ResourceSupport<Buffer, CompositeBuff
         int off = fromOffset - offsets[startBufferIndex];
         Buffer startBuf = bufs[startBufferIndex];
         ByteCursor startCursor = startBuf.openCursor(off, Math.min(startBuf.capacity() - off, length));
-        return new ByteCursor() {
-            int index = fromOffset;
-            final int end = fromOffset + length;
-            int bufferIndex = startBufferIndex;
-            int initOffset = startCursor.currentOffset();
-            ByteCursor cursor = startCursor;
-            byte byteValue = -1;
-
-            @Override
-            public boolean readByte() {
-                if (cursor.readByte()) {
-                    byteValue = cursor.getByte();
-                    return true;
-                }
-                if (bytesLeft() > 0) {
-                    nextCursor();
-                    cursor.readByte();
-                    byteValue = cursor.getByte();
-                    return true;
-                }
-                return false;
-            }
-
-            private void nextCursor() {
-                bufferIndex++;
-                Buffer nextBuf = bufs[bufferIndex];
-                cursor = nextBuf.openCursor(0, Math.min(nextBuf.capacity(), bytesLeft()));
-                initOffset = 0;
-            }
-
-            @Override
-            public byte getByte() {
-                return byteValue;
-            }
-
-            @Override
-            public int currentOffset() {
-                int currOff = cursor.currentOffset();
-                index += currOff - initOffset;
-                initOffset = currOff;
-                return index;
-            }
-
-            @Override
-            public int bytesLeft() {
-                return end - currentOffset();
-            }
-        };
+        return new ForwardCompositeByteCursor(bufs, fromOffset, length, startBufferIndex, startCursor);
     }
 
     @Override
@@ -584,56 +537,7 @@ public final class CompositeBuffer extends ResourceSupport<Buffer, CompositeBuff
         int off = fromOffset - offsets[startBufferIndex];
         Buffer startBuf = bufs[startBufferIndex];
         ByteCursor startCursor = startBuf.openReverseCursor(off, Math.min(off + 1, length));
-        return new ByteCursor() {
-            int index = fromOffset;
-            final int end = fromOffset - length;
-            int bufferIndex = startBufferIndex;
-            int initOffset = startCursor.currentOffset();
-            ByteCursor cursor = startCursor;
-            byte byteValue = -1;
-
-            @Override
-            public boolean readByte() {
-                if (cursor.readByte()) {
-                    byteValue = cursor.getByte();
-                    return true;
-                }
-                if (bytesLeft() > 0) {
-                    nextCursor();
-                    cursor.readByte();
-                    byteValue = cursor.getByte();
-                    return true;
-                }
-                return false;
-            }
-
-            private void nextCursor() {
-                bufferIndex--;
-                Buffer nextBuf = bufs[bufferIndex];
-                int length = Math.min(nextBuf.capacity(), bytesLeft());
-                int offset = nextBuf.capacity() - 1;
-                cursor = nextBuf.openReverseCursor(offset, length);
-                initOffset = offset;
-            }
-
-            @Override
-            public byte getByte() {
-                return byteValue;
-            }
-
-            @Override
-            public int currentOffset() {
-                int currOff = cursor.currentOffset();
-                index -= initOffset - currOff;
-                initOffset = currOff;
-                return index;
-            }
-
-            @Override
-            public int bytesLeft() {
-                return currentOffset() - end;
-            }
-        };
+        return new ReverseCompositeByteCursor(bufs, fromOffset, length, startBufferIndex, startCursor);
     }
 
     @Override
@@ -1732,4 +1636,128 @@ public final class CompositeBuffer extends ResourceSupport<Buffer, CompositeBuff
         }
     }
     // </editor-fold>
+
+    private static final class ForwardCompositeByteCursor implements ByteCursor {
+        final Buffer[] bufs;
+        int index;
+        final int end;
+        int bufferIndex;
+        int initOffset;
+        ByteCursor cursor;
+        byte byteValue;
+
+        ForwardCompositeByteCursor(Buffer[] bufs, int fromOffset, int length, int startBufferIndex,
+                                   ByteCursor startCursor) {
+            this.bufs = bufs;
+            index = fromOffset;
+            end = fromOffset + length;
+            bufferIndex = startBufferIndex;
+            initOffset = startCursor.currentOffset();
+            cursor = startCursor;
+            byteValue = -1;
+        }
+
+        @Override
+        public boolean readByte() {
+            if (cursor.readByte()) {
+                byteValue = cursor.getByte();
+                return true;
+            }
+            if (bytesLeft() > 0) {
+                nextCursor();
+                cursor.readByte();
+                byteValue = cursor.getByte();
+                return true;
+            }
+            return false;
+        }
+
+        private void nextCursor() {
+            bufferIndex++;
+            Buffer nextBuf = bufs[bufferIndex];
+            cursor = nextBuf.openCursor(0, Math.min(nextBuf.capacity(), bytesLeft()));
+            initOffset = 0;
+        }
+
+        @Override
+        public byte getByte() {
+            return byteValue;
+        }
+
+        @Override
+        public int currentOffset() {
+            int currOff = cursor.currentOffset();
+            index += currOff - initOffset;
+            initOffset = currOff;
+            return index;
+        }
+
+        @Override
+        public int bytesLeft() {
+            return end - currentOffset();
+        }
+    }
+
+    private static final class ReverseCompositeByteCursor implements ByteCursor {
+        final Buffer[] bufs;
+        int index;
+        final int end;
+        int bufferIndex;
+        int initOffset;
+        ByteCursor cursor;
+        byte byteValue;
+
+        ReverseCompositeByteCursor(Buffer[] bufs, int fromOffset, int length,
+                                   int startBufferIndex, ByteCursor startCursor) {
+            this.bufs = bufs;
+            index = fromOffset;
+            end = fromOffset - length;
+            bufferIndex = startBufferIndex;
+            initOffset = startCursor.currentOffset();
+            cursor = startCursor;
+            byteValue = -1;
+        }
+
+        @Override
+        public boolean readByte() {
+            if (cursor.readByte()) {
+                byteValue = cursor.getByte();
+                return true;
+            }
+            if (bytesLeft() > 0) {
+                nextCursor();
+                cursor.readByte();
+                byteValue = cursor.getByte();
+                return true;
+            }
+            return false;
+        }
+
+        private void nextCursor() {
+            bufferIndex--;
+            Buffer nextBuf = bufs[bufferIndex];
+            int length = Math.min(nextBuf.capacity(), bytesLeft());
+            int offset = nextBuf.capacity() - 1;
+            cursor = nextBuf.openReverseCursor(offset, length);
+            initOffset = offset;
+        }
+
+        @Override
+        public byte getByte() {
+            return byteValue;
+        }
+
+        @Override
+        public int currentOffset() {
+            int currOff = cursor.currentOffset();
+            index -= initOffset - currOff;
+            initOffset = currOff;
+            return index;
+        }
+
+        @Override
+        public int bytesLeft() {
+            return currentOffset() - end;
+        }
+    }
 }
diff --git a/buffer/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java b/buffer/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java
index 8f76691add..0fa0971b3e 100644
--- a/buffer/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java
+++ b/buffer/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java
@@ -253,38 +253,7 @@ class NioBuffer extends AdaptableBuffer<NioBuffer> implements ReadableComponent,
             throw new IllegalArgumentException("The fromOffset + length is beyond the end of the buffer: " +
                     "fromOffset = " + fromOffset + ", length = " + length + '.');
         }
-        return new ByteCursor() {
-            // Duplicate source buffer to keep our own byte order state.
-            final ByteBuffer buffer = rmem.duplicate().order(ByteOrder.BIG_ENDIAN);
-            int index = fromOffset;
-            final int end = index + length;
-            byte byteValue = -1;
-
-            @Override
-            public boolean readByte() {
-                if (index < end) {
-                    byteValue = buffer.get(index);
-                    index++;
-                    return true;
-                }
-                return false;
-            }
-
-            @Override
-            public byte getByte() {
-                return byteValue;
-            }
-
-            @Override
-            public int currentOffset() {
-                return index;
-            }
-
-            @Override
-            public int bytesLeft() {
-                return end - index;
-            }
-        };
+        return new ForwardNioByteCursor(rmem, fromOffset, length);
     }
 
     @Override
@@ -305,37 +274,7 @@ class NioBuffer extends AdaptableBuffer<NioBuffer> implements ReadableComponent,
             throw new IllegalArgumentException("The fromOffset - length would underflow the buffer: " +
                     "fromOffset = " + fromOffset + ", length = " + length + '.');
         }
-        return new ByteCursor() {
-            final ByteBuffer buffer = rmem.duplicate().order(ByteOrder.LITTLE_ENDIAN);
-            int index = fromOffset;
-            final int end = index - length;
-            byte byteValue = -1;
-
-            @Override
-            public boolean readByte() {
-                if (index > end) {
-                    byteValue = buffer.get(index);
-                    index--;
-                    return true;
-                }
-                return false;
-            }
-
-            @Override
-            public byte getByte() {
-                return byteValue;
-            }
-
-            @Override
-            public int currentOffset() {
-                return index;
-            }
-
-            @Override
-            public int bytesLeft() {
-                return index - end;
-            }
-        };
+        return new ReverseNioByteCursor(rmem, fromOffset, length);
     }
 
     @Override
@@ -1104,4 +1043,83 @@ class NioBuffer extends AdaptableBuffer<NioBuffer> implements ReadableComponent,
     ByteBuffer recoverable() {
         return base;
     }
+
+    private static final class ForwardNioByteCursor implements ByteCursor {
+        // Duplicate source buffer to keep our own byte order state.
+        final ByteBuffer buffer;
+        int index;
+        final int end;
+        byte byteValue;
+
+        ForwardNioByteCursor(ByteBuffer rmem, int fromOffset, int length) {
+            buffer = rmem.duplicate().order(ByteOrder.BIG_ENDIAN);
+            index = fromOffset;
+            end = index + length;
+            byteValue = -1;
+        }
+
+        @Override
+        public boolean readByte() {
+            if (index < end) {
+                byteValue = buffer.get(index);
+                index++;
+                return true;
+            }
+            return false;
+        }
+
+        @Override
+        public byte getByte() {
+            return byteValue;
+        }
+
+        @Override
+        public int currentOffset() {
+            return index;
+        }
+
+        @Override
+        public int bytesLeft() {
+            return end - index;
+        }
+    }
+
+    private static final class ReverseNioByteCursor implements ByteCursor {
+        final ByteBuffer buffer;
+        int index;
+        final int end;
+        byte byteValue;
+
+        ReverseNioByteCursor(ByteBuffer rmem, int fromOffset, int length) {
+            buffer = rmem.duplicate().order(ByteOrder.LITTLE_ENDIAN);
+            index = fromOffset;
+            end = index - length;
+            byteValue = -1;
+        }
+
+        @Override
+        public boolean readByte() {
+            if (index > end) {
+                byteValue = buffer.get(index);
+                index--;
+                return true;
+            }
+            return false;
+        }
+
+        @Override
+        public byte getByte() {
+            return byteValue;
+        }
+
+        @Override
+        public int currentOffset() {
+            return index;
+        }
+
+        @Override
+        public int bytesLeft() {
+            return index - end;
+        }
+    }
 }
diff --git a/buffer/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java b/buffer/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java
index 9079a24ccb..ddaaed5cf3 100644
--- a/buffer/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java
+++ b/buffer/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java
@@ -269,43 +269,7 @@ class UnsafeBuffer extends AdaptableBuffer<UnsafeBuffer> implements ReadableComp
             throw new IllegalArgumentException("The fromOffset + length is beyond the end of the buffer: " +
                     "fromOffset = " + fromOffset + ", length = " + length + '.');
         }
-        return new ByteCursor() {
-            final UnsafeMemory memory = UnsafeBuffer.this.memory; // Keep memory alive.
-            final Object baseObj = base;
-            final long baseAddress = address;
-            int index = fromOffset;
-            final int end = index + length;
-            byte byteValue = -1;
-
-            @Override
-            public boolean readByte() {
-                if (index < end) {
-                    try {
-                        byteValue = PlatformDependent.getByte(baseObj, baseAddress + index);
-                    } finally {
-                        Reference.reachabilityFence(memory);
-                    }
-                    index++;
-                    return true;
-                }
-                return false;
-            }
-
-            @Override
-            public byte getByte() {
-                return byteValue;
-            }
-
-            @Override
-            public int currentOffset() {
-                return index;
-            }
-
-            @Override
-            public int bytesLeft() {
-                return end - index;
-            }
-        };
+        return new ForwardUnsafeByteCursor(memory, base, address, fromOffset, length);
     }
 
     @Override
@@ -326,43 +290,7 @@ class UnsafeBuffer extends AdaptableBuffer<UnsafeBuffer> implements ReadableComp
             throw new IllegalArgumentException("The fromOffset - length would underflow the buffer: " +
                     "fromOffset = " + fromOffset + ", length = " + length + '.');
         }
-        return new ByteCursor() {
-            final UnsafeMemory memory = UnsafeBuffer.this.memory; // Keep memory alive.
-            final Object baseObj = base;
-            final long baseAddress = address;
-            int index = fromOffset;
-            final int end = index - length;
-            byte byteValue = -1;
-
-            @Override
-            public boolean readByte() {
-                if (index > end) {
-                    try {
-                        byteValue = PlatformDependent.getByte(baseObj, baseAddress + index);
-                    } finally {
-                        Reference.reachabilityFence(memory);
-                    }
-                    index--;
-                    return true;
-                }
-                return false;
-            }
-
-            @Override
-            public byte getByte() {
-                return byteValue;
-            }
-
-            @Override
-            public int currentOffset() {
-                return index;
-            }
-
-            @Override
-            public int bytesLeft() {
-                return index - end;
-            }
-        };
+        return new ReverseUnsafeByteCursor(memory, base, address, fromOffset, length);
     }
 
     @Override
@@ -1511,4 +1439,98 @@ class UnsafeBuffer extends AdaptableBuffer<UnsafeBuffer> implements ReadableComp
     Object recover() {
         return memory;
     }
+
+    private static final class ForwardUnsafeByteCursor implements ByteCursor {
+        final UnsafeMemory memory; // Keep memory alive.
+        final Object baseObj;
+        final long baseAddress;
+        int index;
+        final int end;
+        byte byteValue;
+
+        ForwardUnsafeByteCursor(UnsafeMemory memory, Object base, long address, int fromOffset, int length) {
+            this.memory = memory;
+            baseObj = base;
+            baseAddress = address;
+            index = fromOffset;
+            end = index + length;
+            byteValue = -1;
+        }
+
+        @Override
+        public boolean readByte() {
+            if (index < end) {
+                try {
+                    byteValue = PlatformDependent.getByte(baseObj, baseAddress + index);
+                } finally {
+                    Reference.reachabilityFence(memory);
+                }
+                index++;
+                return true;
+            }
+            return false;
+        }
+
+        @Override
+        public byte getByte() {
+            return byteValue;
+        }
+
+        @Override
+        public int currentOffset() {
+            return index;
+        }
+
+        @Override
+        public int bytesLeft() {
+            return end - index;
+        }
+    }
+
+    private static final class ReverseUnsafeByteCursor implements ByteCursor {
+        final UnsafeMemory memory; // Keep memory alive.
+        final Object baseObj;
+        final long baseAddress;
+        int index;
+        final int end;
+        byte byteValue;
+
+        ReverseUnsafeByteCursor(UnsafeMemory memory, Object base, long address, int fromOffset, int length) {
+            this.memory = memory;
+            baseObj = base;
+            baseAddress = address;
+            index = fromOffset;
+            end = index - length;
+            byteValue = -1;
+        }
+
+        @Override
+        public boolean readByte() {
+            if (index > end) {
+                try {
+                    byteValue = PlatformDependent.getByte(baseObj, baseAddress + index);
+                } finally {
+                    Reference.reachabilityFence(memory);
+                }
+                index--;
+                return true;
+            }
+            return false;
+        }
+
+        @Override
+        public byte getByte() {
+            return byteValue;
+        }
+
+        @Override
+        public int currentOffset() {
+            return index;
+        }
+
+        @Override
+        public int bytesLeft() {
+            return index - end;
+        }
+    }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufAllocatorTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufAllocatorTest.java
index e2f3ab72ef..3aac818458 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufAllocatorTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufAllocatorTest.java
@@ -95,7 +95,6 @@ public abstract class AbstractByteBufAllocatorTest<T extends AbstractByteBufAllo
         assertTrue(clazz.isInstance(buffer instanceof SimpleLeakAwareByteBuf ? buffer.unwrap() : buffer));
     }
 
-    @SuppressWarnings("unchecked")
     @Test
     public void testUsedDirectMemory() {
         T allocator =  newAllocator(true);
@@ -114,7 +113,6 @@ public abstract class AbstractByteBufAllocatorTest<T extends AbstractByteBufAllo
         assertEquals(expectedUsedMemoryAfterRelease(allocator, capacity), metric.usedDirectMemory());
     }
 
-    @SuppressWarnings("unchecked")
     @Test
     public void testUsedHeapMemory() {
         T allocator =  newAllocator(true);