[ https://issues.apache.org/jira/browse/GROOVY-11978 ]
Paul King deleted comment on GROOVY-11978:
------------------------------------
was (Author: githubbot):
Copilot commented on code in PR #2504:
URL: https://github.com/apache/groovy/pull/2504#discussion_r3175977209
##########
src/main/java/org/codehaus/groovy/tools/RootLoader.java:
##########
@@ -109,8 +113,16 @@ public RootLoader(final LoaderConfiguration lc) {
Thread.currentThread().setContextClassLoader(this);
+ // Skip URLs already on the bootstrap (system) classpath: the launcher
+ // puts the core groovy jar on -classpath to start GroovyStarter, and
+ // groovy-starter.conf's "load lib/*.jar" then re-globs it. Adding the
+ // same jar to both loaders lets each define its own copy of every core
+ // class, and trips duplicate-resource detection.
+ Set<File> bootstrap = bootstrapClasspathFiles();
Review Comment:
The new logic is based on `System.getProperty("java.class.path")`, which is
the application/system classpath, not the JVM bootstrap classpath. To avoid
confusion for future maintainers, consider renaming
`bootstrapClasspathFiles`/`bootstrap` and adjusting the comment to refer to the
system/application classpath explicitly.
##########
src/main/java/org/codehaus/groovy/tools/RootLoader.java:
##########
@@ -179,4 +191,33 @@ public void addURL(final URL url) {
protected Class<?> findClass(final String name) throws
ClassNotFoundException {
throw new ClassNotFoundException(name);
}
+
+ private static Set<File> bootstrapClasspathFiles() {
+ Set<File> files = new HashSet<>();
+ String cp = System.getProperty("java.class.path");
+ if (cp == null || cp.isEmpty()) return files;
+ for (String entry : cp.split(File.pathSeparator)) {
+ if (entry.isEmpty()) continue;
+ File f = canonicalFile(new File(entry));
+ if (f != null) files.add(f);
+ }
+ return files;
+ }
+
+ private static File canonicalFile(URL url) {
+ if (!"file".equals(url.getProtocol())) return null;
+ try {
+ return canonicalFile(new File(url.toURI()));
+ } catch (URISyntaxException | IllegalArgumentException e) {
+ return null;
+ }
+ }
+
+ private static File canonicalFile(File f) {
+ try {
+ return f.getCanonicalFile();
+ } catch (IOException e) {
Review Comment:
`File#getCanonicalFile()` may also throw `SecurityException` if read access
is denied. Consider catching `SecurityException` alongside `IOException` and
falling back to `getAbsoluteFile()` to keep behavior consistent with the intent
of this helper.
##########
src/main/java/org/codehaus/groovy/tools/RootLoader.java:
##########
@@ -179,4 +191,33 @@ public void addURL(final URL url) {
protected Class<?> findClass(final String name) throws
ClassNotFoundException {
throw new ClassNotFoundException(name);
}
+
+ private static Set<File> bootstrapClasspathFiles() {
+ Set<File> files = new HashSet<>();
+ String cp = System.getProperty("java.class.path");
Review Comment:
`System.getProperty("java.class.path")` can throw `SecurityException` in
restricted environments. Consider catching `SecurityException` here (and
treating it like an empty classpath) so RootLoader construction doesn't fail
under a security manager / sandboxed execution.
> RootLoader returns duplicate resource URLs when the same jar appears on
> parent and child classpaths
> ---------------------------------------------------------------------------------------------------
>
> Key: GROOVY-11978
> URL: https://issues.apache.org/jira/browse/GROOVY-11978
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> {\{RootLoader}} (used by the \{{groovy}}, \{{groovyc}}, \{{groovysh}},
> \{{groovyConsole}}
> and \{{grape}} launcher scripts) inherits the JVM startup classpath as its
> parent
> loader. The launcher puts the core groovy jar on \{{-classpath}} to bootstrap
> {\{GroovyStarter}}, and \{{groovy-starter.conf}}'s \{{load
> !\{groovy.home\}/lib/*.jar}}
> glob then re-adds the same jar to \{{RootLoader}} itself. As a result, the
> same
> jar appears in two places in the loader hierarchy.
> This causes \{{RootLoader.getResources(name)}} to return two URLs that point
> to
> the same entry inside the same jar. Anything that walks the enumeration and
> expects providers to be unique trips on this — for example
> \{{java.util.ServiceLoader}},
> or any \{{ResourceBundle}}/lookup code that aborts on duplicate hits.
> h3. Reproduction
> Run any launcher that hits resource enumeration on the core jar;
> \{{groovyConsole}}
> fails on startup with:
> {noformat}
> java.util.MissingResourceException: Can't find bundle for base name
> groovy.console.ui.Console, locale en_AU
> at
> java.base/java.util.ResourceBundle.throwMissingResourceException(ResourceBundle.java:2045)
> ...
> at
> org.codehaus.groovy.tools.shell.util.MessageSource.createBundles(MessageSource.java:78)
> at
> org.codehaus.groovy.tools.shell.util.MessageSource.getMessage(MessageSource.java:99)
> at groovy.console.ui.Console.main(Console.groovy:336)
> {noformat}
> h3. Fix
> Override \{{RootLoader.getResources(String)}} so URLs whose canonical jar file
> has already been seen are collapsed. Walks \{{findResources}} first then the
> parent chain, matching the existing child-first behaviour of \{{getResource}}.
> Non-jar URLs (directories, custom protocols) are passed through unchanged;
> canonical-file matching means symlinked jars also dedup correctly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
