Skip to content

Commit

Permalink
SimpleMergedByteBuffers: ArrayOutOfBoundsFix
Browse files Browse the repository at this point in the history
This fixes SimpleMergedByteBuffers.getNextBuffer to avoid an ArrayOutOfBounds fix when the buffer has been fully consumed.
We must always check the currentBuffer index before we can attempt to access the array.
In addition the EMPTY_BUFFER_ARRAY can not contain the empty buffer because it makes logic which compares the array length invalid.
  • Loading branch information
jentfoo committed Oct 25, 2019
1 parent 7d9d335 commit 9f78665
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 23 deletions.
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
group = org.threadly
version = 4.12-SNAPSHOT
version = 4.12
threadlyVersion = 5.41
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*
*/
public class SimpleMergedByteBuffers extends AbstractMergedByteBuffers {
private static final ByteBuffer[] EMPTY_BUFFER_ARRAY = new ByteBuffer[] {IOUtils.EMPTY_BYTEBUFFER};
private static final ByteBuffer[] EMPTY_BUFFER_ARRAY = new ByteBuffer[] {};


private final ByteBuffer[] bba;
Expand Down Expand Up @@ -66,14 +66,12 @@ private void doGet(final byte[] destBytes, int start, int len) {
}

private ByteBuffer getNextBuffer() {
while(!this.bba[this.currentBuffer].hasRemaining() && bba.length > currentBuffer+1 ) {
this.bba[this.currentBuffer] = null;
ByteBuffer nextBuffer = IOUtils.EMPTY_BYTEBUFFER;
while(bba.length > currentBuffer && ! (nextBuffer = bba[currentBuffer]).hasRemaining()) {
bba[currentBuffer] = null;
currentBuffer++;
}
if(bba[this.currentBuffer].hasRemaining()) {
return bba[currentBuffer];
}
return IOUtils.EMPTY_BYTEBUFFER;
return nextBuffer;
}

@Override
Expand Down Expand Up @@ -138,9 +136,11 @@ public int nextBufferSize() {
@Override
public ByteBuffer popBuffer() {
ByteBuffer bb = getNextBuffer();
consumedSize += bb.remaining();
currentBuffer++;
return bb.duplicate();
if (bb.hasRemaining()) {
consumedSize += bb.remaining();
currentBuffer++;
}
return bb;
}

@Override
Expand All @@ -154,12 +154,7 @@ public int remaining() {

@Override
public boolean hasRemaining() {
for(int i=currentBuffer; i<bba.length; i++) {
if(bba[i].hasRemaining()) {
return true;
}
}
return false;
return getNextBuffer().hasRemaining();
}

@Override
Expand Down Expand Up @@ -236,11 +231,11 @@ public void discardFromEnd(int size) {
@Override
protected byte get(int pos) {
int currentPos = 0;
for(ByteBuffer bb: this.bba) {
if(bb != null && bb.remaining() > pos-currentPos) {
return bb.get(pos-currentPos);
for (int i = currentBuffer; i < this.bba.length; i++) {
if(this.bba[i].remaining() > pos-currentPos) {
return this.bba[i].get(pos-currentPos);
} else {
currentPos+=bb.remaining();
currentPos+=this.bba[i].remaining();
}
}
return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ public void GetArrayOffset() throws IOException {

@Test
public void getInts() {

ByteBuffer[] bba = new ByteBuffer[200];
for(int i = 0; i<200; i++) {
ByteBuffer bb = ByteBuffer.allocate(4);
Expand Down Expand Up @@ -238,7 +237,6 @@ public void popZeroBuffer() {

@Test
public void popBuffer() {

Random rnd = new Random();
int size = Math.abs(rnd.nextInt(300))+10;
MergedByteBuffers mbb = new SimpleMergedByteBuffers(false, ByteBuffer.allocate(size), ByteBuffer.allocate(rnd.nextInt(300)), ByteBuffer.allocate(rnd.nextInt(300)), ByteBuffer.allocate(rnd.nextInt(300)));
Expand Down

0 comments on commit 9f78665

Please sign in to comment.