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

Close OutputStream before toByteArray() on underlying ByteArrayOutputStream #53

Closed
wants to merge 1 commit into from

Conversation

emopers
Copy link

@emopers emopers commented Sep 3, 2019

When an OutputStream instance wraps an underlying ByteArrayOutputStream instance,
it is recommended to flush or close the OutputStream before invoking the underlying instances' toByteArray().

This pull request adds a call to close at the end of the write method of TiffImageWriterLossy.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 74.417% when pulling f1435b8 on emopers:bug_fix into 8ee267d on apache:master.

Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

The stream is left open intentionally.

@@ -50,6 +50,8 @@ public void write(final OutputStream os, final TiffOutputSet outputSet)
final BinaryOutputStream bos = new BinaryOutputStream(os, byteOrder);

writeStep(bos, outputItems);

bos.close();
Copy link
Member

Choose a reason for hiding this comment

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

Hi @emopers

You are correct on that. The issue on this method (and I think there might be other places in Imaging with the same issue) is that this was by design.

The BinaryOutputStream created here wraps the parameter os, an OutputStream passed by the user. We shouldn't close the stream that the user passed, but calling BinaryOutputStream#close will close the underlying stream as well -

That's why some time ago, when we fixed several cases of resources left open by using the try-with-resources trick, we left this one as-is.

@kinow
Copy link
Member

kinow commented Sep 24, 2019

Closing as per comment above. I've added a comment (b684eea) to let other developers know that the resource is intentionally left open. Thanks!

@kinow kinow closed this Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants