[
https://issues.apache.org/jira/browse/IO-554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16506882#comment-16506882
]
Bruno P. Kinoshita commented on IO-554:
---------------------------------------
TL;DR;
* *2.5 did not close the stream*
* *2.6 closed the stream after the introduction of try-with-resources*
* *This issue is about the changed behaviour, with a pull request to revert it
back to 2.5 behaviour*
Recapitulating;
I believe the *copyToFile* method was released with Release 2.5 – 2016-04-22,
added in this commit from IO-381 from May 2013
[https://github.com/apache/commons-io/commit/ee2f71d9fd2ce253f53de137950fa90087b9f565]
The code from the 2.5 released tag reads:
{code:java}
/**
* Copies bytes from an {@link InputStream} <code>source</code> to a file
* <code>destination</code>. The directories up to <code>destination</code>
* will be created if they don't already exist. <code>destination</code>
* will be overwritten if it already exists.
* The {@code source} stream is left open, e.g. for use with {@link
java.util.zip.ZipInputStream ZipInputStream}.
* See {@link #copyInputStreamToFile(InputStream, File)} for a method that
closes the input stream.
*
* @param source the <code>InputStream</code> to copy bytes from, must
not be {@code null}
* @param destination the non-directory <code>File</code> to write bytes to
* (possibly overwriting), must not be {@code null}
* @throws IOException if <code>destination</code> is a directory
* @throws IOException if <code>destination</code> cannot be written
* @throws IOException if <code>destination</code> needs creating but can't
be
* @throws IOException if an IO error occurs during copying
* @since 2.5
*/
public static void copyToFile(final InputStream source, final File
destination) throws IOException {
final FileOutputStream output = openOutputStream(destination);
try {
IOUtils.copy(source, output);
output.close(); // don't swallow close Exception if copy completes
normally
} finally {
IOUtils.closeQuietly(output);
}
}
{code}
NB the Javadocs states "*The \{@code source} stream is left open*". I had a
look at the code, and tested in Eclipse, and it is indeed left open.
But on this commit from May 2016, released later with Release 2.6 –
2017-10-15, we added try-with-resources, which changed the behaviour of the
code.
The code from IO-505 from the 2.6 released tag_**_ reads:
{code:java}
/**
* Copies bytes from an {@link InputStream} <code>source</code> to a file
* <code>destination</code>. The directories up to <code>destination</code>
* will be created if they don't already exist. <code>destination</code>
* will be overwritten if it already exists.
* The {@code source} stream is left open, e.g. for use with {@link
java.util.zip.ZipInputStream ZipInputStream}.
* See {@link #copyInputStreamToFile(InputStream, File)} for a method that
closes the input stream.
*
* @param source the <code>InputStream</code> to copy bytes from, must
not be {@code null}
* @param destination the non-directory <code>File</code> to write bytes to
* (possibly overwriting), must not be {@code null}
* @throws IOException if <code>destination</code> is a directory
* @throws IOException if <code>destination</code> cannot be written
* @throws IOException if <code>destination</code> needs creating but can't
be
* @throws IOException if an IO error occurs during copying
* @since 2.5
*/
public static void copyToFile(final InputStream source, final File
destination) throws IOException {
try (InputStream in = source;
OutputStream out = openOutputStream(destination)) {
IOUtils.copy(in, out);
}
}
{code}
NB the Javadocs states "*The \{@code source} stream is left open*". But the
try-with-resources closes the in/source input stream. I think this change in
the behaviour was not intentional.
The pull request for this issue is proposing to revert what was added in 2.6. I
would not use commons-io-2.6 `copyToFile`, as I would normally handle the
stream myself, or use the other method that already closed the stream before.
[~tmortagne],
>I agree that behavior should not change but that's actually an argument for a
>bugfix release as fast as possible, not to keep a regression.
+1
In a similar case, Commons Pool 2.4.3 kept binary compatibility but changed the
way the code worked when iterating stack traces (see POOL-320), which caused an
issue and prevented us from using this version in Commons DBCP. Instead of
keeping the method and adding a new one, or updating documentation, we reverted
the change in 2.5.0 (see POOL-335).
In my opinion we should revert the regression added in 2.6, with [~mmariotti]
's pull request suggested, which also includes a unit test to prevent the
regression from happening again.
Cheers
Bruno
(**) There is a tag commons-io-2.5, but no commons-io-2.6, only RC1, RC2, and
commons-io-2.6-RC3.
> FileUtils.copyToFile(InputStream source, File destination) closes input stream
> ------------------------------------------------------------------------------
>
> Key: IO-554
> URL: https://issues.apache.org/jira/browse/IO-554
> Project: Commons IO
> Issue Type: Bug
> Components: Streams/Writers
> Affects Versions: 2.6
> Reporter: Michele Mariotti
> Assignee: Bruno P. Kinoshita
> Priority: Blocker
> Labels: regression
> Fix For: 2.7
>
>
> In 2.6 this method is closing the input stream, while the javadoc states the
> opposite.
> The correct behavior is to leave the stream open, as stated in the javadoc.
> I assigned a high priority because this incorrect behavior breaks existing
> code, especially when used in combination with ZipInputStream.
> {code:java}
> /**
> * Copies bytes from an {@link InputStream} <code>source</code> to a file
> * <code>destination</code>. The directories up to <code>destination</code>
> * will be created if they don't already exist. <code>destination</code>
> * will be overwritten if it already exists.
> * The {@code source} stream is left open, e.g. for use with {@link
> java.util.zip.ZipInputStream ZipInputStream}.
> * See {@link #copyInputStreamToFile(InputStream, File)} for a method that
> closes the input stream.
> *
> * @param source the <code>InputStream</code> to copy bytes from, must
> not be {@code null}
> * @param destination the non-directory <code>File</code> to write bytes to
> * (possibly overwriting), must not be {@code null}
> * @throws IOException if <code>destination</code> is a directory
> * @throws IOException if <code>destination</code> cannot be written
> * @throws IOException if <code>destination</code> needs creating but can't be
> * @throws IOException if an IO error occurs during copying
> * @since 2.5
> */
> public static void copyToFile(final InputStream source, final File
> destination) throws IOException {
> try (InputStream in = source;
> OutputStream out = openOutputStream(destination)) {
> IOUtils.copy(in, out);
> }
> }
> {code}
> instead it should be:
> {code:java}
> public static void copyToFile(final InputStream source, final File
> destination) throws IOException {
> try (OutputStream out = openOutputStream(destination)) {
> IOUtils.copy(source, out);
> }
> }{code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
