Skip to content

ByteBuf#ensureWritable(int, boolean) should not throw#6721

Closed
Scottmitch wants to merge 1 commit into
netty:4.1from
Scottmitch:buffer_ensurewritable_bug
Closed

ByteBuf#ensureWritable(int, boolean) should not throw#6721
Scottmitch wants to merge 1 commit into
netty:4.1from
Scottmitch:buffer_ensurewritable_bug

Conversation

@Scottmitch
Copy link
Copy Markdown
Member

Motivation:
The javadocs for ByteBuf#ensureWritable(int, boolean) indicate that it should not throw, and instead the return code should indicate the result of the operation. Due to a bug in AbstractByteBuf it is possible for a resize to be attempted on a buffer that may exceed maxCapacity() and therefore throw.

Modifications:

  • If there is not enough space in the buffer, and force is false, then a resize should not be attempted

Result:
AbstractByteBuf#ensureWritable(int, boolean) enforces the javadoc constraints and does not throw.

Motivation:
The javadocs for ByteBuf#ensureWritable(int, boolean) indicate that it should not throw, and instead the return code should indicate the result of the operation. Due to a bug in AbstractByteBuf it is possible for a resize to be attempted on a buffer that may exceed maxCapacity() and therefore throw.

Modifications:
- If there is not enough space in the buffer, and force is false, then a resize should not be attempted

Result:
AbstractByteBuf#ensureWritable(int, boolean) enforces the javadoc constraints and does not throw.
Copy link
Copy Markdown
Member

@normanmaurer normanmaurer left a comment

Choose a reason for hiding this comment

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

LGTM... good catch

@Scottmitch
Copy link
Copy Markdown
Member Author

4.1 (63f5cdb) 4.0 (86ce1ef)

@Scottmitch Scottmitch closed this May 9, 2017
@Scottmitch Scottmitch deleted the buffer_ensurewritable_bug branch May 9, 2017 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants