RFR: 8073061: (fs) Files.copy(foo, bar, REPLACE_EXISTING) deletes bar even if foo is not readable [v5]

[Alan Bateman]

On Wed, 13 Sep 2023 02:32:31 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Check that the source is readable before deleting the destination.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8073061: Improve file access checking; modify a test; add a test

src/java.base/share/classes/java/nio/file/CopyMoveHelper.java line 138:

> 136: 
> 137:             Files.deleteIfExists(target);
> 138:         } else if (Files.exists(target))

Looking at again and I think this can be dropped from the cross provider case. Files.copy(InputStream, Path) can optionally replace the target so this only need a delete of the target the the source is a directory. So something like this might be simpler:


@@ -69,6 +71,14 @@ static CopyOptions parse(CopyOption... options) {
             }
             return result;
         }
+
+        CopyOption[] replaceExitingOrEmpty() {
+            if (copyAttributes) {
+                return new CopyOption[] { StandardCopyOption.REPLACE_EXISTING };
+            } else {
+                return new CopyOption[0];
+            }
+        }
     }
 
     /**
@@ -129,18 +139,14 @@ static void copyToForeignTarget(Path source, Path target,
         if (sourceAttrs.isSymbolicLink())
             throw new IOException("Copying of symbolic links not supported");
 
-        // delete target if it exists and REPLACE_EXISTING is specified
-        if (opts.replaceExisting) {
-            Files.deleteIfExists(target);
-        } else if (Files.exists(target))
-            throw new FileAlreadyExistsException(target.toString());
-
         // create directory or copy file
         if (sourceAttrs.isDirectory()) {
+            if (opts.replaceExisting)
+                Files.deleteIfExists(target);
             Files.createDirectory(target);
         } else {
             try (InputStream in = Files.newInputStream(source)) {
-                Files.copy(in, target);
+                Files.copy(in, target, opts.replaceExitingOrEmpty());
             }
         }

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15501#discussion_r1327281840