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

Journal scan should throw IOException when it reads negative length #2176

Closed
karanmehta93 opened this issue Oct 11, 2019 · 6 comments
Closed

Comments

@karanmehta93
Copy link
Member

BUG REPORT

Describe the bug

During journal replay, we can encounter negative length value if there is journal corruption. Currently, due to this bug, we pass negative length to limit a buffer, which throws IllegalArgumentException.

To Reproduce

Minor bug, good to have for debugging.

Expected behavior

Error handling should be done via IOExpection, and it should be thrown instead.

@eolivelli
Copy link
Contributor

can you add version and stacktrace please? maybe FS type

In my opinion the fix is simple

reddycharan pushed a commit that referenced this issue Oct 24, 2019
Descriptions of the changes in this PR:

### Motivation

During journal replay, we can encounter negative length value if there is journal corruption. Currently, due to this bug, we pass negative length to limit a buffer, which throws IllegalArgumentException.

### Changes

Updated the Journal class to throw `IOException` with a clear message, instead of unclear `IllegalArgumentException`.

Master Issue: #2176


Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Charan Reddy Guttapalem <reddycharan18@gmail.com>

This closes #2177 from karanmehta93/master
@atris
Copy link
Contributor

atris commented Jan 25, 2020

Can I please be assigned this issue?

@karanmehta93
Copy link
Member Author

@atris Please verify if there is an PR hanging out there, otherwise you can take it up!

@eolivelli
Copy link
Contributor

Unfortunately we can't assign github issues to people who never contributed to the repository, or maybe even to non "committers"
anyway a PR will be very appreciated !
thank you @atris

@atris
Copy link
Contributor

atris commented Jan 25, 2020

Ok, I am working on this one.

@eolivelli
Copy link
Contributor

@atris it looks like we already merged a fix for this problem
#2177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants