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 a31c43770e3b..c0c74d6640ec 100644 --- a/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java +++ b/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -17,6 +17,7 @@ package com.google.common.io; import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; +import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION; 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; @@ -64,6 +65,41 @@ public void testCreateTempDir() throws IOException { } } + public void testBogusSystemPropertiesUsername() { + /* + * Only under Windows (or hypothetically when running with some other non-POSIX, ACL-based + * filesystem) do we look up the username. Thus, this test doesn't test anything interesting + * under most environments. + * + * Under Windows, we test that: + * + * - Under Java 9+, createTempDir() succeeds because it can look up the *real* username, rather + * than relying on the one from the system property. + * + * - Under Java 8, createTempDir() fails because it falls back to the bogus username from the + * system property. + * + * However: Note that we don't actually run our CI on Windows under Java 8, at least as of this + * writing. + */ + boolean isJava8OnWindows = JAVA_SPECIFICATION_VERSION.value().equals("1.8") && isWindows(); + + String save = System.getProperty("user.name"); + System.setProperty("user.name", "-this-is-definitely-not-the-username-we-are-running-as//?"); + File temp = null; + try { + temp = Files.createTempDir(); + assertThat(isJava8OnWindows).isFalse(); + } catch (IllegalStateException expectedIfJavaWindows8) { + assertThat(isJava8OnWindows).isTrue(); + } finally { + System.setProperty("user.name", save); + if (temp != null) { + assertThat(temp.delete()).isTrue(); + } + } + } + private static boolean isAndroid() { return System.getProperty("java.runtime.name", "").contains("Android"); } diff --git a/android/guava/src/com/google/common/io/TempFileCreator.java b/android/guava/src/com/google/common/io/TempFileCreator.java index 96632359906e..7fd2ce2d1ba7 100644 --- a/android/guava/src/com/google/common/io/TempFileCreator.java +++ b/android/guava/src/com/google/common/io/TempFileCreator.java @@ -16,10 +16,12 @@ import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; import static com.google.common.base.StandardSystemProperty.USER_NAME; +import static com.google.common.base.Throwables.throwIfUnchecked; 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 static java.util.Objects.requireNonNull; import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.J2ktIncompatible; @@ -27,6 +29,8 @@ import com.google.j2objc.annotations.J2ObjCIncompatible; import java.io.File; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.nio.file.FileSystems; import java.nio.file.Paths; import java.nio.file.attribute.AclEntry; @@ -150,7 +154,7 @@ private static PermissionSupplier userPermissions() { UserPrincipal user = FileSystems.getDefault() .getUserPrincipalLookupService() - .lookupPrincipalByName(USER_NAME.value()); + .lookupPrincipalByName(getUsername()); ImmutableList acl = ImmutableList.of( AclEntry.newBuilder() @@ -179,6 +183,62 @@ public ImmutableList value() { }; } } + + private static String getUsername() { + /* + * https://github.com/google/guava/issues/6634: ProcessHandle has more accurate information, + * but that class isn't available under all environments that we support. We use it if + * available and fall back if not. + */ + String fromSystemProperty = requireNonNull(USER_NAME.value()); + + try { + Class processHandleClass = Class.forName("java.lang.ProcessHandle"); + Class processHandleInfoClass = Class.forName("java.lang.ProcessHandle$Info"); + Class optionalClass = Class.forName("java.util.Optional"); + /* + * We don't *need* to use reflection to access Optional: It's available on all JDKs we + * support, and Android code won't get this far, anyway, because ProcessHandle is + * unavailable. But given how much other reflection we're using, we might as well use it + * here, too, so that we don't need to also suppress an AndroidApiChecker error. + */ + + Method currentMethod = processHandleClass.getMethod("current"); + Method infoMethod = processHandleClass.getMethod("info"); + Method userMethod = processHandleInfoClass.getMethod("user"); + Method orElseMethod = optionalClass.getMethod("orElse", Object.class); + + Object current = currentMethod.invoke(null); + Object info = infoMethod.invoke(current); + Object user = userMethod.invoke(info); + return (String) requireNonNull(orElseMethod.invoke(user, fromSystemProperty)); + } catch (ClassNotFoundException runningUnderAndroidOrJava8) { + /* + * I'm not sure that we could actually get here for *Android*: I would expect us to enter + * the POSIX code path instead. And if we tried this code path, we'd have trouble unless we + * were running under a new enough version of Android to support NIO. + * + * So this is probably just the "Windows Java 8" case. In that case, if we wanted *another* + * layer of fallback before consulting the system property, we could try + * com.sun.security.auth.module.NTSystem. + * + * But for now, we use the value from the system property as our best guess. + */ + return fromSystemProperty; + } catch (InvocationTargetException e) { + throwIfUnchecked(e.getCause()); // in case it's an Error or something + return fromSystemProperty; // should be impossible + } catch (NoSuchMethodException shouldBeImpossible) { + return fromSystemProperty; + } catch (IllegalAccessException shouldBeImpossible) { + /* + * We don't merge these into `catch (ReflectiveOperationException ...)` or an equivalent + * multicatch because ReflectiveOperationException isn't available under Android: + * b/124188803 + */ + return fromSystemProperty; + } + } } 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 a31c43770e3b..c0c74d6640ec 100644 --- a/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java +++ b/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -17,6 +17,7 @@ package com.google.common.io; import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; +import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION; 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; @@ -64,6 +65,41 @@ public void testCreateTempDir() throws IOException { } } + public void testBogusSystemPropertiesUsername() { + /* + * Only under Windows (or hypothetically when running with some other non-POSIX, ACL-based + * filesystem) do we look up the username. Thus, this test doesn't test anything interesting + * under most environments. + * + * Under Windows, we test that: + * + * - Under Java 9+, createTempDir() succeeds because it can look up the *real* username, rather + * than relying on the one from the system property. + * + * - Under Java 8, createTempDir() fails because it falls back to the bogus username from the + * system property. + * + * However: Note that we don't actually run our CI on Windows under Java 8, at least as of this + * writing. + */ + boolean isJava8OnWindows = JAVA_SPECIFICATION_VERSION.value().equals("1.8") && isWindows(); + + String save = System.getProperty("user.name"); + System.setProperty("user.name", "-this-is-definitely-not-the-username-we-are-running-as//?"); + File temp = null; + try { + temp = Files.createTempDir(); + assertThat(isJava8OnWindows).isFalse(); + } catch (IllegalStateException expectedIfJavaWindows8) { + assertThat(isJava8OnWindows).isTrue(); + } finally { + System.setProperty("user.name", save); + if (temp != null) { + assertThat(temp.delete()).isTrue(); + } + } + } + private static boolean isAndroid() { return System.getProperty("java.runtime.name", "").contains("Android"); } diff --git a/guava/src/com/google/common/io/TempFileCreator.java b/guava/src/com/google/common/io/TempFileCreator.java index 96632359906e..7fd2ce2d1ba7 100644 --- a/guava/src/com/google/common/io/TempFileCreator.java +++ b/guava/src/com/google/common/io/TempFileCreator.java @@ -16,10 +16,12 @@ import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; import static com.google.common.base.StandardSystemProperty.USER_NAME; +import static com.google.common.base.Throwables.throwIfUnchecked; 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 static java.util.Objects.requireNonNull; import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.J2ktIncompatible; @@ -27,6 +29,8 @@ import com.google.j2objc.annotations.J2ObjCIncompatible; import java.io.File; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.nio.file.FileSystems; import java.nio.file.Paths; import java.nio.file.attribute.AclEntry; @@ -150,7 +154,7 @@ private static PermissionSupplier userPermissions() { UserPrincipal user = FileSystems.getDefault() .getUserPrincipalLookupService() - .lookupPrincipalByName(USER_NAME.value()); + .lookupPrincipalByName(getUsername()); ImmutableList acl = ImmutableList.of( AclEntry.newBuilder() @@ -179,6 +183,62 @@ public ImmutableList value() { }; } } + + private static String getUsername() { + /* + * https://github.com/google/guava/issues/6634: ProcessHandle has more accurate information, + * but that class isn't available under all environments that we support. We use it if + * available and fall back if not. + */ + String fromSystemProperty = requireNonNull(USER_NAME.value()); + + try { + Class processHandleClass = Class.forName("java.lang.ProcessHandle"); + Class processHandleInfoClass = Class.forName("java.lang.ProcessHandle$Info"); + Class optionalClass = Class.forName("java.util.Optional"); + /* + * We don't *need* to use reflection to access Optional: It's available on all JDKs we + * support, and Android code won't get this far, anyway, because ProcessHandle is + * unavailable. But given how much other reflection we're using, we might as well use it + * here, too, so that we don't need to also suppress an AndroidApiChecker error. + */ + + Method currentMethod = processHandleClass.getMethod("current"); + Method infoMethod = processHandleClass.getMethod("info"); + Method userMethod = processHandleInfoClass.getMethod("user"); + Method orElseMethod = optionalClass.getMethod("orElse", Object.class); + + Object current = currentMethod.invoke(null); + Object info = infoMethod.invoke(current); + Object user = userMethod.invoke(info); + return (String) requireNonNull(orElseMethod.invoke(user, fromSystemProperty)); + } catch (ClassNotFoundException runningUnderAndroidOrJava8) { + /* + * I'm not sure that we could actually get here for *Android*: I would expect us to enter + * the POSIX code path instead. And if we tried this code path, we'd have trouble unless we + * were running under a new enough version of Android to support NIO. + * + * So this is probably just the "Windows Java 8" case. In that case, if we wanted *another* + * layer of fallback before consulting the system property, we could try + * com.sun.security.auth.module.NTSystem. + * + * But for now, we use the value from the system property as our best guess. + */ + return fromSystemProperty; + } catch (InvocationTargetException e) { + throwIfUnchecked(e.getCause()); // in case it's an Error or something + return fromSystemProperty; // should be impossible + } catch (NoSuchMethodException shouldBeImpossible) { + return fromSystemProperty; + } catch (IllegalAccessException shouldBeImpossible) { + /* + * We don't merge these into `catch (ReflectiveOperationException ...)` or an equivalent + * multicatch because ReflectiveOperationException isn't available under Android: + * b/124188803 + */ + return fromSystemProperty; + } + } } private static final class JavaIoCreator extends TempFileCreator {