-
-
Notifications
You must be signed in to change notification settings - Fork 347
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
refactor: close resource in ZipFolder.java #2623
refactor: close resource in ZipFolder.java #2623
Conversation
LGTM. @pvojtechovsky do you merge? |
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.
I added failing test and fix for the mentioned problem
ZipEntry entry; | ||
while ((entry = zipInput.getNextEntry()) != null) { | ||
// deflate in buffer | ||
final int buffer = 2048; | ||
ByteArrayOutputStream output = new ByteArrayOutputStream(buffer); |
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.
This code seems to be wrong.
The content of output will grow with each zip entry. Before, it was reset with each entry.
Solution:
A) create ByteArrayOutputStream in scope of while body, similar like before
B) call ByteArrayOutputStream#reset()
method to clear it's content
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.
Thank you Pavel. I felt there is something wrong with it.
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.
Please consider using IOUtils from Apache.
@monperrus please review my changes too and merge it - if ok. Thank you. |
Thanks for the added test, a new test is always valuable. |
Make sure a resource is properly released.
@pvojtechovsky
Pavel could you take a look on public List getFiles() and tell me if this algorithm is correct before and after change.