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

Reviewed-by: alanb
This commit is contained in:
Brian Burkhalter 2023-09-26 15:27:44 +00:00
parent efb7e85ecf
commit 36ac83904c
5 changed files with 314 additions and 14 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -69,6 +69,14 @@ class CopyMoveHelper {
}
return result;
}
CopyOption[] replaceExistingOrEmpty() {
if (replaceExisting) {
return new CopyOption[] { StandardCopyOption.REPLACE_EXISTING };
} else {
return new CopyOption[0];
}
}
}
/**
@ -129,18 +137,14 @@ class CopyMoveHelper {
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.replaceExistingOrEmpty());
}
}

View File

@ -891,6 +891,10 @@ abstract class UnixFileSystem
// get attributes of source file (don't follow links)
try {
sourceAttrs = UnixFileAttributes.get(source, false);
if (sourceAttrs.isDirectory()) {
// ensure source can be moved
access(source, W_OK);
}
} catch (UnixException x) {
x.rethrowAsIOException(source);
}
@ -910,10 +914,9 @@ abstract class UnixFileSystem
if (targetExists) {
if (sourceAttrs.isSameFile(targetAttrs))
return; // nothing to do as files are identical
if (!flags.replaceExisting) {
if (!flags.replaceExisting)
throw new FileAlreadyExistsException(
target.getPathForExceptionMessage());
}
// attempt to delete target
try {
@ -1019,6 +1022,17 @@ abstract class UnixFileSystem
sm.checkPermission(new LinkPermission("symbolic"));
}
// ensure source can be copied
if (!sourceAttrs.isSymbolicLink() || flags.followLinks) {
try {
// the access(2) system call always follows links so it
// is suppressed if the source is an unfollowed link
access(source, R_OK);
} catch (UnixException exc) {
exc.rethrowAsIOException(source);
}
}
// get attributes of target file (don't follow links)
try {
targetAttrs = UnixFileAttributes.get(target, false);
@ -1037,6 +1051,7 @@ abstract class UnixFileSystem
if (!flags.replaceExisting)
throw new FileAlreadyExistsException(
target.getPathForExceptionMessage());
try {
if (targetAttrs.isDirectory()) {
rmdir(target);

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2008, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -495,7 +495,7 @@ public abstract class UnixFileSystemProvider
if (attrs.length > 0) {
UnixFileModeAttribute.toUnixMode(0, attrs); // may throw NPE or UOE
throw new UnsupportedOperationException("Initial file attributes" +
"not supported when creating symbolic link");
" not supported when creating symbolic link");
}
// permission check

View File

@ -22,7 +22,8 @@
*/
/* @test
* @bug 4313887 6838333 6917021 7006126 6950237 8006645 8201407 8264744 8267820
* @bug 4313887 6838333 6917021 7006126 6950237 8006645 8073061 8201407 8264744
* 8267820
* @summary Unit test for java.nio.file.Files copy and move methods (use -Dseed=X to set PRNG seed)
* @library .. /test/lib
* @build jdk.test.lib.Platform jdk.test.lib.RandomFactory
@ -35,8 +36,8 @@ import java.io.*;
import java.nio.ByteBuffer;
import java.nio.file.*;
import static java.nio.file.Files.*;
import static java.nio.file.StandardCopyOption.*;
import static java.nio.file.LinkOption.*;
import static java.nio.file.StandardCopyOption.*;
import java.nio.file.attribute.*;
import java.util.*;
import java.util.concurrent.TimeUnit;
@ -814,6 +815,25 @@ public class CopyAndMove {
delete(target);
/**
* Test: ensure target not deleted if source permissions are zero
*/
source = createSourceFile(dir1);
if (getFileStore(source).supportsFileAttributeView("posix")) {
Files.setPosixFilePermissions(source, Set.of());
target = getTargetFile(dir2);
createFile(target);
try {
Files.copy(source, target, REPLACE_EXISTING);
throw new RuntimeException("AccessDeniedException not thrown");
} catch (AccessDeniedException expected) {
}
if (!Files.exists(target))
throw new RuntimeException("target deleted");
delete(target);
}
delete(source);
// -- directory --
/*

View File

@ -0,0 +1,261 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/* @test
* @bug 8073061
* @requires (os.family == "linux") | (os.family == "mac")
* @summary Test Files.copy and Files.move with numerous parameters
* @run junit CopyMoveVariations
*/
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.AccessDeniedException;
import java.nio.file.CopyOption;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
import static java.nio.file.LinkOption.*;
import static java.nio.file.StandardCopyOption.*;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledIf;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import static org.junit.jupiter.api.Assertions.*;
public class CopyMoveVariations {
enum OpType {
COPY,
MOVE
}
enum PathType {
FILE,
DIR,
LINK
}
private static final boolean SUPPORTS_POSIX_PERMISSIONS;
static {
Path tmp = null;
try {
tmp = Files.createTempFile("this", "that");
SUPPORTS_POSIX_PERMISSIONS =
Files.getFileStore(tmp).supportsFileAttributeView("posix");
} catch (IOException cause) {
throw new UncheckedIOException(cause);
} finally {
if (tmp != null) {
try {
Files.delete(tmp);
} catch (IOException ignore) {
}
}
}
}
private static boolean supportsPosixPermissions() {
return SUPPORTS_POSIX_PERMISSIONS;
}
private static Stream<Arguments> params() {
List<Arguments> list = new ArrayList<Arguments>();
boolean[] falseAndTrue = new boolean[] {false, true};
for (PathType type : PathType.values()) {
String[] modes = new String[] {
"---------", "r--r--r--", "-w--w--w-", "rw-rw-rw-"
};
for (String mode : modes) {
for (boolean replaceExisting : falseAndTrue) {
for (boolean targetExists : falseAndTrue) {
list.add(Arguments.of(type, mode, replaceExisting,
targetExists));
}
}
}
}
return list.stream();
}
@ParameterizedTest
@EnabledIf("supportsPosixPermissions")
@MethodSource("params")
void copyFollow(PathType type, String mode, boolean replaceExisting,
boolean targetExists) throws IOException {
op(OpType.COPY, type, mode, replaceExisting, targetExists, true);
}
@ParameterizedTest
@EnabledIf("supportsPosixPermissions")
@MethodSource("params")
void copyNoFollow(PathType type, String mode, boolean replaceExisting,
boolean targetExists) throws IOException {
op(OpType.COPY, type, mode, replaceExisting, targetExists, false);
}
@ParameterizedTest
@EnabledIf("supportsPosixPermissions")
@MethodSource("params")
void move(PathType type, String mode, boolean replaceExisting,
boolean targetExists) throws IOException {
op(OpType.MOVE, type, mode, replaceExisting, targetExists, false);
}
void op(OpType op, PathType type, String mode, boolean replaceExisting,
boolean targetExists, boolean followLinks) throws IOException {
Path source = null;
Path target = null;
Path linkTarget = null;
try {
switch (type) {
case FILE ->
source = Files.createTempFile("file", "dat");
case DIR ->
source = Files.createTempDirectory("dir");
case LINK -> {
linkTarget = Files.createTempFile("link", "target");
Path link = Path.of("link");
source = Files.createSymbolicLink(link, linkTarget);
}
}
Set<PosixFilePermission> perms =
PosixFilePermissions.fromString(mode);
if (op == OpType.COPY && type == PathType.LINK && followLinks)
Files.setPosixFilePermissions(linkTarget, perms);
else
Files.setPosixFilePermissions(source, perms);
if (targetExists)
target = Files.createTempFile("file", "target");
else
target = Path.of("target");
Set<CopyOption> optionSet = new HashSet();
if (replaceExisting)
optionSet.add(REPLACE_EXISTING);
if (op == OpType.COPY && !followLinks)
optionSet.add(NOFOLLOW_LINKS);
CopyOption[] options = optionSet.toArray(new CopyOption[0]);
final Path src = source;
final Path dst = target;
if (type == PathType.FILE) {
if (op == OpType.COPY) {
try {
Files.copy(source, target, options);
assert Files.exists(target);
} catch (AccessDeniedException ade) {
assertTrue(mode.charAt(0) != 'r');
} catch (FileAlreadyExistsException faee) {
assertTrue(targetExists && !replaceExisting);
}
if (targetExists && mode.charAt(0) == '-')
assertTrue(Files.exists(target));
} else if (!replaceExisting && targetExists) {
assertThrows(FileAlreadyExistsException.class,
() -> Files.move(src, dst, options));
} else {
Files.move(source, target, options);
assert Files.exists(target);
}
} else if (type == PathType.DIR) {
if (op == OpType.COPY) {
try {
Files.copy(source, target, options);
assert Files.exists(target);
} catch (AccessDeniedException ade) {
assertTrue(mode.charAt(0) != 'r');
} catch (FileAlreadyExistsException faee) {
assertTrue(targetExists && !replaceExisting);
}
if (targetExists && mode.charAt(0) == '-')
assertTrue(Files.exists(target));
} else {
try {
Files.move(source, target, options);
assert Files.exists(target);
} catch (AccessDeniedException ade) {
assertTrue(mode.charAt(1) != 'w');
} catch (FileAlreadyExistsException faee) {
assertTrue(targetExists && !replaceExisting);
}
}
} else if (type == PathType.LINK) {
if (op == OpType.COPY) {
try {
Files.copy(source, target, options);
assert Files.exists(target);
} catch (AccessDeniedException ade) {
assertTrue(mode.charAt(0) != 'r');
} catch (FileAlreadyExistsException faee) {
assertTrue(targetExists && !replaceExisting);
}
} else {
try {
Files.move(source, target, options);
assert Files.exists(target);
} catch (AccessDeniedException ade) {
assertTrue(mode.charAt(0) != 'r');
} catch (FileAlreadyExistsException faee) {
assertTrue(targetExists && !replaceExisting);
}
}
} else {
assert false;
}
} finally {
try {
if (source != null)
Files.deleteIfExists(source);
} catch (IOException x) {
}
try {
if (target != null)
Files.deleteIfExists(target);
} catch (IOException x) {
}
try {
if (linkTarget != null)
Files.deleteIfExists(linkTarget);
} catch (IOException x) {
}
}
}
}