kinow commented on a change in pull request #53: Close OutputStream before
toByteArray() on underlying ByteArrayOutputStream
URL: https://github.com/apache/commons-imaging/pull/53#discussion_r320098729
##########
File path:
src/main/java/org/apache/commons/imaging/formats/tiff/write/TiffImageWriterLossy.java
##########
@@ -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();
Review comment:
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 -
https://github.com/apache/commons-imaging/blob/8ee267db7a05d26f66e5f0b0ec7af6b813f99345/src/main/java/org/apache/commons/imaging/common/BinaryOutputStream.java#L71
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
