From e5af70717f9509d3b22cd95decf5d096bc70104b Mon Sep 17 00:00:00 2001 From: cpovirk Date: Wed, 7 Jun 2023 08:18:08 -0700 Subject: [PATCH] Fix `Files.createTempDir` and `FileBackedOutputStream` under Windows. Based on [some discussions in GitHub](https://github.com/google/guava/issues/6535#issuecomment-1579806211), 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 https://github.com/google/guava/issues/6535 Relates to: - https://github.com/google/guava/issues/4011 - https://github.com/google/guava/issues/2575 - https://github.com/google/guava/issues/2686. (It sure would be convenient to have Windows CI set up!) RELNOTES=`io`: Fixed `Files.createTempDir` and `FileBackedOutputStream` under Windows. PiperOrigin-RevId: 538492945 --- .../common/io/FileBackedOutputStream.java | 7 -- .../guava/src/com/google/common/io/Files.java | 13 +-- .../com/google/common/io/TempFileCreator.java | 98 ++++++++++++++++++- .../common/io/FileBackedOutputStream.java | 7 -- guava/src/com/google/common/io/Files.java | 13 +-- .../com/google/common/io/TempFileCreator.java | 98 ++++++++++++++++++- 6 files changed, 192 insertions(+), 44 deletions(-) diff --git a/android/guava/src/com/google/common/io/FileBackedOutputStream.java b/android/guava/src/com/google/common/io/FileBackedOutputStream.java index 51b279971ef7..dd297e90a8b2 100644 --- a/android/guava/src/com/google/common/io/FileBackedOutputStream.java +++ b/android/guava/src/com/google/common/io/FileBackedOutputStream.java @@ -58,13 +58,6 @@ * *

This class is thread-safe. * - *

Warning for Windows users: This class is one of the Guava APIs known to not work - * under Windows. Note that we do not run our - * CI under Windows, we know that some of - * our tests fail under Windows, and we warn - * about using some features of Guava under Windows, 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 */ diff --git a/android/guava/src/com/google/common/io/Files.java b/android/guava/src/com/google/common/io/Files.java index ffa92f6bfc58..2a25d2fcd896 100644 --- a/android/guava/src/com/google/common/io/Files.java +++ b/android/guava/src/com/google/common/io/Files.java @@ -412,20 +412,13 @@ public static boolean equal(File file1, File file2) throws IOException { *

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

Warning for Windows users: This method is one of the Guava APIs known to not - * work under Windows. Note that we do not - * run our CI under Windows, we know - * that some of our tests fail under Windows, and we warn about using some features of Guava under - * Windows, especially I/O features, and that warning applies even to APIs whose documentation - * doesn't include individual warnings like this one. - * *

{@link java.nio.file.Path} equivalent: {@link * java.nio.file.Files#createTempDirectory}. * * @return the newly-created directory - * @throws IllegalStateException if the directory could not be created, such as if the system does - * not support creating temporary directories securely + * @throws IllegalStateException if the directory could not be created + * @throws UnsupportedOperationException if the system does not support creating temporary + * directories securely * @deprecated For Android users, see the Data and File * Storage overview to select an appropriate temporary directory (perhaps {@code diff --git a/android/guava/src/com/google/common/io/TempFileCreator.java b/android/guava/src/com/google/common/io/TempFileCreator.java index 03c5ae99cec6..e3de72283399 100644 --- a/android/guava/src/com/google/common/io/TempFileCreator.java +++ b/android/guava/src/com/google/common/io/TempFileCreator.java @@ -15,17 +15,37 @@ 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_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, @@ -90,16 +110,62 @@ private static TempFileCreator pickSecureCreator() { @IgnoreJRERequirement // used only when Path is available private static final class JavaNioCreator extends TempFileCreator { - private static final FileAttribute> RWX_USER_ONLY = + private static final FileAttribute RWX_USER_ONLY = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------")); - private static final FileAttribute> 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 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_NAMED_ATTRS, + WRITE_OWNER) + .setFlags(DIRECTORY_INHERIT, FILE_INHERIT) + .build()); + return userOnly = + new FileAttribute>() { + @Override + public String name() { + return "acl:acl"; + } + + @Override + public ImmutableList 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); @@ -112,9 +178,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 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 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 { diff --git a/guava/src/com/google/common/io/FileBackedOutputStream.java b/guava/src/com/google/common/io/FileBackedOutputStream.java index 51b279971ef7..dd297e90a8b2 100644 --- a/guava/src/com/google/common/io/FileBackedOutputStream.java +++ b/guava/src/com/google/common/io/FileBackedOutputStream.java @@ -58,13 +58,6 @@ * *

This class is thread-safe. * - *

Warning for Windows users: This class is one of the Guava APIs known to not work - * under Windows. Note that we do not run our - * CI under Windows, we know that some of - * our tests fail under Windows, and we warn - * about using some features of Guava under Windows, 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 */ diff --git a/guava/src/com/google/common/io/Files.java b/guava/src/com/google/common/io/Files.java index ffa92f6bfc58..2a25d2fcd896 100644 --- a/guava/src/com/google/common/io/Files.java +++ b/guava/src/com/google/common/io/Files.java @@ -412,20 +412,13 @@ public static boolean equal(File file1, File file2) throws IOException { *

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

Warning for Windows users: This method is one of the Guava APIs known to not - * work under Windows. Note that we do not - * run our CI under Windows, we know - * that some of our tests fail under Windows, and we warn about using some features of Guava under - * Windows, especially I/O features, and that warning applies even to APIs whose documentation - * doesn't include individual warnings like this one. - * *

{@link java.nio.file.Path} equivalent: {@link * java.nio.file.Files#createTempDirectory}. * * @return the newly-created directory - * @throws IllegalStateException if the directory could not be created, such as if the system does - * not support creating temporary directories securely + * @throws IllegalStateException if the directory could not be created + * @throws UnsupportedOperationException if the system does not support creating temporary + * directories securely * @deprecated For Android users, see the Data and File * Storage overview to select an appropriate temporary directory (perhaps {@code diff --git a/guava/src/com/google/common/io/TempFileCreator.java b/guava/src/com/google/common/io/TempFileCreator.java index 03c5ae99cec6..e3de72283399 100644 --- a/guava/src/com/google/common/io/TempFileCreator.java +++ b/guava/src/com/google/common/io/TempFileCreator.java @@ -15,17 +15,37 @@ 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_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, @@ -90,16 +110,62 @@ private static TempFileCreator pickSecureCreator() { @IgnoreJRERequirement // used only when Path is available private static final class JavaNioCreator extends TempFileCreator { - private static final FileAttribute> RWX_USER_ONLY = + private static final FileAttribute RWX_USER_ONLY = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------")); - private static final FileAttribute> 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 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_NAMED_ATTRS, + WRITE_OWNER) + .setFlags(DIRECTORY_INHERIT, FILE_INHERIT) + .build()); + return userOnly = + new FileAttribute>() { + @Override + public String name() { + return "acl:acl"; + } + + @Override + public ImmutableList 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); @@ -112,9 +178,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 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 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 {