[IO-808] non writeable destinations are I/O errors (#582)

* non writeable destinations are I/O errors

* remove requireCanWrite

Author: elharo

src/main/java/org/apache/commons/io/FileUtils.java

@@ -518,9 +518,9 @@ public static File[] convertFileCollectionToFileArray(final Collection<File> fil
      * @param destDir the new directory, must not be {@code null}.
      * @throws NullPointerException if any of the given {@link File}s are {@code null}.
      * @throws IllegalArgumentException if {@code srcDir} exists but is not a directory,
-     *     the source and the destination directory are the same, or the destination is not writable
+     *     the source and the destination directory are the same
      * @throws FileNotFoundException if the source does not exist.
-     * @throws IOException if an error occurs or setting the last-modified time didn't succeed.
+     * @throws IOException if an error occurs, the destination is not writable, or setting the last-modified time didn't succeed
      * @since 1.1
      */
     public static void copyDirectory(final File srcDir, final File destDir) throws IOException {
@@ -545,10 +545,10 @@ public static void copyDirectory(final File srcDir, final File destDir) throws I
      * @param srcDir an existing directory to copy, must not be {@code null}.
      * @param destDir the new directory, must not be {@code null}.
      * @param preserveFileDate true if the file date of the copy should be the same as the original.
-     * @throws IllegalArgumentException if {@code srcDir} exists but is not a directory,
-     *     the source and the destination directory are the same, or the destination is not writable
+     * @throws IllegalArgumentException if {@code srcDir} exists but is not a directory, or
+     *     the source and the destination directory are the same
      * @throws FileNotFoundException if the source does not exist.
-     * @throws IOException if an error occurs or setting the last-modified time didn't succeed.
+     * @throws IOException if an error occurs, the destination is not writable, or setting the last-modified time didn't succeed
      * @since 1.1
      */
     public static void copyDirectory(final File srcDir, final File destDir, final boolean preserveFileDate)
@@ -595,10 +595,10 @@ public static void copyDirectory(final File srcDir, final File destDir, final bo
      * @param destDir the new directory, must not be {@code null}.
      * @param filter the filter to apply, null means copy all directories and files should be the same as the original.
      * @throws NullPointerException if any of the given {@link File}s are {@code null}.
-     * @throws IllegalArgumentException if {@code srcDir} exists but is not a directory,
-     *     the source and the destination directory are the same, or the destination is not writable
+     * @throws IllegalArgumentException if {@code srcDir} exists but is not a directory, or
+     *     the source and the destination directory are the same
      * @throws FileNotFoundException if the source does not exist.
-     * @throws IOException if an error occurs or setting the last-modified time didn't succeed.
+     * @throws IOException if an error occurs, the destination is not writable, or setting the last-modified time didn't succeed
      * @since 1.4
      */
     public static void copyDirectory(final File srcDir, final File destDir, final FileFilter filter)
@@ -699,10 +699,10 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi
      * @param preserveFileDate true if the file date of the copy should be the same as the original
      * @param copyOptions options specifying how the copy should be done, for example {@link StandardCopyOption}.
      * @throws NullPointerException if any of the given {@link File}s are {@code null}.
-     * @throws IllegalArgumentException if {@code srcDir} exists but is not a directory,
-     *     the source and the destination directory are the same, or the destination is not writable
+     * @throws IllegalArgumentException if {@code srcDir} exists but is not a directory, or
+     *     the source and the destination directory are the same
      * @throws FileNotFoundException if the source does not exist.
-     * @throws IOException if an error occurs or setting the last-modified time didn't succeed.
+     * @throws IOException if an error occurs, the destination is not writable, or setting the last-modified time didn't succeed
      * @since 2.8.0
      */
     public static void copyDirectory(final File srcDir, final File destDir, final FileFilter fileFilter, final boolean preserveFileDate,
@@ -749,7 +749,7 @@ public static void copyDirectory(final File srcDir, final File destDir, final Fi
      * @throws NullPointerException if any of the given {@link File}s are {@code null}.
      * @throws IllegalArgumentException if the source or destination is invalid.
      * @throws FileNotFoundException if the source does not exist.
-     * @throws IOException if an error occurs or setting the last-modified time didn't succeed.
+     * @throws IOException if an error occurs, the destination is not writable, or setting the last-modified time didn't succeed
      * @since 1.2
      */
     public static void copyDirectoryToDirectory(final File sourceDir, final File destinationDir) throws IOException {
@@ -838,9 +838,10 @@ public static void copyFile(final File srcFile, final File destFile, final boole
      * @param copyOptions options specifying how the copy should be done, for example {@link StandardCopyOption}.
      * @throws NullPointerException if any of the given {@link File}s are {@code null}.
      * @throws FileNotFoundException if the source does not exist.
-     * @throws IllegalArgumentException if {@code srcFile} or {@code destFile} is not a file, or {@code destFile} is not writable
+     * @throws IllegalArgumentException if {@code srcFile} or {@code destFile} is not a file
      * @throws IOException if the output file length is not the same as the input file length after the copy completes.
-     * @throws IOException if an I/O error occurs, or setting the last-modified time didn't succeed.
+     * @throws IOException if an I/O error occurs, setting the last-modified time didn't succeed,
+     *     or the destination is not writable
      * @see #copyFileToDirectory(File, File, boolean)
      * @since 2.8.0
      */
@@ -851,7 +852,6 @@ public static void copyFile(final File srcFile, final File destFile, final boole
         createParentDirectories(destFile);
         if (destFile.exists()) {
             checkFileExists(destFile, "destFile");
-            requireCanWrite(destFile, "destFile");
         }
 
         final Path srcPath = srcFile.toPath();
@@ -1347,7 +1347,6 @@ private static void doCopyDirectory(final File srcDir, final File destDir, final
         final File[] srcFiles = listFiles(srcDir, fileFilter);
         requireDirectoryIfExists(destDir, "destDir");
         mkdirs(destDir);
-        requireCanWrite(destDir, "destDir");
         for (final File srcFile : srcFiles) {
             final File dstFile = new File(destDir, srcFile.getName());
             if (exclusionList == null || !exclusionList.contains(srcFile.getCanonicalPath())) {
@@ -2623,15 +2622,13 @@ public static FileOutputStream openOutputStream(final File file) throws IOExcept
      * @return a new {@link FileOutputStream} for the specified file
      * @throws NullPointerException if the file object is {@code null}.
      * @throws IllegalArgumentException if the file object is a directory
-     * @throws IllegalArgumentException if the file is not writable.
-     * @throws IOException if the directories could not be created.
+     * @throws IOException if the directories could not be created, or the file is not writable
      * @since 2.1
      */
     public static FileOutputStream openOutputStream(final File file, final boolean append) throws IOException {
         Objects.requireNonNull(file, "file");
         if (file.exists()) {
             checkIsFile(file, "file");
-            requireCanWrite(file, "file");
         } else {
             createParentDirectories(file);
         }
@@ -2774,21 +2771,6 @@ private static void requireCanonicalPathsNotEquals(final File file1, final File
         }
     }
 
-    /**
-     * Throws an {@link IllegalArgumentException} if the file is not writable. This provides a more precise exception
-     * message than a plain access denied.
-     *
-     * @param file The file to test.
-     * @param name The parameter name to use in the exception message.
-     * @throws NullPointerException if the given {@link File} is {@code null}.
-     * @throws IllegalArgumentException if the file is not writable.
-     */
-    private static void requireCanWrite(final File file, final String name) {
-        if (!file.canWrite()) {
-            throw new IllegalArgumentException("File parameter '" + name + " is not writable: '" + file + "'");
-        }
-    }
-
     /**
      * Requires that the given {@link File} exists and is a directory.
      *

src/test/java/org/apache/commons/io/FileUtilsTest.java

@@ -1242,6 +1242,18 @@ public void testCopyFile1ToDir() throws Exception {
             "Should not be able to copy a file into the same directory as itself");
     }
 
+    @Test
+    public void testCopyFileToReadOnlyDirectory() throws Exception {
+        final File directory = new File(tempDirFile, "readonly");
+        if (!directory.exists()) {
+            assumeTrue(directory.mkdirs());
+        }
+        assumeTrue(directory.setWritable(false));
+
+        assertThrows(IOException.class, () -> FileUtils.copyFileToDirectory(testFile1, directory),
+            "Should not be able to copy a file into a readonly directory");
+    }
+
     @Test
     public void testCopyFile2() throws Exception {
         final File destination = new File(tempDirFile, "copy2.txt");