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

[Alan Bateman]

On Thu, 31 Aug 2023 21:09:07 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: Check file system access only for move; add move sub-test

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

> 134:             // First ensure source can be read
> 135:             source.getFileSystem().provider().checkAccess(source,
> 136:                                                           AccessMode.READ);

This looks right for the cross-provider case. Minor nit is that you don't ned the line break here. Also the existing comments started in lower case.

src/java.base/unix/classes/sun/nio/fs/UnixFileSystem.java line 920:

> 918:                 // Ensure source can be moved
> 919:                 try {
> 920:                     access(source, W_OK);

Are you sure W_OK is correct and not R_OK or R_OK|W_OK ?

src/java.base/unix/classes/sun/nio/fs/UnixFileSystem.java line 1050:

> 1048:             else if (!sourceAttrs.isSymbolicLink() || flags.followLinks)
> 1049:                 // Ensure source can be read
> 1050:                 provider.checkAccess(source, AccessMode.READ);

I think access(source, R_OK) would be better here, otherwise the additional SM check is observable.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15501#discussion_r1315444819
PR Review Comment: https://git.openjdk.org/jdk/pull/15501#discussion_r1315448193
PR Review Comment: https://git.openjdk.org/jdk/pull/15501#discussion_r1315448697