andreacavalli/netty-incubator-buffer-api
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix bounds checking bugs when setting bytes

Browse Source 
These should not take the read offset into account.
This commit is contained in:
Chris Vest 2021-03-09 16:16:38 +01:00
parent da70f29ff4
commit 2dee6f8516
3 changed files with 58 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/io/netty/buffer/api/CompositeBuffer.java
View File

@ -1279,7 +1279,7 @@ final class CompositeBuffer extends RcSupport<Buffer, CompositeBuffer> implement
} }
return new IndexOutOfBoundsException( return new IndexOutOfBoundsException(
"Index " + index + " is out of bounds: [read 0 to " + woff + ", write 0 to " + "Index " + index + " is out of bounds: [read 0 to " + woff + ", write 0 to " +
(capacity - 1) + "]."); capacity + "].");
} }
private static IllegalStateException bufferIsClosed() { private static IllegalStateException bufferIsClosed() {

16
src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java
View File

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

70
src/test/java/io/netty/buffer/api/BufferTest.java
View File

@ -582,6 +582,28 @@ public class BufferTest {
} }
} }
@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 @ParameterizedTest
@MethodSource("allocators") @MethodSource("allocators")
void setReaderOffsetMustNotThrowWithinBounds(Fixture fixture) { void setReaderOffsetMustNotThrowWithinBounds(Fixture fixture) {
@ -2454,30 +2476,30 @@ public class BufferTest {
} }
private static void verifyWriteAccessible(Buffer buf) { private static void verifyWriteAccessible(Buffer buf) {
buf.writerOffset(0).writeByte((byte) 32); buf.reset().writeByte((byte) 32);
assertThat(buf.readerOffset(0).readByte()).isEqualTo((byte) 32); assertThat(buf.readByte()).isEqualTo((byte) 32);
buf.writerOffset(0).writeUnsignedByte(32); buf.reset().writerOffset(0).writeUnsignedByte(32);
assertThat(buf.readerOffset(0).readUnsignedByte()).isEqualTo(32); assertThat(buf.readUnsignedByte()).isEqualTo(32);
buf.writerOffset(0).writeChar('3'); buf.reset().writerOffset(0).writeChar('3');
assertThat(buf.readerOffset(0).readChar()).isEqualTo('3'); assertThat(buf.readChar()).isEqualTo('3');
buf.writerOffset(0).writeShort((short) 32); buf.reset().writerOffset(0).writeShort((short) 32);
assertThat(buf.readerOffset(0).readShort()).isEqualTo((short) 32); assertThat(buf.readShort()).isEqualTo((short) 32);
buf.writerOffset(0).writeUnsignedShort(32); buf.reset().writerOffset(0).writeUnsignedShort(32);
assertThat(buf.readerOffset(0).readUnsignedShort()).isEqualTo(32); assertThat(buf.readUnsignedShort()).isEqualTo(32);
buf.writerOffset(0).writeMedium(32); buf.reset().writerOffset(0).writeMedium(32);
assertThat(buf.readerOffset(0).readMedium()).isEqualTo(32); assertThat(buf.readMedium()).isEqualTo(32);
buf.writerOffset(0).writeUnsignedMedium(32); buf.reset().writerOffset(0).writeUnsignedMedium(32);
assertThat(buf.readerOffset(0).readUnsignedMedium()).isEqualTo(32); assertThat(buf.readUnsignedMedium()).isEqualTo(32);
buf.writerOffset(0).writeInt(32); buf.reset().writerOffset(0).writeInt(32);
assertThat(buf.readerOffset(0).readInt()).isEqualTo(32); assertThat(buf.readInt()).isEqualTo(32);
buf.writerOffset(0).writeUnsignedInt(32); buf.reset().writerOffset(0).writeUnsignedInt(32);
assertThat(buf.readerOffset(0).readUnsignedInt()).isEqualTo(32L); assertThat(buf.readUnsignedInt()).isEqualTo(32L);
buf.writerOffset(0).writeFloat(3.2f); buf.reset().writerOffset(0).writeFloat(3.2f);
assertThat(buf.readerOffset(0).readFloat()).isEqualTo(3.2f); assertThat(buf.readFloat()).isEqualTo(3.2f);
buf.writerOffset(0).writeLong(32); buf.reset().writerOffset(0).writeLong(32);
assertThat(buf.readerOffset(0).readLong()).isEqualTo(32L); assertThat(buf.readLong()).isEqualTo(32L);
buf.writerOffset(0).writeDouble(3.2); buf.reset().writerOffset(0).writeDouble(3.2);
assertThat(buf.readerOffset(0).readDouble()).isEqualTo(3.2); assertThat(buf.readDouble()).isEqualTo(3.2);
buf.setByte(0, (byte) 32); buf.setByte(0, (byte) 32);
assertThat(buf.getByte(0)).isEqualTo((byte) 32); assertThat(buf.getByte(0)).isEqualTo((byte) 32);