Fix bounds checking bugs when setting bytes

These should not take the read offset into account.

Author: chrisvest

src/main/java/io/netty/buffer/api/CompositeBuffer.java

@@ -1279,7 +1279,7 @@ private RuntimeException indexOutOfBounds(int index, boolean write) {
         }
         return new IndexOutOfBoundsException(
                 "Index " + index + " is out of bounds: [read 0 to " + woff + ", write 0 to " +
-                (capacity - 1) + "].");
+                capacity + "].");
     }
 
     private static IllegalStateException bufferIsClosed() {

src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java

@@ -133,7 +133,7 @@ public MemSegBuffer writerOffset(int index) {
 
     @Override
     public Buffer fill(byte value) {
-        checkWrite(0, capacity());
+        checkSet(0, capacity());
         seg.fill(value);
         return this;
     }
@@ -281,7 +281,7 @@ private void copyInto(int srcPos, MemorySegment dest, int destPos, int length) {
     public void copyInto(int srcPos, Buffer dest, int destPos, int length) {
         if (dest instanceof MemSegBuffer) {
             var memSegBuf = (MemSegBuffer) dest;
-            memSegBuf.checkWrite(destPos, length);
+            memSegBuf.checkSet(destPos, length);
             copyInto(srcPos, memSegBuf.seg, destPos, length);
             return;
         }
@@ -824,7 +824,7 @@ public Buffer writeMedium(int value) {
 
     @Override
     public Buffer setMedium(int woff, int value) {
-        checkWrite(woff, 3);
+        checkSet(woff, 3);
         if (order == ByteOrder.BIG_ENDIAN) {
             setByteAtOffset(wseg, woff, (byte) (value >> 16));
             setByteAtOffset(wseg, woff + 1, (byte) (value >> 8 & 0xFF));
@@ -855,7 +855,7 @@ public Buffer writeUnsignedMedium(int value) {
 
     @Override
     public Buffer setUnsignedMedium(int woff, int value) {
-        checkWrite(woff, 3);
+        checkSet(woff, 3);
         if (order == ByteOrder.BIG_ENDIAN) {
             setByteAtOffset(wseg, woff, (byte) (value >> 16));
             setByteAtOffset(wseg, woff + 1, (byte) (value >> 8 & 0xFF));
@@ -1100,6 +1100,12 @@ private void checkGet(int index, int size) {
     }
 
     private void checkWrite(int index, int size) {
+        if (index < roff || wseg.byteSize() < index + size) {
+            throw writeAccessCheckException(index);
+        }
+    }
+
+    private void checkSet(int index, int size) {
         if (index < 0 || wseg.byteSize() < index + size) {
             throw writeAccessCheckException(index);
         }
@@ -1143,7 +1149,7 @@ private static IllegalStateException bufferIsReadOnly() {
     private IndexOutOfBoundsException outOfBounds(int index) {
         return new IndexOutOfBoundsException(
                 "Index " + index + " is out of bounds: [read 0 to " + woff + ", write 0 to " +
-                (seg.byteSize() - 1) + "].");
+                seg.byteSize() + "].");
     }
 
     Object recoverableMemory() {

src/test/java/io/netty/buffer/api/BufferTest.java

@@ -582,6 +582,28 @@ void setReaderOffsetMustThrowOnOversizedIndex(Fixture fixture) {
         }
     }
 
+    @ParameterizedTest
+    @MethodSource("allocators")
+    public void setWriterOffsetMustThrowOutsideOfWritableRegion(Fixture fixture) {
+        try (BufferAllocator allocator = fixture.createAllocator();
+             Buffer buf = allocator.allocate(8)) {
+            // Writer offset cannot be negative.
+            assertThrows(IndexOutOfBoundsException.class, () -> buf.writerOffset(-1));
+
+            buf.writerOffset(4);
+            buf.readerOffset(4);
+
+            // Cannot set writer offset before reader offset.
+            assertThrows(IndexOutOfBoundsException.class, () -> buf.writerOffset(3));
+            assertThrows(IndexOutOfBoundsException.class, () -> buf.writerOffset(0));
+
+            buf.writerOffset(buf.capacity());
+
+            // Cannot set writer offset beyond capacity.
+            assertThrows(IndexOutOfBoundsException.class, () -> buf.writerOffset(buf.capacity() + 1));
+        }
+    }
+
     @ParameterizedTest
     @MethodSource("allocators")
     void setReaderOffsetMustNotThrowWithinBounds(Fixture fixture) {
@@ -2454,30 +2476,30 @@ public void readOnlyBufferMustBecomeWritableAgainAfterTogglingReadOnlyOff(Fixtur
     }
 
     private static void verifyWriteAccessible(Buffer buf) {
-        buf.writerOffset(0).writeByte((byte) 32);
-        assertThat(buf.readerOffset(0).readByte()).isEqualTo((byte) 32);
-        buf.writerOffset(0).writeUnsignedByte(32);
-        assertThat(buf.readerOffset(0).readUnsignedByte()).isEqualTo(32);
-        buf.writerOffset(0).writeChar('3');
-        assertThat(buf.readerOffset(0).readChar()).isEqualTo('3');
-        buf.writerOffset(0).writeShort((short) 32);
-        assertThat(buf.readerOffset(0).readShort()).isEqualTo((short) 32);
-        buf.writerOffset(0).writeUnsignedShort(32);
-        assertThat(buf.readerOffset(0).readUnsignedShort()).isEqualTo(32);
-        buf.writerOffset(0).writeMedium(32);
-        assertThat(buf.readerOffset(0).readMedium()).isEqualTo(32);
-        buf.writerOffset(0).writeUnsignedMedium(32);
-        assertThat(buf.readerOffset(0).readUnsignedMedium()).isEqualTo(32);
-        buf.writerOffset(0).writeInt(32);
-        assertThat(buf.readerOffset(0).readInt()).isEqualTo(32);
-        buf.writerOffset(0).writeUnsignedInt(32);
-        assertThat(buf.readerOffset(0).readUnsignedInt()).isEqualTo(32L);
-        buf.writerOffset(0).writeFloat(3.2f);
-        assertThat(buf.readerOffset(0).readFloat()).isEqualTo(3.2f);
-        buf.writerOffset(0).writeLong(32);
-        assertThat(buf.readerOffset(0).readLong()).isEqualTo(32L);
-        buf.writerOffset(0).writeDouble(3.2);
-        assertThat(buf.readerOffset(0).readDouble()).isEqualTo(3.2);
+        buf.reset().writeByte((byte) 32);
+        assertThat(buf.readByte()).isEqualTo((byte) 32);
+        buf.reset().writerOffset(0).writeUnsignedByte(32);
+        assertThat(buf.readUnsignedByte()).isEqualTo(32);
+        buf.reset().writerOffset(0).writeChar('3');
+        assertThat(buf.readChar()).isEqualTo('3');
+        buf.reset().writerOffset(0).writeShort((short) 32);
+        assertThat(buf.readShort()).isEqualTo((short) 32);
+        buf.reset().writerOffset(0).writeUnsignedShort(32);
+        assertThat(buf.readUnsignedShort()).isEqualTo(32);
+        buf.reset().writerOffset(0).writeMedium(32);
+        assertThat(buf.readMedium()).isEqualTo(32);
+        buf.reset().writerOffset(0).writeUnsignedMedium(32);
+        assertThat(buf.readUnsignedMedium()).isEqualTo(32);
+        buf.reset().writerOffset(0).writeInt(32);
+        assertThat(buf.readInt()).isEqualTo(32);
+        buf.reset().writerOffset(0).writeUnsignedInt(32);
+        assertThat(buf.readUnsignedInt()).isEqualTo(32L);
+        buf.reset().writerOffset(0).writeFloat(3.2f);
+        assertThat(buf.readFloat()).isEqualTo(3.2f);
+        buf.reset().writerOffset(0).writeLong(32);
+        assertThat(buf.readLong()).isEqualTo(32L);
+        buf.reset().writerOffset(0).writeDouble(3.2);
+        assertThat(buf.readDouble()).isEqualTo(3.2);
 
         buf.setByte(0, (byte) 32);
         assertThat(buf.getByte(0)).isEqualTo((byte) 32);