You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Phil D (Jira)" <ji...@apache.org> on 2023/08/31 19:23:00 UTC
[jira] [Updated] (IO-808) FileUtils.moveFile, copyFile and others can throw undocumened IllegalArgumentException
[ https://issues.apache.org/jira/browse/IO-808?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil D updated IO-808:
----------------------
Description:
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() 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.
was:
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 validateMoveParameters() and requireFile()
{code}
This pattern of calling validateMoveParameters() and requireFile() will throw IllegalArgumentException every when the srcFile is removed between between validateMoveParameters() 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.
> 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: 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() 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)