[prev in list] [next in list] [prev in thread] [next in thread] 

List:       jakarta-commons-dev
Subject:    [jira] [Commented] (IO-808) FileUtils.moveFile, copyFile and others can throw undocumened IllegalArg
From:       "Phil D (Jira)" <jira () apache ! org>
Date:       2023-08-31 19:58:00
Message-ID: JIRA.13549323.1693509708000.48078.1693511880216 () 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 commented on IO-808:
---------------------------

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]