diff --git a/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java b/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java index 9d25e42affb3..4ecdcf8d5688 100644 --- a/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java +++ b/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java @@ -16,7 +16,6 @@ package com.google.common.io; -import static com.google.common.base.StandardSystemProperty.OS_NAME; import static com.google.common.io.FileBackedOutputStreamTest.write; import com.google.common.testing.GcFinalization; @@ -31,9 +30,6 @@ public class FileBackedOutputStreamAndroidIncompatibleTest extends IoTestCase { public void testFinalizeDeletesFile() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } byte[] data = newPreFilledByteArray(100); FileBackedOutputStream out = new FileBackedOutputStream(0, true); @@ -55,8 +51,4 @@ public boolean isDone() { } }); } - - private static boolean isWindows() { - return OS_NAME.value().startsWith("Windows"); - } } diff --git a/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java b/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java index 2924cc081909..87247e1ed4d5 100644 --- a/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java +++ b/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java @@ -17,7 +17,6 @@ package com.google.common.io; import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; -import static com.google.common.base.StandardSystemProperty.OS_NAME; import static com.google.common.truth.Truth.assertThat; import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; @@ -41,9 +40,6 @@ public class FileBackedOutputStreamTest extends IoTestCase { public void testThreshold() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } testThreshold(0, 100, true, false); testThreshold(10, 100, true, false); testThreshold(100, 100, true, false); @@ -103,9 +99,6 @@ private void testThreshold( public void testThreshold_resetOnFinalize() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } testThreshold(0, 100, true, true); testThreshold(10, 100, true, true); testThreshold(100, 100, true, true); @@ -131,9 +124,6 @@ static void write(OutputStream out, byte[] b, int off, int len, boolean singleBy // TODO(chrisn): only works if we ensure we have crossed file threshold public void testWriteErrorAfterClose() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } byte[] data = newPreFilledByteArray(100); FileBackedOutputStream out = new FileBackedOutputStream(50); ByteSource source = out.asByteSource(); @@ -177,8 +167,4 @@ public void testReset() throws Exception { private static boolean isAndroid() { return System.getProperty("java.runtime.name", "").contains("Android"); } - - private static boolean isWindows() { - return OS_NAME.value().startsWith("Windows"); - } } 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 228891d27255..66418d233413 100644 --- a/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java +++ b/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -17,7 +17,6 @@ package com.google.common.io; import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; -import static com.google.common.base.StandardSystemProperty.OS_NAME; import static com.google.common.truth.Truth.assertThat; import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE; import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; @@ -39,18 +38,18 @@ @SuppressWarnings("deprecation") // tests of a deprecated method public class FilesCreateTempDirTest extends TestCase { public void testCreateTempDir() throws IOException { - if (isWindows()) { - return; // TODO: b/285742623 - Fix Files.createTempDir under Windows. - } if (JAVA_IO_TMPDIR.value().equals("/sdcard")) { assertThrows(IllegalStateException.class, Files::createTempDir); return; } File temp = Files.createTempDir(); try { - assertTrue(temp.exists()); - assertTrue(temp.isDirectory()); + assertThat(temp.exists()).isTrue(); + assertThat(temp.isDirectory()).isTrue(); assertThat(temp.listFiles()).isEmpty(); + File child = new File(temp, "child"); + assertThat(child.createNewFile()).isTrue(); + assertThat(child.delete()).isTrue(); if (isAndroid()) { return; @@ -60,15 +59,11 @@ public void testCreateTempDir() throws IOException { .readAttributes(); assertThat(attributes.permissions()).containsExactly(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE); } finally { - assertTrue(temp.delete()); + assertThat(temp.delete()).isTrue(); } } private static boolean isAndroid() { return System.getProperty("java.runtime.name", "").contains("Android"); } - - private static boolean isWindows() { - return OS_NAME.value().startsWith("Windows"); - } } diff --git a/android/guava-tests/test/com/google/common/reflect/ClassPathTest.java b/android/guava-tests/test/com/google/common/reflect/ClassPathTest.java index f2de67c19b28..72e3cb5b7304 100644 --- a/android/guava-tests/test/com/google/common/reflect/ClassPathTest.java +++ b/android/guava-tests/test/com/google/common/reflect/ClassPathTest.java @@ -174,13 +174,6 @@ public void testToFile_AndroidIncompatible() throws Exception { @AndroidIncompatible // Android forbids null parent ClassLoader // https://github.com/google/guava/issues/2152 public void testJarFileWithSpaces() throws Exception { - if (isWindows()) { - /* - * TODO: b/285742623 - Fix c.g.c.io.Files.createTempDir under Windows. Or use java.nio.files - * instead? - */ - return; - } URL url = makeJarUrlWithName("To test unescaped spaces in jar file name.jar"); URLClassLoader classloader = new URLClassLoader(new URL[] {url}, null); assertThat(ClassPath.from(classloader).getTopLevelClasses()).isNotEmpty(); @@ -570,6 +563,10 @@ private static File fullpath(String path) { } private static URL makeJarUrlWithName(String name) throws IOException { + /* + * TODO: cpovirk - Use java.nio.file.Files.createTempDirectory instead of + * c.g.c.io.Files.createTempDir? + */ File fullPath = new File(Files.createTempDir(), name); File jarFile = pickAnyJarFile(); Files.copy(jarFile, fullPath); diff --git a/android/guava/src/com/google/common/io/TempFileCreator.java b/android/guava/src/com/google/common/io/TempFileCreator.java index 03c5ae99cec6..54d331164885 100644 --- a/android/guava/src/com/google/common/io/TempFileCreator.java +++ b/android/guava/src/com/google/common/io/TempFileCreator.java @@ -15,16 +15,26 @@ 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.AclEntryType.ALLOW; +import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute; 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.AclEntryPermission; 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.EnumSet; import java.util.Set; /** @@ -90,16 +100,13 @@ 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 = - PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------")); - private static final FileAttribute> RW_USER_ONLY = - PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")); - @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, + PermissionSupplier.INSTANCE.directoryPermissions.get()) .toFile(); } catch (IOException e) { throw new IllegalStateException("Failed to create directory", e); @@ -112,9 +119,82 @@ File createTempFile(String prefix) throws IOException { Paths.get(JAVA_IO_TMPDIR.value()), /* prefix= */ prefix, /* suffix= */ null, - RW_USER_ONLY) + PermissionSupplier.INSTANCE.filePermissions.get()) .toFile(); } + + private interface IoSupplier { + T get() throws IOException; + } + + private static final class PermissionSupplier { + static final PermissionSupplier INSTANCE = pickSupplier(); + + final IoSupplier> filePermissions; + final IoSupplier> directoryPermissions; + + private PermissionSupplier( + IoSupplier> filePermissions, + IoSupplier> directoryPermissions) { + this.filePermissions = filePermissions; + this.directoryPermissions = directoryPermissions; + } + + private PermissionSupplier(IoSupplier> filePermissions) { + this(filePermissions, filePermissions); + } + + private static PermissionSupplier pickSupplier() { + Set views = FileSystems.getDefault().supportedFileAttributeViews(); + if (views.contains("posix")) { + return new PermissionSupplier( + () -> asFileAttribute(PosixFilePermissions.fromString("rw-------")), + () -> asFileAttribute(PosixFilePermissions.fromString("rwx------"))); + } else if (views.contains("acl")) { + return new PermissionSupplier(makeAclSupplier()); + } else { + return new PermissionSupplier( + () -> { + throw new IOException("unrecognized FileSystem type " + FileSystems.getDefault()); + }); + } + } + + private static IoSupplier> makeAclSupplier() { + try { + UserPrincipal user = + FileSystems.getDefault() + .getUserPrincipalLookupService() + .lookupPrincipalByName(USER_NAME.value()); + ImmutableList acl = + ImmutableList.of( + AclEntry.newBuilder() + .setType(ALLOW) + .setPrincipal(user) + .setPermissions(EnumSet.allOf(AclEntryPermission.class)) + .setFlags(DIRECTORY_INHERIT, FILE_INHERIT) + .build()); + FileAttribute> attribute = + new FileAttribute<>() { + @Override + public String name() { + return "acl:acl"; + } + + @Override + public ImmutableList value() { + return acl; + } + }; + return () -> attribute; + } catch (IOException e) { + // We throw a new exception each time so that the stack trace is right. + return () -> { + throw new IOException("Could not find user", e); + }; + } + } + } } private static final class JavaIoCreator extends TempFileCreator { diff --git a/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java b/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java index 9d25e42affb3..4ecdcf8d5688 100644 --- a/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java +++ b/guava-tests/test/com/google/common/io/FileBackedOutputStreamAndroidIncompatibleTest.java @@ -16,7 +16,6 @@ package com.google.common.io; -import static com.google.common.base.StandardSystemProperty.OS_NAME; import static com.google.common.io.FileBackedOutputStreamTest.write; import com.google.common.testing.GcFinalization; @@ -31,9 +30,6 @@ public class FileBackedOutputStreamAndroidIncompatibleTest extends IoTestCase { public void testFinalizeDeletesFile() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } byte[] data = newPreFilledByteArray(100); FileBackedOutputStream out = new FileBackedOutputStream(0, true); @@ -55,8 +51,4 @@ public boolean isDone() { } }); } - - private static boolean isWindows() { - return OS_NAME.value().startsWith("Windows"); - } } diff --git a/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java b/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java index 2924cc081909..87247e1ed4d5 100644 --- a/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java +++ b/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java @@ -17,7 +17,6 @@ package com.google.common.io; import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; -import static com.google.common.base.StandardSystemProperty.OS_NAME; import static com.google.common.truth.Truth.assertThat; import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; @@ -41,9 +40,6 @@ public class FileBackedOutputStreamTest extends IoTestCase { public void testThreshold() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } testThreshold(0, 100, true, false); testThreshold(10, 100, true, false); testThreshold(100, 100, true, false); @@ -103,9 +99,6 @@ private void testThreshold( public void testThreshold_resetOnFinalize() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } testThreshold(0, 100, true, true); testThreshold(10, 100, true, true); testThreshold(100, 100, true, true); @@ -131,9 +124,6 @@ static void write(OutputStream out, byte[] b, int off, int len, boolean singleBy // TODO(chrisn): only works if we ensure we have crossed file threshold public void testWriteErrorAfterClose() throws Exception { - if (isWindows()) { - return; // TODO: b/285742623 - Fix FileBackedOutputStream under Windows. - } byte[] data = newPreFilledByteArray(100); FileBackedOutputStream out = new FileBackedOutputStream(50); ByteSource source = out.asByteSource(); @@ -177,8 +167,4 @@ public void testReset() throws Exception { private static boolean isAndroid() { return System.getProperty("java.runtime.name", "").contains("Android"); } - - private static boolean isWindows() { - return OS_NAME.value().startsWith("Windows"); - } } diff --git a/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java b/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java index 228891d27255..66418d233413 100644 --- a/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java +++ b/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -17,7 +17,6 @@ package com.google.common.io; import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; -import static com.google.common.base.StandardSystemProperty.OS_NAME; import static com.google.common.truth.Truth.assertThat; import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE; import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; @@ -39,18 +38,18 @@ @SuppressWarnings("deprecation") // tests of a deprecated method public class FilesCreateTempDirTest extends TestCase { public void testCreateTempDir() throws IOException { - if (isWindows()) { - return; // TODO: b/285742623 - Fix Files.createTempDir under Windows. - } if (JAVA_IO_TMPDIR.value().equals("/sdcard")) { assertThrows(IllegalStateException.class, Files::createTempDir); return; } File temp = Files.createTempDir(); try { - assertTrue(temp.exists()); - assertTrue(temp.isDirectory()); + assertThat(temp.exists()).isTrue(); + assertThat(temp.isDirectory()).isTrue(); assertThat(temp.listFiles()).isEmpty(); + File child = new File(temp, "child"); + assertThat(child.createNewFile()).isTrue(); + assertThat(child.delete()).isTrue(); if (isAndroid()) { return; @@ -60,15 +59,11 @@ public void testCreateTempDir() throws IOException { .readAttributes(); assertThat(attributes.permissions()).containsExactly(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE); } finally { - assertTrue(temp.delete()); + assertThat(temp.delete()).isTrue(); } } private static boolean isAndroid() { return System.getProperty("java.runtime.name", "").contains("Android"); } - - private static boolean isWindows() { - return OS_NAME.value().startsWith("Windows"); - } } diff --git a/guava-tests/test/com/google/common/reflect/ClassPathTest.java b/guava-tests/test/com/google/common/reflect/ClassPathTest.java index 587419576b72..9a95c4a6a4ea 100644 --- a/guava-tests/test/com/google/common/reflect/ClassPathTest.java +++ b/guava-tests/test/com/google/common/reflect/ClassPathTest.java @@ -180,13 +180,6 @@ public void testToFile_AndroidIncompatible() throws Exception { @AndroidIncompatible // Android forbids null parent ClassLoader // https://github.com/google/guava/issues/2152 public void testJarFileWithSpaces() throws Exception { - if (isWindows()) { - /* - * TODO: b/285742623 - Fix c.g.c.io.Files.createTempDir under Windows. Or use java.nio.files - * instead? - */ - return; - } URL url = makeJarUrlWithName("To test unescaped spaces in jar file name.jar"); URLClassLoader classloader = new URLClassLoader(new URL[] {url}, null); assertThat(ClassPath.from(classloader).getTopLevelClasses()).isNotEmpty(); @@ -635,6 +628,10 @@ private static File fullpath(String path) { } private static URL makeJarUrlWithName(String name) throws IOException { + /* + * TODO: cpovirk - Use java.nio.file.Files.createTempDirectory instead of + * c.g.c.io.Files.createTempDir? + */ File fullPath = new File(Files.createTempDir(), name); File jarFile = pickAnyJarFile(); Files.copy(jarFile, fullPath); diff --git a/guava/src/com/google/common/io/TempFileCreator.java b/guava/src/com/google/common/io/TempFileCreator.java index 03c5ae99cec6..54d331164885 100644 --- a/guava/src/com/google/common/io/TempFileCreator.java +++ b/guava/src/com/google/common/io/TempFileCreator.java @@ -15,16 +15,26 @@ 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.AclEntryType.ALLOW; +import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute; 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.AclEntryPermission; 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.EnumSet; import java.util.Set; /** @@ -90,16 +100,13 @@ 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 = - PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------")); - private static final FileAttribute> RW_USER_ONLY = - PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")); - @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, + PermissionSupplier.INSTANCE.directoryPermissions.get()) .toFile(); } catch (IOException e) { throw new IllegalStateException("Failed to create directory", e); @@ -112,9 +119,82 @@ File createTempFile(String prefix) throws IOException { Paths.get(JAVA_IO_TMPDIR.value()), /* prefix= */ prefix, /* suffix= */ null, - RW_USER_ONLY) + PermissionSupplier.INSTANCE.filePermissions.get()) .toFile(); } + + private interface IoSupplier { + T get() throws IOException; + } + + private static final class PermissionSupplier { + static final PermissionSupplier INSTANCE = pickSupplier(); + + final IoSupplier> filePermissions; + final IoSupplier> directoryPermissions; + + private PermissionSupplier( + IoSupplier> filePermissions, + IoSupplier> directoryPermissions) { + this.filePermissions = filePermissions; + this.directoryPermissions = directoryPermissions; + } + + private PermissionSupplier(IoSupplier> filePermissions) { + this(filePermissions, filePermissions); + } + + private static PermissionSupplier pickSupplier() { + Set views = FileSystems.getDefault().supportedFileAttributeViews(); + if (views.contains("posix")) { + return new PermissionSupplier( + () -> asFileAttribute(PosixFilePermissions.fromString("rw-------")), + () -> asFileAttribute(PosixFilePermissions.fromString("rwx------"))); + } else if (views.contains("acl")) { + return new PermissionSupplier(makeAclSupplier()); + } else { + return new PermissionSupplier( + () -> { + throw new IOException("unrecognized FileSystem type " + FileSystems.getDefault()); + }); + } + } + + private static IoSupplier> makeAclSupplier() { + try { + UserPrincipal user = + FileSystems.getDefault() + .getUserPrincipalLookupService() + .lookupPrincipalByName(USER_NAME.value()); + ImmutableList acl = + ImmutableList.of( + AclEntry.newBuilder() + .setType(ALLOW) + .setPrincipal(user) + .setPermissions(EnumSet.allOf(AclEntryPermission.class)) + .setFlags(DIRECTORY_INHERIT, FILE_INHERIT) + .build()); + FileAttribute> attribute = + new FileAttribute<>() { + @Override + public String name() { + return "acl:acl"; + } + + @Override + public ImmutableList value() { + return acl; + } + }; + return () -> attribute; + } catch (IOException e) { + // We throw a new exception each time so that the stack trace is right. + return () -> { + throw new IOException("Could not find user", e); + }; + } + } + } } private static final class JavaIoCreator extends TempFileCreator {