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 authored and Google Java Core Libraries committed Jun 7, 2023
1 parent e82e2a2 commit b88ced8
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,6 @@
*
* <p>This class is thread-safe.
*
* <p><b>Warning for Windows users:</b> This class is one of the Guava APIs known to <i>not</i> work
* under Windows. Note that <a href="https://github.com/google/guava/issues/2686">we do not run our
* CI under Windows</a>, <a href="https://github.com/google/guava/issues/2130">we know that some of
* our tests fail under Windows</a>, and <a href="https://guava.dev/#important-warnings">we warn
* about using some features of Guava under Windows</a>, especially I/O features, and that warning
* applies even to APIs whose documentation doesn't include individual warnings like this one.
*
* @author Chris Nokleberg
* @since 1.0
*/
Expand Down
8 changes: 0 additions & 8 deletions android/guava/src/com/google/common/io/Files.java
Original file line number Diff line number Diff line change
Expand Up @@ -412,14 +412,6 @@ public static boolean equal(File file1, File file2) throws IOException {
* <p>This method assumes that the temporary volume is writable, has free inodes and free blocks,
* and that it will not be called thousands of times per second.
*
* <p><b>Warning for Windows users:</b> This method is one of the Guava APIs known to <i>not</i>
* work under Windows. Note that <a href="https://github.com/google/guava/issues/2686">we do not
* run our CI under Windows</a>, <a href="https://github.com/google/guava/issues/2130">we know
* that some of our tests fail under Windows</a>, and <a
* href="https://guava.dev/#important-warnings">we warn about using some features of Guava under
* Windows</a>, especially I/O features, and that warning applies even to APIs whose documentation
* doesn't include individual warnings like this one.
*
* <p><b>{@link java.nio.file.Path} equivalent:</b> {@link
* java.nio.file.Files#createTempDirectory}.
*
Expand Down
100 changes: 95 additions & 5 deletions android/guava/src/com/google/common/io/TempFileCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,38 @@
package com.google.common.io;

import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
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 +111,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(System.getProperty("user.name"));
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 +180,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
7 changes: 0 additions & 7 deletions guava/src/com/google/common/io/FileBackedOutputStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,6 @@
*
* <p>This class is thread-safe.
*
* <p><b>Warning for Windows users:</b> This class is one of the Guava APIs known to <i>not</i> work
* under Windows. Note that <a href="https://github.com/google/guava/issues/2686">we do not run our
* CI under Windows</a>, <a href="https://github.com/google/guava/issues/2130">we know that some of
* our tests fail under Windows</a>, and <a href="https://guava.dev/#important-warnings">we warn
* about using some features of Guava under Windows</a>, especially I/O features, and that warning
* applies even to APIs whose documentation doesn't include individual warnings like this one.
*
* @author Chris Nokleberg
* @since 1.0
*/
Expand Down
8 changes: 0 additions & 8 deletions guava/src/com/google/common/io/Files.java
Original file line number Diff line number Diff line change
Expand Up @@ -412,14 +412,6 @@ public static boolean equal(File file1, File file2) throws IOException {
* <p>This method assumes that the temporary volume is writable, has free inodes and free blocks,
* and that it will not be called thousands of times per second.
*
* <p><b>Warning for Windows users:</b> This method is one of the Guava APIs known to <i>not</i>
* work under Windows. Note that <a href="https://github.com/google/guava/issues/2686">we do not
* run our CI under Windows</a>, <a href="https://github.com/google/guava/issues/2130">we know
* that some of our tests fail under Windows</a>, and <a
* href="https://guava.dev/#important-warnings">we warn about using some features of Guava under
* Windows</a>, especially I/O features, and that warning applies even to APIs whose documentation
* doesn't include individual warnings like this one.
*
* <p><b>{@link java.nio.file.Path} equivalent:</b> {@link
* java.nio.file.Files#createTempDirectory}.
*
Expand Down
100 changes: 95 additions & 5 deletions guava/src/com/google/common/io/TempFileCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,38 @@
package com.google.common.io;

import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
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 +111,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(System.getProperty("user.name"));
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 +180,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 b88ced8

Please sign in to comment.