Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible unsafe sharing of internal NIO buffer in CompositeByteBuf #9169

Merged
merged 1 commit into from May 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 11 additions & 2 deletions buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
Expand Up @@ -1606,8 +1606,7 @@ public ByteBuffer internalNioBuffer(int index, int length) {
case 0:
return EMPTY_NIO_BUFFER;
case 1:
Component c = components[0];
return c.buf.internalNioBuffer(c.idx(index), length);
return components[0].internalNioBuffer(index, length);
default:
throw new UnsupportedOperationException();
}
Expand Down Expand Up @@ -1889,6 +1888,16 @@ ByteBuf duplicate() {
return buf.duplicate().setIndex(idx(offset), idx(endOffset));
}

ByteBuffer internalNioBuffer(int index, int length) {
// We must not return the unwrapped buffer's internal buffer
// if it was originally added as a slice - this check of the
// slice field is threadsafe since we only care whether it
// was set upon Component construction, and we aren't
// attempting to access the referenced slice itself
return slice != null ? buf.nioBuffer(idx(index), length)
: buf.internalNioBuffer(idx(index), length);
}

void free() {
// Release the slice if present since it may have a different
// refcount to the unwrapped buf if it is a PooledSlicedByteBuf
Expand Down
Expand Up @@ -977,7 +977,27 @@ private static void testGatheringWritesSingleBuf(ByteBuf buf1) throws Exception
@Override
@Test
public void testInternalNioBuffer() {
// ignore
CompositeByteBuf buf = compositeBuffer();
assertEquals(0, buf.internalNioBuffer(0, 0).remaining());

// If non-derived buffer is added, its internal buffer should be returned
ByteBuf concreteBuffer = directBuffer().writeByte(1);
buf.addComponent(concreteBuffer);
assertSame(concreteBuffer.internalNioBuffer(0, 1), buf.internalNioBuffer(0, 1));
buf.release();

// In derived cases, the original internal buffer must not be used
buf = compositeBuffer();
concreteBuffer = directBuffer().writeByte(1);
buf.addComponent(concreteBuffer.slice());
assertNotSame(concreteBuffer.internalNioBuffer(0, 1), buf.internalNioBuffer(0, 1));
buf.release();

buf = compositeBuffer();
concreteBuffer = directBuffer().writeByte(1);
buf.addComponent(concreteBuffer.duplicate());
assertNotSame(concreteBuffer.internalNioBuffer(0, 1), buf.internalNioBuffer(0, 1));
buf.release();
}

@Test
Expand Down