[prev in list] [next in list] [prev in thread] [next in thread]
List: jakarta-commons-dev
Subject: [jira] [Comment Edited] (IO-808) FileUtils.moveFile, copyFile and others can throw undocumened Illeg
From: "Phil D (Jira)" <jira () apache ! org>
Date: 2023-08-31 19:59:00
Message-ID: JIRA.13549323.1693509708000.48079.1693511940014 () Atlassian ! JIRA
[Download RAW message or body]
[ https://issues.apache.org/jira/browse/IO-808?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17761077#comment-17761077 \
]
Phil D edited comment on IO-808 at 8/31/23 7:58 PM:
----------------------------------------------------
While I get that, I don't think this was an intended change because:
(1) The version upgrade notes from 1.4 to 2.0 \
https://commons.apache.org/proper/commons-io/upgradeto2_0.html does not list this as \
a change. In fact it lists 1.4 to 2.0 as source compatible. This change is not \
source compatible, it requires catching IllegalArgumentException in places where it \
didn't previously need to be. (2) The javadoc does not have a \
IllegalArgumentException as a documented exception for these functions (3) In cases \
where 1.4 would throw IOException, now IllegalArgumentExceptions are now thrown. As \
mentioned above this is not source compatible and can cause unexpected behavior of \
applications.
was (Author: JIRAUSER302068):
While I get that, I don't think this was an intended change because:
(1) The version upgrade notes from 1.4 to 2.0 \
https://commons.apache.org/proper/commons-io/upgradeto2_0.html does not list this as \
a change. In fact it lists 1.4 to 2.0 as source compatible. This change is not \
source compatible, it requires catching IllegalArgumentException in places where it \
didn't previously need to be. (2) The javadoc does not have a \
IllegalArgumentException as a documented exception (3) In cases where 1.4 would throw \
IOException, now IllegalArgumentExceptions are now thrown. As mentioned above this \
is not source compatible and can cause unexpected behavior of applications.
> 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() 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)
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic