From a994ff2730617b69e105625c389c8f770b458fc2 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/FilesCreateTempDirTest.java | 3 + .../com/google/common/io/TempFileCreator.java | 101 +++++++++++++++++- .../common/io/FilesCreateTempDirTest.java | 3 + .../com/google/common/io/TempFileCreator.java | 101 +++++++++++++++++- 4 files changed, 198 insertions(+), 10 deletions(-) diff --git a/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java b/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java index 62098ef0288a..f8f23ccb0c65 100644 --- a/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java +++ b/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -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; diff --git a/android/guava/src/com/google/common/io/TempFileCreator.java b/android/guava/src/com/google/common/io/TempFileCreator.java index 03c5ae99cec6..b7b7ee7fb8cb 100644 --- a/android/guava/src/com/google/common/io/TempFileCreator.java +++ b/android/guava/src/com/google/common/io/TempFileCreator.java @@ -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, @@ -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> 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(USER_NAME.value()); + 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_DATA, + 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 +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 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-tests/test/com/google/common/io/FilesCreateTempDirTest.java b/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java index 62098ef0288a..f8f23ccb0c65 100644 --- a/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java +++ b/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -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; diff --git a/guava/src/com/google/common/io/TempFileCreator.java b/guava/src/com/google/common/io/TempFileCreator.java index 03c5ae99cec6..b7b7ee7fb8cb 100644 --- a/guava/src/com/google/common/io/TempFileCreator.java +++ b/guava/src/com/google/common/io/TempFileCreator.java @@ -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, @@ -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> 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(USER_NAME.value()); + 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_DATA, + 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 +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 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 {