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.

Sign up for GitHub

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

Remove dangerous ByteBufStreamInput methods #27076

Merged
merged 3 commits into from
Oct 24, 2017
Merged

Remove dangerous ByteBufStreamInput methods #27076

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10082be
Remove dangerous `ByteBufStreamInput` methods
Tim-Brooks Oct 22, 2017
8685ed5
Refactor variable
Tim-Brooks Oct 22, 2017
b0f364a
Add explanation
Tim-Brooks Oct 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions ...transport-netty4/src/main/java/org/elasticsearch/transport/netty4/ByteBufStreamInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,36 +33,18 @@
class ByteBufStreamInput extends StreamInput {

private final ByteBuf buffer;
private final int startIndex;
private final int endIndex;

ByteBufStreamInput(ByteBuf buffer, int length) {
if (length > buffer.readableBytes()) {
throw new IndexOutOfBoundsException();
}
this.buffer = buffer;
startIndex = buffer.readerIndex();
int startIndex = buffer.readerIndex();
endIndex = startIndex + length;
buffer.markReaderIndex();
}

@Override
public BytesReference readBytesReference(int length) throws IOException {
BytesReference ref = Netty4Utils.toBytesReference(buffer.slice(buffer.readerIndex(), length));
buffer.skipBytes(length);
return ref;
}

@Override
public BytesRef readBytesRef(int length) throws IOException {
if (!buffer.hasArray()) {
return super.readBytesRef(length);
}
BytesRef bytesRef = new BytesRef(buffer.array(), buffer.arrayOffset() + buffer.readerIndex(), length);
buffer.skipBytes(length);
return bytesRef;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we keep them, delegating to the parent impl with a comment, eg.

     @Override
    public BytesReference readBytesReference(int length) throws IOException {
        // NOTE: It is unsafe to share a reference of the internal structure, so we
        // need to return a copy, like the default implementation does.
        return super.readBytesReference(length);
    }

My motivation is that it won't be possible to optimize it unsafely in the future without seeing that comment.


@Override
public int available() throws IOException {
return endIndex - buffer.readerIndex();
Expand Down