Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clone of https://github.com/google/guava/pull/6730 but without the prod change -- should fail Windows testing #6732

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ jobs:
strategy:
matrix:
os: [ ubuntu-latest ]
java: [ 8, 11, 17 ]
root-pom: [ 'pom.xml', 'android/pom.xml' ]
java: [ 8 ]
root-pom: [ 'pom.xml' ]
include:
- os: windows-latest
java: 17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,6 +65,45 @@ public void testCreateTempDir() throws IOException {
}
}

public void testBogusSystemPropertiesUsername() {
if (isAndroid()) {
/*
* The test calls directly into the "ACL-based filesystem" code, which isn't available under
* old versions of Android. Since Android doesn't use that code path, anyway, there's no need
* to test it.
*/
return;
}

/*
* Only under Windows (or hypothetically when running with some other non-POSIX, ACL-based
* filesystem) does our prod code look up the username. Thus, this test doesn't necessarily test
* anything interesting under most environments. Still, we can run it (except for Android, at
* least old versions), so we mostly do. This is useful because we don't actually run our CI on
* Windows under Java 8, at least as of this writing.
*
* Under Windows in particular, we want to 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.
*/
boolean isJava8 = JAVA_SPECIFICATION_VERSION.value().equals("1.8");

String save = System.getProperty("user.name");
System.setProperty("user.name", "-this-is-definitely-not-the-username-we-are-running-as//?");
try {
TempFileCreator.testMakingUserPermissionsFromScratch();
assertThat(isJava8).isFalse();
} catch (IOException expectedIfJava8) {
assertThat(isJava8).isTrue();
} finally {
System.setProperty("user.name", save);
}
}

private static boolean isAndroid() {
return System.getProperty("java.runtime.name", "").contains("Android");
}
Expand Down
15 changes: 15 additions & 0 deletions android/guava/src/com/google/common/io/TempFileCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import com.google.common.annotations.GwtIncompatible;
import com.google.common.annotations.J2ktIncompatible;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.j2objc.annotations.J2ObjCIncompatible;
import java.io.File;
Expand Down Expand Up @@ -98,6 +99,20 @@ private static TempFileCreator pickSecureCreator() {
return new JavaIoCreator();
}

/**
* Creates the permissions normally used for Windows filesystems, looking up the user afresh, even
* if previous calls have initialized the {@code PermissionSupplier} fields.
*
* <p>This lets us test the effects of different values of the {@code user.name} system property
* without needing a separate VM or classloader.
*/
@IgnoreJRERequirement // used only when Path is available (and only from tests)
@VisibleForTesting
static void testMakingUserPermissionsFromScratch() throws IOException {
// All we're testing is whether it throws.
FileAttribute<?> unused = JavaNioCreator.userPermissions().get();
}

@IgnoreJRERequirement // used only when Path is available
private static final class JavaNioCreator extends TempFileCreator {
@Override
Expand Down
40 changes: 40 additions & 0 deletions guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,6 +65,45 @@ public void testCreateTempDir() throws IOException {
}
}

public void testBogusSystemPropertiesUsername() {
if (isAndroid()) {
/*
* The test calls directly into the "ACL-based filesystem" code, which isn't available under
* old versions of Android. Since Android doesn't use that code path, anyway, there's no need
* to test it.
*/
return;
}

/*
* Only under Windows (or hypothetically when running with some other non-POSIX, ACL-based
* filesystem) does our prod code look up the username. Thus, this test doesn't necessarily test
* anything interesting under most environments. Still, we can run it (except for Android, at
* least old versions), so we mostly do. This is useful because we don't actually run our CI on
* Windows under Java 8, at least as of this writing.
*
* Under Windows in particular, we want to 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.
*/
boolean isJava8 = JAVA_SPECIFICATION_VERSION.value().equals("1.8");

String save = System.getProperty("user.name");
System.setProperty("user.name", "-this-is-definitely-not-the-username-we-are-running-as//?");
try {
TempFileCreator.testMakingUserPermissionsFromScratch();
assertThat(isJava8).isFalse();
} catch (IOException expectedIfJava8) {
assertThat(isJava8).isTrue();
} finally {
System.setProperty("user.name", save);
}
}

private static boolean isAndroid() {
return System.getProperty("java.runtime.name", "").contains("Android");
}
Expand Down
12 changes: 12 additions & 0 deletions guava/src/com/google/common/io/TempFileCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import com.google.common.annotations.GwtIncompatible;
import com.google.common.annotations.J2ktIncompatible;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.j2objc.annotations.J2ObjCIncompatible;
import java.io.File;
Expand Down Expand Up @@ -98,6 +99,17 @@ private static TempFileCreator pickSecureCreator() {
return new JavaIoCreator();
}

/**
* Creates the permissions normally used for Windows filesystems, looking up the user afresh, even
* if previous calls have initialized the PermissionSupplier fields.
*/
@IgnoreJRERequirement // used only when Path is available (and only from tests)
@VisibleForTesting
static void testMakingUserPermissionsFromScratch() throws IOException {
// All we're testing is whether it throws.
FileAttribute<?> unused = JavaNioCreator.userPermissions().get();
}

@IgnoreJRERequirement // used only when Path is available
private static final class JavaNioCreator extends TempFileCreator {
@Override
Expand Down
Loading