From 63898e263ddcdae00fc760fdaf97d90c8a656608 Mon Sep 17 00:00:00 2001 From: dpb Date: Tue, 12 Sep 2017 18:05:58 -0700 Subject: [PATCH] Make ClassPath search the contents of the "java.class.path" system property for the system class loader in Java 9. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=168478543 --- .../google/common/reflect/ClassPathTest.java | 79 ++++++++++++++----- .../com/google/common/reflect/ClassPath.java | 69 +++++++++++++--- .../google/common/reflect/ClassPathTest.java | 79 ++++++++++++++----- .../com/google/common/reflect/ClassPath.java | 69 +++++++++++++--- 4 files changed, 234 insertions(+), 62 deletions(-) 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 7975ba45f890..f2de1d635f55 100644 --- a/android/guava-tests/test/com/google/common/reflect/ClassPathTest.java +++ b/android/guava-tests/test/com/google/common/reflect/ClassPathTest.java @@ -17,8 +17,11 @@ package com.google.common.reflect; import static com.google.common.base.Charsets.US_ASCII; +import static com.google.common.base.StandardSystemProperty.JAVA_CLASS_PATH; +import static com.google.common.base.StandardSystemProperty.PATH_SEPARATOR; import static com.google.common.truth.Truth.assertThat; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.io.Closer; import com.google.common.io.Files; @@ -39,6 +42,7 @@ import java.net.URL; import java.net.URLClassLoader; import java.security.Permission; +import java.security.PermissionCollection; import java.util.Enumeration; import java.util.HashSet; import java.util.Set; @@ -359,6 +363,40 @@ public void testGetPackageName() { // Test that ResourceInfo.urls() returns identical content to ClassLoader.getResources() + public void testGetClassPathUrls() throws Exception { + String oldPathSeparator = PATH_SEPARATOR.value(); + String oldClassPath = JAVA_CLASS_PATH.value(); + System.setProperty(PATH_SEPARATOR.key(), ":"); + System.setProperty( + JAVA_CLASS_PATH.key(), + Joiner.on(":") + .join( + "relative/path/to/some.jar", + "/absolute/path/to/some.jar", + "relative/path/to/class/root", + "/absolute/path/to/class/root")); + try { + ImmutableList urls = ClassPath.Scanner.parseJavaClassPath(); + + assertThat(urls.get(0).getProtocol()).isEqualTo("file"); + assertThat(urls.get(0).getAuthority()).isNull(); + assertThat(urls.get(0).getPath()).endsWith("/relative/path/to/some.jar"); + + assertThat(urls.get(1)).isEqualTo(new URL("file:///absolute/path/to/some.jar")); + + assertThat(urls.get(2).getProtocol()).isEqualTo("file"); + assertThat(urls.get(2).getAuthority()).isNull(); + assertThat(urls.get(2).getPath()).endsWith("/relative/path/to/class/root"); + + assertThat(urls.get(3)).isEqualTo(new URL("file:///absolute/path/to/class/root")); + + assertThat(urls).hasSize(4); + } finally { + System.setProperty(PATH_SEPARATOR.key(), oldPathSeparator); + System.setProperty(JAVA_CLASS_PATH.key(), oldClassPath); + } + } + private static boolean contentEquals(URL left, URL right) throws IOException { return Resources.asByteSource(left).contentEquals(Resources.asByteSource(right)); } @@ -387,32 +425,37 @@ public void testExistsThrowsSecurityException() throws IOException, URISyntaxExc } private void doTestExistsThrowsSecurityException() throws IOException, URISyntaxException { - URLClassLoader myLoader = (URLClassLoader) getClass().getClassLoader(); - URL[] urls = myLoader.getURLs(); - ImmutableList.Builder filesBuilder = ImmutableList.builder(); - for (URL url : urls) { + File file = null; + // In Java 9, Logger may read the TZ database. Only disallow reading the class path URLs. + final PermissionCollection readClassPathFiles = + new FilePermission("", "read").newPermissionCollection(); + for (URL url : ClassPath.Scanner.parseJavaClassPath()) { if (url.getProtocol().equalsIgnoreCase("file")) { - filesBuilder.add(new File(url.toURI())); + file = new File(url.toURI()); + readClassPathFiles.add(new FilePermission(file.getAbsolutePath(), "read")); } } - ImmutableList files = filesBuilder.build(); - assertThat(files).isNotEmpty(); - SecurityManager disallowFilesSecurityManager = new SecurityManager() { - @Override - public void checkPermission(Permission p) { - if (p instanceof FilePermission) { - throw new SecurityException("Disallowed: " + p); - } - } - }; + assertThat(file).isNotNull(); + SecurityManager disallowFilesSecurityManager = + new SecurityManager() { + @Override + public void checkPermission(Permission p) { + if (readClassPathFiles.implies(p)) { + throw new SecurityException("Disallowed: " + p); + } + } + }; System.setSecurityManager(disallowFilesSecurityManager); try { - files.get(0).exists(); + file.exists(); fail("Did not get expected SecurityException"); } catch (SecurityException expected) { } - ClassPath classPath = ClassPath.from(myLoader); - assertThat(classPath.getResources()).isEmpty(); + ClassPath classPath = ClassPath.from(getClass().getClassLoader()); + // ClassPath may contain resources from the boot class loader; just not from the class path. + for (ResourceInfo resource : classPath.getResources()) { + assertThat(resource.getResourceName()).doesNotContain("com/google/common/reflect/"); + } } private static ClassPath.ClassInfo findClass( diff --git a/android/guava/src/com/google/common/reflect/ClassPath.java b/android/guava/src/com/google/common/reflect/ClassPath.java index 3fe3e4860a9e..f0f03ae242bf 100644 --- a/android/guava/src/com/google/common/reflect/ClassPath.java +++ b/android/guava/src/com/google/common/reflect/ClassPath.java @@ -16,6 +16,9 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.StandardSystemProperty.JAVA_CLASS_PATH; +import static com.google.common.base.StandardSystemProperty.PATH_SEPARATOR; +import static java.util.logging.Level.WARNING; import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; @@ -23,6 +26,7 @@ import com.google.common.base.Predicate; import com.google.common.base.Splitter; import com.google.common.collect.FluentIterable; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; @@ -55,8 +59,13 @@ /** * Scans the source of a {@link ClassLoader} and finds all loadable classes and resources. * - *

Warning: Currently only {@link URLClassLoader} and only {@code file://} urls are - * supported. + *

Warning: Current limitations: + * + *

    + *
  • Looks only for files and JARs in URLs available from {@link URLClassLoader} instances or + * the {@linkplain ClassLoader#getSystemClassLoader() system class loader}. + *
  • Only understands {@code file:} URLs. + *
* *

In the case of directory classloaders, symlinks are supported but cycles are not traversed. * This guarantees discovery of each unique loadable resource. However, not all possible @@ -91,10 +100,16 @@ private ClassPath(ImmutableSet resources) { /** * Returns a {@code ClassPath} representing all classes and resources loadable from {@code - * classloader} and its parent class loaders. + * classloader} and its ancestor class loaders. + * + *

Warning: {@code ClassPath} can find classes and resources only from: * - *

Warning: Currently only {@link URLClassLoader} and only {@code file://} urls are - * supported. + *

    + *
  • {@link URLClassLoader} instances' {@code file:} URLs + *
  • the {@linkplain ClassLoader#getSystemClassLoader() system class loader}. To search the + * system class loader even when it is not a {@link URLClassLoader} (as in Java 9), {@code + * ClassPath} searches the files from the {@code java.class.path} system property. + *
* * @throws IOException if the attempt to read class path resources (jar files or directories) * failed. @@ -432,20 +447,48 @@ static ImmutableMap getClassPathEntries(ClassLoader classload if (parent != null) { entries.putAll(getClassPathEntries(parent)); } - if (classloader instanceof URLClassLoader) { - URLClassLoader urlClassLoader = (URLClassLoader) classloader; - for (URL entry : urlClassLoader.getURLs()) { - if (entry.getProtocol().equals("file")) { - File file = toFile(entry); - if (!entries.containsKey(file)) { - entries.put(file, classloader); - } + for (URL url : getClassLoaderUrls(classloader)) { + if (url.getProtocol().equals("file")) { + File file = toFile(url); + if (!entries.containsKey(file)) { + entries.put(file, classloader); } } } return ImmutableMap.copyOf(entries); } + private static ImmutableList getClassLoaderUrls(ClassLoader classloader) { + if (classloader instanceof URLClassLoader) { + return ImmutableList.copyOf(((URLClassLoader) classloader).getURLs()); + } + if (classloader.equals(ClassLoader.getSystemClassLoader())) { + return parseJavaClassPath(); + } + return ImmutableList.of(); + } + + /** + * Returns the URLs in the class path specified by the {@code java.class.path} {@linkplain + * System#getProperty system property}. + */ + @VisibleForTesting // TODO(b/65488446): Make this a public API. + static ImmutableList parseJavaClassPath() { + ImmutableList.Builder urls = ImmutableList.builder(); + for (String entry : Splitter.on(PATH_SEPARATOR.value()).split(JAVA_CLASS_PATH.value())) { + try { + try { + urls.add(new File(entry).toURI().toURL()); + } catch (SecurityException e) { // File.toURI checks to see if the file is a directory + urls.add(new URL("file", null, new File(entry).getAbsolutePath())); + } + } catch (MalformedURLException e) { + logger.log(WARNING, "malformed classpath entry: " + entry, e); + } + } + return urls.build(); + } + /** * Returns the absolute uri of the Class-Path entry value as specified in * diff --git a/guava-tests/test/com/google/common/reflect/ClassPathTest.java b/guava-tests/test/com/google/common/reflect/ClassPathTest.java index 603598a68305..99ffe7b974a6 100644 --- a/guava-tests/test/com/google/common/reflect/ClassPathTest.java +++ b/guava-tests/test/com/google/common/reflect/ClassPathTest.java @@ -17,6 +17,8 @@ package com.google.common.reflect; import static com.google.common.base.Charsets.US_ASCII; +import static com.google.common.base.StandardSystemProperty.JAVA_CLASS_PATH; +import static com.google.common.base.StandardSystemProperty.PATH_SEPARATOR; import static com.google.common.io.MoreFiles.deleteRecursively; import static com.google.common.truth.Truth.assertThat; import static java.nio.file.Files.createDirectory; @@ -25,6 +27,7 @@ import static java.nio.file.Files.createTempDirectory; import static java.util.logging.Level.WARNING; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.io.Closer; @@ -46,6 +49,7 @@ import java.net.URL; import java.net.URLClassLoader; import java.security.Permission; +import java.security.PermissionCollection; import java.util.Enumeration; import java.util.HashSet; import java.util.Set; @@ -425,6 +429,40 @@ public void testGetPackageName() { // Test that ResourceInfo.urls() returns identical content to ClassLoader.getResources() + public void testGetClassPathUrls() throws Exception { + String oldPathSeparator = PATH_SEPARATOR.value(); + String oldClassPath = JAVA_CLASS_PATH.value(); + System.setProperty(PATH_SEPARATOR.key(), ":"); + System.setProperty( + JAVA_CLASS_PATH.key(), + Joiner.on(":") + .join( + "relative/path/to/some.jar", + "/absolute/path/to/some.jar", + "relative/path/to/class/root", + "/absolute/path/to/class/root")); + try { + ImmutableList urls = ClassPath.Scanner.parseJavaClassPath(); + + assertThat(urls.get(0).getProtocol()).isEqualTo("file"); + assertThat(urls.get(0).getAuthority()).isNull(); + assertThat(urls.get(0).getPath()).endsWith("/relative/path/to/some.jar"); + + assertThat(urls.get(1)).isEqualTo(new URL("file:///absolute/path/to/some.jar")); + + assertThat(urls.get(2).getProtocol()).isEqualTo("file"); + assertThat(urls.get(2).getAuthority()).isNull(); + assertThat(urls.get(2).getPath()).endsWith("/relative/path/to/class/root"); + + assertThat(urls.get(3)).isEqualTo(new URL("file:///absolute/path/to/class/root")); + + assertThat(urls).hasSize(4); + } finally { + System.setProperty(PATH_SEPARATOR.key(), oldPathSeparator); + System.setProperty(JAVA_CLASS_PATH.key(), oldClassPath); + } + } + private static boolean contentEquals(URL left, URL right) throws IOException { return Resources.asByteSource(left).contentEquals(Resources.asByteSource(right)); } @@ -453,32 +491,37 @@ public void testExistsThrowsSecurityException() throws IOException, URISyntaxExc } private void doTestExistsThrowsSecurityException() throws IOException, URISyntaxException { - URLClassLoader myLoader = (URLClassLoader) getClass().getClassLoader(); - URL[] urls = myLoader.getURLs(); - ImmutableList.Builder filesBuilder = ImmutableList.builder(); - for (URL url : urls) { + File file = null; + // In Java 9, Logger may read the TZ database. Only disallow reading the class path URLs. + final PermissionCollection readClassPathFiles = + new FilePermission("", "read").newPermissionCollection(); + for (URL url : ClassPath.Scanner.parseJavaClassPath()) { if (url.getProtocol().equalsIgnoreCase("file")) { - filesBuilder.add(new File(url.toURI())); + file = new File(url.toURI()); + readClassPathFiles.add(new FilePermission(file.getAbsolutePath(), "read")); } } - ImmutableList files = filesBuilder.build(); - assertThat(files).isNotEmpty(); - SecurityManager disallowFilesSecurityManager = new SecurityManager() { - @Override - public void checkPermission(Permission p) { - if (p instanceof FilePermission) { - throw new SecurityException("Disallowed: " + p); - } - } - }; + assertThat(file).isNotNull(); + SecurityManager disallowFilesSecurityManager = + new SecurityManager() { + @Override + public void checkPermission(Permission p) { + if (readClassPathFiles.implies(p)) { + throw new SecurityException("Disallowed: " + p); + } + } + }; System.setSecurityManager(disallowFilesSecurityManager); try { - files.get(0).exists(); + file.exists(); fail("Did not get expected SecurityException"); } catch (SecurityException expected) { } - ClassPath classPath = ClassPath.from(myLoader); - assertThat(classPath.getResources()).isEmpty(); + ClassPath classPath = ClassPath.from(getClass().getClassLoader()); + // ClassPath may contain resources from the boot class loader; just not from the class path. + for (ResourceInfo resource : classPath.getResources()) { + assertThat(resource.getResourceName()).doesNotContain("com/google/common/reflect/"); + } } private static ClassPath.ClassInfo findClass( diff --git a/guava/src/com/google/common/reflect/ClassPath.java b/guava/src/com/google/common/reflect/ClassPath.java index 3fe3e4860a9e..f0f03ae242bf 100644 --- a/guava/src/com/google/common/reflect/ClassPath.java +++ b/guava/src/com/google/common/reflect/ClassPath.java @@ -16,6 +16,9 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.StandardSystemProperty.JAVA_CLASS_PATH; +import static com.google.common.base.StandardSystemProperty.PATH_SEPARATOR; +import static java.util.logging.Level.WARNING; import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; @@ -23,6 +26,7 @@ import com.google.common.base.Predicate; import com.google.common.base.Splitter; import com.google.common.collect.FluentIterable; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; @@ -55,8 +59,13 @@ /** * Scans the source of a {@link ClassLoader} and finds all loadable classes and resources. * - *

Warning: Currently only {@link URLClassLoader} and only {@code file://} urls are - * supported. + *

Warning: Current limitations: + * + *

    + *
  • Looks only for files and JARs in URLs available from {@link URLClassLoader} instances or + * the {@linkplain ClassLoader#getSystemClassLoader() system class loader}. + *
  • Only understands {@code file:} URLs. + *
* *

In the case of directory classloaders, symlinks are supported but cycles are not traversed. * This guarantees discovery of each unique loadable resource. However, not all possible @@ -91,10 +100,16 @@ private ClassPath(ImmutableSet resources) { /** * Returns a {@code ClassPath} representing all classes and resources loadable from {@code - * classloader} and its parent class loaders. + * classloader} and its ancestor class loaders. + * + *

Warning: {@code ClassPath} can find classes and resources only from: * - *

Warning: Currently only {@link URLClassLoader} and only {@code file://} urls are - * supported. + *

    + *
  • {@link URLClassLoader} instances' {@code file:} URLs + *
  • the {@linkplain ClassLoader#getSystemClassLoader() system class loader}. To search the + * system class loader even when it is not a {@link URLClassLoader} (as in Java 9), {@code + * ClassPath} searches the files from the {@code java.class.path} system property. + *
* * @throws IOException if the attempt to read class path resources (jar files or directories) * failed. @@ -432,20 +447,48 @@ static ImmutableMap getClassPathEntries(ClassLoader classload if (parent != null) { entries.putAll(getClassPathEntries(parent)); } - if (classloader instanceof URLClassLoader) { - URLClassLoader urlClassLoader = (URLClassLoader) classloader; - for (URL entry : urlClassLoader.getURLs()) { - if (entry.getProtocol().equals("file")) { - File file = toFile(entry); - if (!entries.containsKey(file)) { - entries.put(file, classloader); - } + for (URL url : getClassLoaderUrls(classloader)) { + if (url.getProtocol().equals("file")) { + File file = toFile(url); + if (!entries.containsKey(file)) { + entries.put(file, classloader); } } } return ImmutableMap.copyOf(entries); } + private static ImmutableList getClassLoaderUrls(ClassLoader classloader) { + if (classloader instanceof URLClassLoader) { + return ImmutableList.copyOf(((URLClassLoader) classloader).getURLs()); + } + if (classloader.equals(ClassLoader.getSystemClassLoader())) { + return parseJavaClassPath(); + } + return ImmutableList.of(); + } + + /** + * Returns the URLs in the class path specified by the {@code java.class.path} {@linkplain + * System#getProperty system property}. + */ + @VisibleForTesting // TODO(b/65488446): Make this a public API. + static ImmutableList parseJavaClassPath() { + ImmutableList.Builder urls = ImmutableList.builder(); + for (String entry : Splitter.on(PATH_SEPARATOR.value()).split(JAVA_CLASS_PATH.value())) { + try { + try { + urls.add(new File(entry).toURI().toURL()); + } catch (SecurityException e) { // File.toURI checks to see if the file is a directory + urls.add(new URL("file", null, new File(entry).getAbsolutePath())); + } + } catch (MalformedURLException e) { + logger.log(WARNING, "malformed classpath entry: " + entry, e); + } + } + return urls.build(); + } + /** * Returns the absolute uri of the Class-Path entry value as specified in *