Skip to content

Commit

Permalink
Fix Files.createTempDir and FileBackedOutputStream under Windows.
Browse files Browse the repository at this point in the history 
Based on [some discussions in GitHub](#6535 (comment)), I'm under the impression that Windows would create the temporary directory/file in a secure location even if our call to `java.nio.file.Files.createTempDirectory`/`createTempFile` passes no ACL attribute. However, in case we are in an unusual situation (Linux with NFS temporary directory???) in which ACLs are supported but the temporary directory is _not_ a secure location, I've arranged for the file to be created with an ACL that grants permissions only to the current user. I set the user's permissions to the ones that I saw on a file created in the temporary directory under Java's default settings, and I didn't do anything to set the additional permissions I was seeing for administrators. The resulting file's permissions look plausibly correct in the Windows property dialog, if slightly different than what I get when creating a file/directory myself through Explorer.

Fixes #6535

Relates to:
- #4011
- #2575
- #2686. (It sure would be convenient to have Windows CI set up!)

RELNOTES=`io`: Fixed `Files.createTempDir` and `FileBackedOutputStream` under Windows.
PiperOrigin-RevId: 538492945
  • Loading branch information
@cpovirk
cpovirk authored and Google Java Core Libraries committed Jun 7, 2023
1 parent 9916d82 commit a994ff2
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 10 deletions.
3 changes: 3 additions & 0 deletions android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ public void testCreateTempDir() throws IOException {
assertTrue(temp.exists());
assertTrue(temp.isDirectory());
assertThat(temp.listFiles()).isEmpty();
File child = new File(temp, "child");
assertTrue(child.createNewFile());
assertTrue(child.delete());

if (isAndroid()) {
return;
Expand Down
101 changes: 96 additions & 5 deletions android/guava/src/com/google/common/io/TempFileCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,39 @@
package com.google.common.io;

import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
import static com.google.common.base.StandardSystemProperty.USER_NAME;
import static java.nio.file.attribute.AclEntryFlag.DIRECTORY_INHERIT;
import static java.nio.file.attribute.AclEntryFlag.FILE_INHERIT;
import static java.nio.file.attribute.AclEntryPermission.APPEND_DATA;
import static java.nio.file.attribute.AclEntryPermission.DELETE;
import static java.nio.file.attribute.AclEntryPermission.DELETE_CHILD;
import static java.nio.file.attribute.AclEntryPermission.EXECUTE;
import static java.nio.file.attribute.AclEntryPermission.READ_ACL;
import static java.nio.file.attribute.AclEntryPermission.READ_ATTRIBUTES;
import static java.nio.file.attribute.AclEntryPermission.READ_DATA;
import static java.nio.file.attribute.AclEntryPermission.READ_NAMED_ATTRS;
import static java.nio.file.attribute.AclEntryPermission.SYNCHRONIZE;
import static java.nio.file.attribute.AclEntryPermission.WRITE_ACL;
import static java.nio.file.attribute.AclEntryPermission.WRITE_ATTRIBUTES;
import static java.nio.file.attribute.AclEntryPermission.WRITE_DATA;
import static java.nio.file.attribute.AclEntryPermission.WRITE_NAMED_ATTRS;
import static java.nio.file.attribute.AclEntryPermission.WRITE_OWNER;
import static java.nio.file.attribute.AclEntryType.ALLOW;

import com.google.common.annotations.GwtIncompatible;
import com.google.common.annotations.J2ktIncompatible;
import com.google.common.collect.ImmutableList;
import com.google.j2objc.annotations.J2ObjCIncompatible;
import java.io.File;
import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Paths;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.nio.file.attribute.UserPrincipal;
import java.util.Set;
import javax.annotation.CheckForNull;

/**
* Creates temporary files and directories whose permissions are restricted to the current user or,
Expand Down Expand Up @@ -90,16 +112,63 @@ private static TempFileCreator pickSecureCreator() {

@IgnoreJRERequirement // used only when Path is available
private static final class JavaNioCreator extends TempFileCreator {
private static final FileAttribute<Set<PosixFilePermission>> RWX_USER_ONLY =
private static final FileAttribute<?> RWX_USER_ONLY =
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------"));
private static final FileAttribute<Set<PosixFilePermission>> RW_USER_ONLY =
private static final FileAttribute<?> RW_USER_ONLY =
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"));
@CheckForNull private static FileAttribute<?> userOnly;

private static FileAttribute<?> userOnly() throws IOException {
FileAttribute<?> result = userOnly;
if (result != null) {
return result;
}

UserPrincipal user =
FileSystems.getDefault()
.getUserPrincipalLookupService()
.lookupPrincipalByName(USER_NAME.value());
ImmutableList<AclEntry> acl =
ImmutableList.of(
AclEntry.newBuilder()
.setType(ALLOW)
.setPrincipal(user)
.setPermissions(
APPEND_DATA,
DELETE,
DELETE_CHILD,
EXECUTE,
READ_ACL,
READ_ATTRIBUTES,
READ_DATA,
READ_NAMED_ATTRS,
SYNCHRONIZE,
WRITE_ACL,
WRITE_ATTRIBUTES,
WRITE_DATA,
WRITE_NAMED_ATTRS,
WRITE_OWNER)
.setFlags(DIRECTORY_INHERIT, FILE_INHERIT)
.build());
return userOnly =
new FileAttribute<ImmutableList<AclEntry>>() {
@Override
public String name() {
return "acl:acl";
}

@Override
public ImmutableList<AclEntry> value() {
return acl;
}
};
}

@Override
File createTempDir() {
try {
return java.nio.file.Files.createTempDirectory(
Paths.get(JAVA_IO_TMPDIR.value()), /* prefix= */ null, RWX_USER_ONLY)
Paths.get(JAVA_IO_TMPDIR.value()), /* prefix= */ null, directoryPermissions())
.toFile();
} catch (IOException e) {
throw new IllegalStateException("Failed to create directory", e);
Expand All @@ -112,9 +181,31 @@ File createTempFile(String prefix) throws IOException {
Paths.get(JAVA_IO_TMPDIR.value()),
/* prefix= */ prefix,
/* suffix= */ null,
RW_USER_ONLY)
filePermissions())
.toFile();
}

private static FileAttribute<?> directoryPermissions() throws IOException {
Set<String> views = FileSystems.getDefault().supportedFileAttributeViews();
if (views.contains("posix")) {
return RWX_USER_ONLY;
} else if (views.contains("acl")) {
return userOnly();
} else {
throw new IOException("unrecognized FileSystem type " + FileSystems.getDefault());
}
}

private static FileAttribute<?> filePermissions() throws IOException {
Set<String> views = FileSystems.getDefault().supportedFileAttributeViews();
if (views.contains("posix")) {
return RW_USER_ONLY;
} else if (views.contains("acl")) {
return userOnly();
} else {
throw new IOException("unrecognized FileSystem type " + FileSystems.getDefault());
}
}
}

private static final class JavaIoCreator extends TempFileCreator {
Expand Down
3 changes: 3 additions & 0 deletions guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ public void testCreateTempDir() throws IOException {
assertTrue(temp.exists());
assertTrue(temp.isDirectory());
assertThat(temp.listFiles()).isEmpty();
File child = new File(temp, "child");
assertTrue(child.createNewFile());
assertTrue(child.delete());

if (isAndroid()) {
return;
Expand Down
101 changes: 96 additions & 5 deletions guava/src/com/google/common/io/TempFileCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,39 @@
package com.google.common.io;

import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
import static com.google.common.base.StandardSystemProperty.USER_NAME;
import static java.nio.file.attribute.AclEntryFlag.DIRECTORY_INHERIT;
import static java.nio.file.attribute.AclEntryFlag.FILE_INHERIT;
import static java.nio.file.attribute.AclEntryPermission.APPEND_DATA;
import static java.nio.file.attribute.AclEntryPermission.DELETE;
import static java.nio.file.attribute.AclEntryPermission.DELETE_CHILD;
import static java.nio.file.attribute.AclEntryPermission.EXECUTE;
import static java.nio.file.attribute.AclEntryPermission.READ_ACL;
import static java.nio.file.attribute.AclEntryPermission.READ_ATTRIBUTES;
import static java.nio.file.attribute.AclEntryPermission.READ_DATA;
import static java.nio.file.attribute.AclEntryPermission.READ_NAMED_ATTRS;
import static java.nio.file.attribute.AclEntryPermission.SYNCHRONIZE;
import static java.nio.file.attribute.AclEntryPermission.WRITE_ACL;
import static java.nio.file.attribute.AclEntryPermission.WRITE_ATTRIBUTES;
import static java.nio.file.attribute.AclEntryPermission.WRITE_DATA;
import static java.nio.file.attribute.AclEntryPermission.WRITE_NAMED_ATTRS;
import static java.nio.file.attribute.AclEntryPermission.WRITE_OWNER;
import static java.nio.file.attribute.AclEntryType.ALLOW;

import com.google.common.annotations.GwtIncompatible;
import com.google.common.annotations.J2ktIncompatible;
import com.google.common.collect.ImmutableList;
import com.google.j2objc.annotations.J2ObjCIncompatible;
import java.io.File;
import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Paths;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.nio.file.attribute.UserPrincipal;
import java.util.Set;
import javax.annotation.CheckForNull;

/**
* Creates temporary files and directories whose permissions are restricted to the current user or,
Expand Down Expand Up @@ -90,16 +112,63 @@ private static TempFileCreator pickSecureCreator() {

@IgnoreJRERequirement // used only when Path is available
private static final class JavaNioCreator extends TempFileCreator {
private static final FileAttribute<Set<PosixFilePermission>> RWX_USER_ONLY =
private static final FileAttribute<?> RWX_USER_ONLY =
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------"));
private static final FileAttribute<Set<PosixFilePermission>> RW_USER_ONLY =
private static final FileAttribute<?> RW_USER_ONLY =
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"));
@CheckForNull private static FileAttribute<?> userOnly;

private static FileAttribute<?> userOnly() throws IOException {
FileAttribute<?> result = userOnly;
if (result != null) {
return result;
}

UserPrincipal user =
FileSystems.getDefault()
.getUserPrincipalLookupService()
.lookupPrincipalByName(USER_NAME.value());
ImmutableList<AclEntry> acl =
ImmutableList.of(
AclEntry.newBuilder()
.setType(ALLOW)
.setPrincipal(user)
.setPermissions(
APPEND_DATA,
DELETE,
DELETE_CHILD,
EXECUTE,
READ_ACL,
READ_ATTRIBUTES,
READ_DATA,
READ_NAMED_ATTRS,
SYNCHRONIZE,
WRITE_ACL,
WRITE_ATTRIBUTES,
WRITE_DATA,
WRITE_NAMED_ATTRS,
WRITE_OWNER)
.setFlags(DIRECTORY_INHERIT, FILE_INHERIT)
.build());
return userOnly =
new FileAttribute<ImmutableList<AclEntry>>() {
@Override
public String name() {
return "acl:acl";
}

@Override
public ImmutableList<AclEntry> value() {
return acl;
}
};
}

@Override
File createTempDir() {
try {
return java.nio.file.Files.createTempDirectory(
Paths.get(JAVA_IO_TMPDIR.value()), /* prefix= */ null, RWX_USER_ONLY)
Paths.get(JAVA_IO_TMPDIR.value()), /* prefix= */ null, directoryPermissions())
.toFile();
} catch (IOException e) {
throw new IllegalStateException("Failed to create directory", e);
Expand All @@ -112,9 +181,31 @@ File createTempFile(String prefix) throws IOException {
Paths.get(JAVA_IO_TMPDIR.value()),
/* prefix= */ prefix,
/* suffix= */ null,
RW_USER_ONLY)
filePermissions())
.toFile();
}

private static FileAttribute<?> directoryPermissions() throws IOException {
Set<String> views = FileSystems.getDefault().supportedFileAttributeViews();
if (views.contains("posix")) {
return RWX_USER_ONLY;
} else if (views.contains("acl")) {
return userOnly();
} else {
throw new IOException("unrecognized FileSystem type " + FileSystems.getDefault());
}
}

private static FileAttribute<?> filePermissions() throws IOException {
Set<String> views = FileSystems.getDefault().supportedFileAttributeViews();
if (views.contains("posix")) {
return RW_USER_ONLY;
} else if (views.contains("acl")) {
return userOnly();
} else {
throw new IOException("unrecognized FileSystem type " + FileSystems.getDefault());
}
}
}

private static final class JavaIoCreator extends TempFileCreator {
Expand Down

0 comments on commit a994ff2

Please sign in to comment.