[
https://issues.apache.org/jira/browse/IO-808?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17799024#comment-17799024
]
Elliotte Rusty Harold commented on IO-808:
------------------------------------------
After reflection and experimentation, I now suspect that this and similar
methods should not throw IllegalArgumentException, at all. These should all be
some form of IOException since they do not indicate a programming error such as
passing an invalid value as an argument. Instead they indicate a change in the
external environment where something that is thought to be a file or a
directory is now a directory or file respectively. Since this is not something
that can be known at compile time, and indeed as this bug indicates is
something that may change while a program is executing, these should all be
IOExceptions.
> FileUtils.moveFile, copyFile and others can throw undocumened
> IllegalArgumentException
> --------------------------------------------------------------------------------------
>
> Key: IO-808
> URL: https://issues.apache.org/jira/browse/IO-808
> Project: Commons IO
> Issue Type: Bug
> Components: Utilities
> Affects Versions: 2.12.0
> Environment: Windows 10
> Reporter: Phil D
> Priority: Major
> Attachments: MakyAckyBreaky.java, TestMoveFileIAE.java
>
>
> Several of the functions in FileUtils are throwing undocumented
> IllegalArgumentException such as moveFile, copyFile and other locations.
> If the desire is to maintain backwards compatibility with the 1.4 branch for
> these functions, then the 2.12 (and 2.13) versions are throwing
> IllegalArgumentException in cases where 1.4 is not. In fact, it seems like
> 1.4 was coded to specifically avoid IllegalArgumentException and throws
> IOExceptions instead.
> There are several different cases where this is possible. In the most basic,
> I've attached TestMoveFileIAE, where this can be reproduced by simple running:
> {code:bash}
> mkdir one
> java -cp <your_classpath> TestMoveFileIAE one two
> Exception in thread "main" java.lang.IllegalArgumentException: Parameter
> 'srcFile' is not a file: one
> at org.apache.commons.io.FileUtils.requireFile(FileUtils.java:2824)
> at org.apache.commons.io.FileUtils.moveFile(FileUtils.java:2395)
> at org.apache.commons.io.FileUtils.moveFile(FileUtils.java:2374)
> at TestMoveFileIAE.main(TestMoveFileIAE.java:13)
> {code}
> In a less likely scenario (which is how I found this issue because this
> happened on a production system); If the srcFile is removed at a certain
> point during moveFile() execution then IllegalArgumentException is throws:
> https://github.com/apache/commons-io/blob/master/src/main/java/org/apache/commons/io/FileUtils.java#L2392
> {code:java}
> 2392 public static void moveFile(final File srcFile, final File destFile,
> final CopyOption... copyOptions) throws IOException {
> 2393 validateMoveParameters(srcFile, destFile); // checks srcFile.exists()
> /// ==== srcFile deleted here!!!
> 2394 requireFile(srcFile, "srcFile"); // checks srcFile.isFile()
> and throws IAE
> 2395 requireAbsent(destFile, "destFile");
> /// ==== srcFile could also be deleted here
> 2396 ... // renameTo or copyFile() which also calls requireCopyFile() and
> requireFile()
> {code}
> This pattern of calling validateMoveParameters() and requireFile() will throw
> IllegalArgumentException every when the srcFile is removed between between
> validateMoveParameters() and requireFile() or requireFileCopy() and
> requireFile()
> Preferably, it would be best if the 2.x versions of FileUtils were backwards
> compatible with 1.x and IllegalArgumentException would not be thrown, but
> IOException (or one of its derivatives) would be. IAE is an unchecked
> exception and can cause unexpected issues.
> I would also suggest that unit tests be created to ensure that these
> functions behave as expected in error conditions.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
