Skip to content

Commit

Permalink
8073061: (fs) Files.copy(foo, bar, REPLACE_EXISTING) deletes bar even…
Browse files Browse the repository at this point in the history
… if foo is not readable

Reviewed-by: alanb
  • Loading branch information
Brian Burkhalter committed Sep 26, 2023
1 parent efb7e85 commit 36ac839
Show file tree
Hide file tree
Showing 5 changed files with 314 additions and 14 deletions.
20 changes: 12 additions & 8 deletions src/java.base/share/classes/java/nio/file/CopyMoveHelper.java
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -69,6 +69,14 @@ static CopyOptions parse(CopyOption... options) {
}
return result;
}

CopyOption[] replaceExistingOrEmpty() {
if (replaceExisting) {
return new CopyOption[] { StandardCopyOption.REPLACE_EXISTING };
} else {
return new CopyOption[0];
}
}
}

/**
Expand Down Expand Up @@ -129,18 +137,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.replaceExistingOrEmpty());
}
}

Expand Down
19 changes: 17 additions & 2 deletions src/java.base/unix/classes/sun/nio/fs/UnixFileSystem.java
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,10 @@ void move(UnixPath source, UnixPath target, CopyOption... options)
// 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);
}
Expand All @@ -910,10 +914,9 @@ void move(UnixPath source, UnixPath target, CopyOption... options)
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 {
Expand Down Expand Up @@ -1019,6 +1022,17 @@ void copy(final UnixPath source,
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);
Expand All @@ -1037,6 +1051,7 @@ void copy(final UnixPath source,
if (!flags.replaceExisting)
throw new FileAlreadyExistsException(
target.getPathForExceptionMessage());

try {
if (targetAttrs.isDirectory()) {
rmdir(target);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -495,7 +495,7 @@ public void createSymbolicLink(Path obj1, Path obj2, FileAttribute<?>... attrs)
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
Expand Down
24 changes: 22 additions & 2 deletions test/jdk/java/nio/file/Files/CopyAndMove.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -35,8 +36,8 @@
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;
Expand Down Expand Up @@ -814,6 +815,25 @@ static void testCopyFileToFile(Path dir1, Path dir2, boolean supportsLinks)
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 --

/*
Expand Down
Loading

1 comment on commit 36ac839

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.