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

8317128: java/nio/file/Files/CopyAndMove.java failed with AccessDeniedException

Reviewed-by: phh
Backport-of: 36ac83904c9e81a01822b0e36ef677cae2808709
  • Loading branch information
toddjonker authored and shipilev committed Aug 14, 2024
1 parent 108c2e4 commit 590715f
Show file tree
Hide file tree
Showing 6 changed files with 316 additions and 12 deletions.
16 changes: 12 additions & 4 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 All @@ -25,9 +25,13 @@

package java.nio.file;

import java.nio.file.attribute.*;
import java.io.InputStream;
import java.io.IOException;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.BasicFileAttributeView;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.spi.FileSystemProvider;

/**
* Helper class to support copying or moving files when the source and target
Expand Down Expand Up @@ -129,10 +133,14 @@ static void copyToForeignTarget(Path source, Path target,
if (sourceAttrs.isSymbolicLink())
throw new IOException("Copying of symbolic links not supported");

// ensure source can be copied
FileSystemProvider provider = source.getFileSystem().provider();
provider.checkAccess(source, AccessMode.READ);

// delete target if it exists and REPLACE_EXISTING is specified
if (opts.replaceExisting) {
if (opts.replaceExisting)
Files.deleteIfExists(target);
} else if (Files.exists(target))
else if (Files.exists(target))
throw new FileAlreadyExistsException(target.toString());

// create directory or copy file
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 @@ -882,6 +882,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 @@ -901,10 +905,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 @@ -1010,6 +1013,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 @@ -1028,6 +1042,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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2016, 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 @@ -31,7 +31,7 @@
import static sun.nio.fs.WindowsConstants.*;

/**
* Internal exception thrown when a Win32 calls fails.
* Internal exception thrown when a Win32 call fails.
*/

class WindowsException extends Exception {
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 590715f

@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.