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 ReflectiveByteBufChecksum with direct buffers #9244

Merged

Conversation

Projects
None yet
3 participants
@iamaleksey
Copy link
Contributor

commented Jun 17, 2019

Motivation:

ReflectiveByteBufChecksum#update(buf, off, len) ignores provided offset
and length arguments when operating on direct buffers, leading to wrong
byte sequences being checksummed and ultimately incorrect checksum
values (unless checksumming the entire buffer).

Modifications:

Use the provided offset and length arguments to get the correct nio
buffer to checksum; add test coverage exercising the four meaningfully
different offset and length combinations.

Result:

Offset and length are respected and a correct checksum gets calculated;
simple unit test should prevent regressions in the future.

Fix ReflectiveByteBufChecksum with direct buffers
Motivation:

ReflectiveByteBufChecksum#update(buf, off, len) ignores provided offset
and length arguments when operating on direct buffers, leading to wrong
byte sequences being checksummed and ultimately incorrect checksum
values (unless checksumming the entire buffer).

Modifications:

Use the provided offset and length arguments to get the correct nio
buffer to checksum; add test coverage exercising the four meaningfully
different offset and length combinations.

Result:

Offset and length are respected and a correct checksum gets calculated;
simple unit test should prevent regressions in the future.
@netty-bot

This comment has been minimized.

Copy link

commented Jun 17, 2019

Can one of the admins verify this patch?

@iamaleksey

This comment has been minimized.

Copy link
Contributor Author

commented Jun 17, 2019

Addressed in 0cafe54

@normanmaurer

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

@netty-bot test this please

@normanmaurer

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

test-failure is not related... merging this.

@normanmaurer normanmaurer merged commit a2583d0 into netty:4.1 Jun 17, 2019

2 of 3 checks passed

pull request validation (centos6-java11) Build finished.
Details
pull request validation (centos6-java12) Build finished.
Details
pull request validation (centos6-java8) Build finished.
Details

@normanmaurer normanmaurer added this to the 4.1.37.Final milestone Jun 17, 2019

normanmaurer added a commit that referenced this pull request Jun 17, 2019

Fix ReflectiveByteBufChecksum with direct buffers (#9244)
Motivation:

ReflectiveByteBufChecksum#update(buf, off, len) ignores provided offset
and length arguments when operating on direct buffers, leading to wrong
byte sequences being checksummed and ultimately incorrect checksum
values (unless checksumming the entire buffer).

Modifications:

Use the provided offset and length arguments to get the correct nio
buffer to checksum; add test coverage exercising the four meaningfully
different offset and length combinations.

Result:

Offset and length are respected and a correct checksum gets calculated;
simple unit test should prevent regressions in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.