Issue #11092 - Allow MetaInfConfiguration parsing of `java.class.path…

…` to support globs (#12287) * Issue #11092 - Allow MetaInfConfiguration parsing of java.class.path to support globs * Work with unwrapped URIs for uriPredicate (and test assertion)
jetty · Sep 26, 2024 · f4c38c4 · f4c38c4
1 parent 09b8aac
commit f4c38c4
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 20 deletions.
diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java
@@ -447,21 +447,40 @@ default Resource newJarFileResource(URI uri)
     }
 
     /**
-     * Split a string of references, that may be split with '{@code ,}', or '{@code ;}', or '{@code |}' into URIs.
+     * Split a string of references, that may be split with '{@code ,}', or '{@code ;}', or '{@code |}' into a List of {@link Resource}.
      * <p>
-     *     Each part of the input string could be path references (unix or windows style), or string URI references.
+     *     Each part of the input string could be path references (unix or windows style), string URI references, or even glob references (eg: {@code /path/to/libs/*}).
      * </p>
      * <p>
      *     If the result of processing the input segment is a java archive, then its resulting URI will be a mountable URI as {@code jar:file:...!/}
      * </p>
      *
      * @param str the input string of references
+     * @return list of resources
      */
     default List<Resource> split(String str)
+    {
+        return split(str, ",;|");
+    }
+
+    /**
+     * Split a string of references by provided delims into a List of {@link Resource}.
+     * <p>
+     *     Each part of the input string could be path references (unix or windows style), string URI references, or even glob references (eg: {@code /path/to/libs/*}).
+     *     Note: that if you use the {@code :} character in your delims, then URI references will be impossible.
+     * </p>
+     * <p>
+     *     If the result of processing the input segment is a java archive, then its resulting URI will be a mountable URI as {@code jar:file:...!/}
+     * </p>
+     *
+     * @param str the input string of references
+     * @return list of resources
+     */
+    default List<Resource> split(String str, String delims)
     {
         List<Resource> list = new ArrayList<>();
 
-        StringTokenizer tokenizer = new StringTokenizer(str, ",;|");
+        StringTokenizer tokenizer = new StringTokenizer(str, delims);
         while (tokenizer.hasMoreTokens())
         {
             String reference = tokenizer.nextToken();
@@ -475,7 +494,6 @@ default List<Resource> split(String str)
                     {
                         List<Resource> expanded = dir.list();
                         expanded.sort(ResourceCollators.byName(true));
-                        // TODO it is unclear why non archive files are not expanded into the list
                         expanded.stream().filter(r -> FileID.isLibArchive(r.getName())).forEach(list::add);
                     }
                 }
@@ -487,11 +505,12 @@ default List<Resource> split(String str)
             }
             catch (Exception e)
             {
-                LOG.warn("Invalid Resource Reference: " + reference);
+                LOG.warn("Invalid Resource Reference: {}", reference);
                 throw e;
             }
         }
 
+        // Perform Archive file mounting (if needed)
         for (ListIterator<Resource> i = list.listIterator(); i.hasNext(); )
         {
             Resource resource = i.next();

diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceFactoryTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceFactoryTest.java
@@ -405,7 +405,7 @@ public void testSplitOnSemicolon()
     }
 
     @Test
-    public void testSplitOnPipeWithGlob() throws IOException
+    public void testSplitOnPathSeparatorWithGlob() throws IOException
     {
         try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable())
         {
@@ -418,9 +418,54 @@ public void testSplitOnPipeWithGlob() throws IOException
             FS.ensureDirExists(bar);
             Files.copy(MavenPaths.findTestResourceFile("jar-file-resource.jar"), bar.resolve("lib-foo.jar"));
             Files.copy(MavenPaths.findTestResourceFile("jar-file-resource.jar"), bar.resolve("lib-zed.zip"));
+            Path exampleJar = base.resolve("example.jar");
+            Files.copy(MavenPaths.findTestResourceFile("example.jar"), exampleJar);
+
+            // This represents a classpath with a glob
+            String config = String.join(File.pathSeparator, List.of(
+                dir.toString(), foo.toString(), bar + File.separator + "*", exampleJar.toString()
+            ));
+
+            // Split using commas
+            List<URI> uris = resourceFactory.split(config, File.pathSeparator).stream().map(Resource::getURI).toList();
+
+            URI[] expected = new URI[]{
+                dir.toUri(),
+                foo.toUri(),
+                // Should see the two archives as `jar:file:` URI entries
+                URIUtil.toJarFileUri(bar.resolve("lib-foo.jar").toUri()),
+                URIUtil.toJarFileUri(bar.resolve("lib-zed.zip").toUri()),
+                URIUtil.toJarFileUri(exampleJar.toUri())
+            };
+
+            assertThat(uris, contains(expected));
+        }
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {";", "|", ","})
+    public void testSplitOnDelimWithGlob(String delimChar) throws IOException
+    {
+        try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable())
+        {
+            // TIP: don't allow raw delim to show up in base dir, otherwise the string split later will be wrong.
+            Path base = MavenPaths.targetTestDir("testSplitOnPipeWithGlob_%02x".formatted((byte)delimChar.charAt(0)));
+            FS.ensureEmpty(base);
+            Path dir = base.resolve("dir");
+            FS.ensureDirExists(dir);
+            Path foo = dir.resolve("foo");
+            FS.ensureDirExists(foo);
+            Path bar = dir.resolve("bar");
+            FS.ensureDirExists(bar);
+            Files.copy(MavenPaths.findTestResourceFile("jar-file-resource.jar"), bar.resolve("lib-foo.jar"));
+            Files.copy(MavenPaths.findTestResourceFile("jar-file-resource.jar"), bar.resolve("lib-zed.zip"));
+            Path exampleJar = base.resolve("example.jar");
+            Files.copy(MavenPaths.findTestResourceFile("example.jar"), exampleJar);
 
             // This represents the user-space raw configuration with a glob
-            String config = String.format("%s;%s;%s%s*", dir, foo, bar, File.separator);
+            String config = String.join(delimChar, List.of(
+                dir.toString(), foo.toString(), bar + File.separator + "*", exampleJar.toString()
+            ));
 
             // Split using commas
             List<URI> uris = resourceFactory.split(config).stream().map(Resource::getURI).toList();
@@ -430,7 +475,8 @@ public void testSplitOnPipeWithGlob() throws IOException
                 foo.toUri(),
                 // Should see the two archives as `jar:file:` URI entries
                 URIUtil.toJarFileUri(bar.resolve("lib-foo.jar").toUri()),
-                URIUtil.toJarFileUri(bar.resolve("lib-zed.zip").toUri())
+                URIUtil.toJarFileUri(bar.resolve("lib-zed.zip").toUri()),
+                URIUtil.toJarFileUri(exampleJar.toUri())
             };
 
             assertThat(uris, contains(expected));

diff --git a/...0/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java b/...0/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java
@@ -166,10 +166,10 @@ public void findAndFilterContainerPaths(final WebAppContext context) throws Exce
         String classPath = System.getProperty("java.class.path");
         if (classPath != null)
         {
-            Stream.of(classPath.split(File.pathSeparator))
-                .map(resourceFactory::newResource)
+            resourceFactory.split(classPath, File.pathSeparator)
+                .stream()
                 .filter(Objects::nonNull)
-                .filter(r -> uriPatternPredicate.test(r.getURI()))
+                .filter(r -> uriPatternPredicate.test(URIUtil.unwrapContainer(r.getURI())))
                 .forEach(addContainerResource);
         }
 

diff --git a/...tty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/MetaInfConfigurationTest.java b/...tty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/MetaInfConfigurationTest.java
@@ -557,6 +557,7 @@ public void testGetContainerPathsWithModuleSystem() throws Exception
                 .stream()
                 .sorted(ResourceCollators.byName(true))
                 .map(Resource::getURI)
+                .map(URIUtil::unwrapContainer)
                 .map(URI::toASCIIString)
                 .toList();
             // we "correct" the bad file URLs that come from the ClassLoader

diff --git a/...ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/MetaInfConfiguration.java b/...ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/MetaInfConfiguration.java
@@ -169,9 +169,10 @@ public void findAndFilterContainerPaths(final WebAppContext context) throws Exce
         String classPath = System.getProperty("java.class.path");
         if (classPath != null)
         {
-            Stream.of(classPath.split(File.pathSeparator))
-                .map(resourceFactory::newResource)
-                .filter(r -> uriPatternPredicate.test(r.getURI()))
+            resourceFactory.split(classPath, File.pathSeparator)
+                .stream()
+                .filter(Objects::nonNull)
+                .filter(r -> uriPatternPredicate.test(URIUtil.unwrapContainer(r.getURI())))
                 .forEach(addContainerResource);
         }
 

diff --git a/...jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/MetaInfConfigurationTest.java b/...jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/MetaInfConfigurationTest.java
@@ -20,14 +20,11 @@
 
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
-import org.eclipse.jetty.util.resource.FileSystemPool;
+import org.eclipse.jetty.util.URIUtil;
+import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.util.resource.Resource;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.empty;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -153,13 +150,14 @@ public void testFindAndFilterContainerPathsJDK9() throws Exception
             assertEquals(2, containerResources.size());
             for (Resource r : containerResources)
             {
-                String s = r.toString();
+                String s = URIUtil.unwrapContainer(r.getURI()).toASCIIString();
                 assertTrue(s.endsWith("foo-bar-janb.jar") || s.contains("servlet-api"));
             }
         }
         finally
         {
             config.postConfigure(context);
+            LifeCycle.stop(context.getResourceFactory());
         }
     }
 }