-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
bugfix: BufferOverflow when BranchSession size too large #1552
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1552 +/- ##
=============================================
- Coverage 46.44% 46.41% -0.04%
Complexity 1717 1717
=============================================
Files 350 350
Lines 12847 12847
Branches 1618 1618
=============================================
- Hits 5967 5963 -4
- Misses 6232 6234 +2
- Partials 648 650 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (bs.length + 4 )> int.max, how to process?
it's origin code.
bytebuffer will write extra 4 byte content (bs.length), but when allocate DirectByteBuffer, the length of the DirectByteBuffer is bs.length ( should be bs.length +4 ) |
@finalcola public static ByteBuffer allocateDirect(int capacity) accept a int param, if bs.len<int.max but bs.len+4>int.max , it need to judge? |
MAX_WRITE_BUFFER_SIZE is 16KB not int.max. |
@finalcola if (bs.length=int.max-1 ) ,can it run normally? |
it can't , because byteBuffer always write extra 4 byte content(Larger than the allocated buffer). |
So do we need to add code to judge bs.length over int.max? |
no,it's already judged before enter this method. |
|
so we should judge bs.length<=int.max-4? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Ⅰ. Describe what this PR did
fix BufferOverflowException when FileTransactionStoreManager write a big BranchSession(size > 16KB).
Ⅱ. Does this pull request fix one issue?
fixes #1551
#1551 (comment)
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews