Add proper boundary / freeness check on ByteBuf impls

- Fixes #827
This commit is contained in:
Trustin Lee 2012-12-17 18:27:30 +09:00
parent ca93b624ff
commit a8f5efdb26
5 changed files with 98 additions and 109 deletions

View File

@ -155,6 +155,7 @@ public abstract class AbstractByteBuf implements ByteBuf {
@Override
public ByteBuf discardReadBytes() {
checkUnfreed();
if (readerIndex == 0) {
return this;
}
@ -173,6 +174,7 @@ public abstract class AbstractByteBuf implements ByteBuf {
@Override
public ByteBuf discardSomeReadBytes() {
checkUnfreed();
if (readerIndex == 0) {
return this;
}
@ -926,15 +928,38 @@ public abstract class AbstractByteBuf implements ByteBuf {
')';
}
protected final void checkIndex(int index) {
checkUnfreed();
if (index < 0 || index >= capacity()) {
throw new IndexOutOfBoundsException(String.format(
"index: %d (expected: range(0, %d))", index, capacity()));
}
}
protected final void checkIndex(int index, int fieldLength) {
checkUnfreed();
if (index < 0 || index > capacity() - fieldLength) {
throw new IndexOutOfBoundsException(String.format(
"index: %d, length: %d (expected: range(0, %d))", index, fieldLength, capacity()));
}
}
/**
* Throws an {@link IndexOutOfBoundsException} if the current
* {@linkplain #readableBytes() readable bytes} of this buffer is less
* than the specified value.
*/
protected void checkReadableBytes(int minimumReadableBytes) {
protected final void checkReadableBytes(int minimumReadableBytes) {
checkUnfreed();
if (readableBytes() < minimumReadableBytes) {
throw new IndexOutOfBoundsException("Not enough readable bytes - Need "
+ minimumReadableBytes + ", maximum is " + readableBytes());
}
}
protected final void checkUnfreed() {
if (isFreed()) {
throw new IllegalBufferAccessException();
}
}
}

View File

@ -36,7 +36,7 @@ abstract class PooledByteBuf<T> extends AbstractByteBuf {
super(maxCapacity);
}
void init(PoolChunk<T> chunk, long handle, T memory, int offset, int length) {
final void init(PoolChunk<T> chunk, long handle, T memory, int offset, int length) {
assert handle >= 0;
assert memory != null;
@ -50,14 +50,13 @@ abstract class PooledByteBuf<T> extends AbstractByteBuf {
}
@Override
public int capacity() {
public final int capacity() {
return length;
}
@Override
public ByteBuf capacity(int newCapacity) {
assert !isFreed();
public final ByteBuf capacity(int newCapacity) {
checkUnfreed();
if (suspendedDeallocations == null) {
chunk.arena.reallocate(this, newCapacity, true);
} else {
@ -69,21 +68,21 @@ abstract class PooledByteBuf<T> extends AbstractByteBuf {
}
@Override
public ByteBufAllocator alloc() {
public final ByteBufAllocator alloc() {
return chunk.arena.parent;
}
@Override
public ByteOrder order() {
public final ByteOrder order() {
return ByteOrder.BIG_ENDIAN;
}
@Override
public ByteBuf unwrap() {
public final ByteBuf unwrap() {
return null;
}
protected ByteBuffer internalNioBuffer() {
protected final ByteBuffer internalNioBuffer() {
ByteBuffer tmpNioBuf = this.tmpNioBuf;
if (tmpNioBuf == null) {
this.tmpNioBuf = tmpNioBuf = newInternalNioBuffer(memory);
@ -94,8 +93,8 @@ abstract class PooledByteBuf<T> extends AbstractByteBuf {
protected abstract ByteBuffer newInternalNioBuffer(T memory);
@Override
public ByteBuf suspendIntermediaryDeallocations() {
assert !isFreed();
public final ByteBuf suspendIntermediaryDeallocations() {
checkUnfreed();
if (suspendedDeallocations == null) {
suspendedDeallocations = new ArrayDeque<Allocation<T>>(2);
}
@ -103,7 +102,8 @@ abstract class PooledByteBuf<T> extends AbstractByteBuf {
}
@Override
public ByteBuf resumeIntermediaryDeallocations() {
public final ByteBuf resumeIntermediaryDeallocations() {
checkUnfreed();
if (suspendedDeallocations == null) {
return this;
}
@ -122,41 +122,25 @@ abstract class PooledByteBuf<T> extends AbstractByteBuf {
}
@Override
public boolean isFreed() {
return handle < 0;
public final boolean isFreed() {
return memory == null;
}
@Override
public void free() {
public final void free() {
if (handle >= 0) {
resumeIntermediaryDeallocations();
final long handle = this.handle;
this.handle = -1;
memory = null;
resumeIntermediaryDeallocations();
chunk.arena.free(chunk, handle);
}
}
protected int idx(int index) {
protected final int idx(int index) {
return offset + index;
}
protected void checkIndex(int index) {
assert !isFreed();
if (index < 0 || index >= length) {
throw new IndexOutOfBoundsException(String.format(
"index: %d (expected: range(0, %d))", index, length));
}
}
protected void checkIndex(int index, int fieldLength) {
assert !isFreed();
if (index < 0 || index > length - fieldLength) {
throw new IndexOutOfBoundsException(String.format(
"index: %d, length: %d (expected: range(0, %d))", index, fieldLength, length));
}
}
private static final class Allocation<T> {
final PoolChunk<T> chunk;
final long handle;

View File

@ -283,27 +283,6 @@ public class SlicedByteBuf extends AbstractByteBuf {
return buffer.nioBuffers(index, length);
}
private void checkIndex(int index) {
if (index < 0 || index >= capacity()) {
throw new IndexOutOfBoundsException("Invalid index: " + index
+ ", maximum is " + capacity());
}
}
private void checkIndex(int startIndex, int length) {
if (length < 0) {
throw new IllegalArgumentException(
"length is negative: " + length);
}
if (startIndex < 0) {
throw new IndexOutOfBoundsException("startIndex cannot be negative");
}
if (startIndex + length > capacity()) {
throw new IndexOutOfBoundsException("Index too big - Bytes needed: "
+ (startIndex + length) + ", maximum is " + capacity());
}
}
@Override
public boolean isFreed() {
return buffer.isFreed();

View File

@ -72,7 +72,6 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
private ByteBuffer buffer;
private ByteBuffer tmpNioBuf;
private int capacity;
private boolean freed;
private boolean doNotFree;
private Queue<ByteBuffer> suspendedDeallocations;
@ -165,7 +164,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf capacity(int newCapacity) {
assert !freed;
checkUnfreed();
if (newCapacity < 0 || newCapacity > maxCapacity()) {
throw new IllegalArgumentException("newCapacity: " + newCapacity);
}
@ -228,37 +227,37 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public byte getByte(int index) {
assert !freed;
checkUnfreed();
return buffer.get(index);
}
@Override
public short getShort(int index) {
assert !freed;
checkUnfreed();
return buffer.getShort(index);
}
@Override
public int getUnsignedMedium(int index) {
assert !freed;
checkUnfreed();
return (getByte(index) & 0xff) << 16 | (getByte(index + 1) & 0xff) << 8 | getByte(index + 2) & 0xff;
}
@Override
public int getInt(int index) {
assert !freed;
checkUnfreed();
return buffer.getInt(index);
}
@Override
public long getLong(int index) {
assert !freed;
checkUnfreed();
return buffer.getLong(index);
}
@Override
public ByteBuf getBytes(int index, ByteBuf dst, int dstIndex, int length) {
assert !freed;
checkUnfreed();
if (dst instanceof UnpooledDirectByteBuf) {
UnpooledDirectByteBuf bbdst = (UnpooledDirectByteBuf) dst;
ByteBuffer data = bbdst.internalNioBuffer();
@ -274,7 +273,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf getBytes(int index, byte[] dst, int dstIndex, int length) {
assert !freed;
checkUnfreed();
ByteBuffer tmpBuf = internalNioBuffer();
try {
tmpBuf.clear().position(index).limit(index + length);
@ -288,7 +287,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf getBytes(int index, ByteBuffer dst) {
assert !freed;
checkUnfreed();
int bytesToCopy = Math.min(capacity() - index, dst.remaining());
ByteBuffer tmpBuf = internalNioBuffer();
try {
@ -303,21 +302,21 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setByte(int index, int value) {
assert !freed;
checkUnfreed();
buffer.put(index, (byte) value);
return this;
}
@Override
public ByteBuf setShort(int index, int value) {
assert !freed;
checkUnfreed();
buffer.putShort(index, (short) value);
return this;
}
@Override
public ByteBuf setMedium(int index, int value) {
assert !freed;
checkUnfreed();
setByte(index, (byte) (value >>> 16));
setByte(index + 1, (byte) (value >>> 8));
setByte(index + 2, (byte) value);
@ -326,21 +325,21 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setInt(int index, int value) {
assert !freed;
checkUnfreed();
buffer.putInt(index, value);
return this;
}
@Override
public ByteBuf setLong(int index, long value) {
assert !freed;
checkUnfreed();
buffer.putLong(index, value);
return this;
}
@Override
public ByteBuf setBytes(int index, ByteBuf src, int srcIndex, int length) {
assert !freed;
checkUnfreed();
if (src instanceof UnpooledDirectByteBuf) {
UnpooledDirectByteBuf bbsrc = (UnpooledDirectByteBuf) src;
ByteBuffer data = bbsrc.internalNioBuffer();
@ -357,7 +356,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setBytes(int index, byte[] src, int srcIndex, int length) {
assert !freed;
checkUnfreed();
ByteBuffer tmpBuf = internalNioBuffer();
tmpBuf.clear().position(index).limit(index + length);
tmpBuf.put(src, srcIndex, length);
@ -366,7 +365,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setBytes(int index, ByteBuffer src) {
assert !freed;
checkUnfreed();
ByteBuffer tmpBuf = internalNioBuffer();
if (src == tmpBuf) {
src = src.duplicate();
@ -379,7 +378,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf getBytes(int index, OutputStream out, int length) throws IOException {
assert !freed;
checkUnfreed();
if (length == 0) {
return this;
}
@ -398,7 +397,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public int getBytes(int index, GatheringByteChannel out, int length) throws IOException {
assert !freed;
checkUnfreed();
if (length == 0) {
return 0;
}
@ -410,7 +409,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public int setBytes(int index, InputStream in, int length) throws IOException {
assert !freed;
checkUnfreed();
if (buffer.hasArray()) {
return in.read(buffer.array(), buffer.arrayOffset() + index, length);
} else {
@ -428,7 +427,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public int setBytes(int index, ScatteringByteChannel in, int length) throws IOException {
assert !freed;
checkUnfreed();
ByteBuffer tmpNioBuf = internalNioBuffer();
tmpNioBuf.clear().position(index).limit(index + length);
try {
@ -445,7 +444,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuffer nioBuffer(int index, int length) {
assert !freed;
checkUnfreed();
if (index == 0 && length == capacity()) {
return buffer.duplicate();
} else {
@ -460,7 +459,7 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public ByteBuf copy(int index, int length) {
assert !freed;
checkUnfreed();
ByteBuffer src;
try {
src = (ByteBuffer) internalNioBuffer().clear().position(index).limit(index + length);
@ -486,16 +485,18 @@ final class UnpooledDirectByteBuf extends AbstractByteBuf {
@Override
public boolean isFreed() {
return freed;
return buffer == null;
}
@Override
public void free() {
if (freed) {
ByteBuffer buffer = this.buffer;
if (buffer == null) {
return;
}
freed = true;
this.buffer = null;
if (doNotFree) {
return;
}

View File

@ -32,7 +32,6 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
private final ByteBufAllocator alloc;
private byte[] array;
private ByteBuffer tmpNioBuf;
private boolean freed;
/**
* Creates a new heap buffer with a newly allocated byte array.
@ -97,12 +96,13 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public int capacity() {
checkUnfreed();
return array.length;
}
@Override
public ByteBuf capacity(int newCapacity) {
assert !freed;
checkUnfreed();
if (newCapacity < 0 || newCapacity > maxCapacity()) {
throw new IllegalArgumentException("newCapacity: " + newCapacity);
}
@ -136,7 +136,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public byte[] array() {
assert !freed;
checkUnfreed();
return array;
}
@ -147,13 +147,13 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public byte getByte(int index) {
assert !freed;
checkUnfreed();
return array[index];
}
@Override
public ByteBuf getBytes(int index, ByteBuf dst, int dstIndex, int length) {
assert !freed;
checkUnfreed();
if (dst.hasArray()) {
getBytes(index, dst.array(), dst.arrayOffset() + dstIndex, length);
} else {
@ -164,41 +164,41 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuf getBytes(int index, byte[] dst, int dstIndex, int length) {
assert !freed;
checkUnfreed();
System.arraycopy(array, index, dst, dstIndex, length);
return this;
}
@Override
public ByteBuf getBytes(int index, ByteBuffer dst) {
assert !freed;
checkUnfreed();
dst.put(array, index, Math.min(capacity() - index, dst.remaining()));
return this;
}
@Override
public ByteBuf getBytes(int index, OutputStream out, int length) throws IOException {
assert !freed;
checkUnfreed();
out.write(array, index, length);
return this;
}
@Override
public int getBytes(int index, GatheringByteChannel out, int length) throws IOException {
assert !freed;
checkUnfreed();
return out.write((ByteBuffer) internalNioBuffer().clear().position(index).limit(index + length));
}
@Override
public ByteBuf setByte(int index, int value) {
assert !freed;
checkUnfreed();
array[index] = (byte) value;
return this;
}
@Override
public ByteBuf setBytes(int index, ByteBuf src, int srcIndex, int length) {
assert !freed;
checkUnfreed();
if (src.hasArray()) {
setBytes(index, src.array(), src.arrayOffset() + srcIndex, length);
} else {
@ -209,27 +209,27 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setBytes(int index, byte[] src, int srcIndex, int length) {
assert !freed;
checkUnfreed();
System.arraycopy(src, srcIndex, array, index, length);
return this;
}
@Override
public ByteBuf setBytes(int index, ByteBuffer src) {
assert !freed;
checkUnfreed();
src.get(array, index, src.remaining());
return this;
}
@Override
public int setBytes(int index, InputStream in, int length) throws IOException {
assert !freed;
checkUnfreed();
return in.read(array, index, length);
}
@Override
public int setBytes(int index, ScatteringByteChannel in, int length) throws IOException {
assert !freed;
checkUnfreed();
try {
return in.read((ByteBuffer) internalNioBuffer().clear().position(index).limit(index + length));
} catch (ClosedChannelException e) {
@ -244,7 +244,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuffer nioBuffer(int index, int length) {
assert !freed;
checkUnfreed();
return ByteBuffer.wrap(array, index, length);
}
@ -255,13 +255,13 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public short getShort(int index) {
assert !freed;
checkUnfreed();
return (short) (array[index] << 8 | array[index + 1] & 0xFF);
}
@Override
public int getUnsignedMedium(int index) {
assert !freed;
checkUnfreed();
return (array[index] & 0xff) << 16 |
(array[index + 1] & 0xff) << 8 |
array[index + 2] & 0xff;
@ -269,7 +269,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public int getInt(int index) {
assert !freed;
checkUnfreed();
return (array[index] & 0xff) << 24 |
(array[index + 1] & 0xff) << 16 |
(array[index + 2] & 0xff) << 8 |
@ -278,7 +278,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public long getLong(int index) {
assert !freed;
checkUnfreed();
return ((long) array[index] & 0xff) << 56 |
((long) array[index + 1] & 0xff) << 48 |
((long) array[index + 2] & 0xff) << 40 |
@ -291,7 +291,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setShort(int index, int value) {
assert !freed;
checkUnfreed();
array[index] = (byte) (value >>> 8);
array[index + 1] = (byte) value;
return this;
@ -299,7 +299,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setMedium(int index, int value) {
assert !freed;
checkUnfreed();
array[index] = (byte) (value >>> 16);
array[index + 1] = (byte) (value >>> 8);
array[index + 2] = (byte) value;
@ -308,7 +308,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setInt(int index, int value) {
assert !freed;
checkUnfreed();
array[index] = (byte) (value >>> 24);
array[index + 1] = (byte) (value >>> 16);
array[index + 2] = (byte) (value >>> 8);
@ -318,7 +318,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuf setLong(int index, long value) {
assert !freed;
checkUnfreed();
array[index] = (byte) (value >>> 56);
array[index + 1] = (byte) (value >>> 48);
array[index + 2] = (byte) (value >>> 40);
@ -332,7 +332,7 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public ByteBuf copy(int index, int length) {
assert !freed;
checkUnfreed();
if (index < 0 || length < 0 || index + length > array.length) {
throw new IndexOutOfBoundsException("Too many bytes to copy - Need "
+ (index + length) + ", maximum is " + array.length);
@ -353,12 +353,12 @@ final class UnpooledHeapByteBuf extends AbstractByteBuf {
@Override
public boolean isFreed() {
return freed;
return array == null;
}
@Override
public void free() {
freed = true;
array = null;
}
@Override