Issue #11092 - Allow MetaInfConfiguration parsing of java.class.path to support globs (#12287)

joakime · web-flow · commit f4c38c433e11 · 2024-09-26T06:40:29.000-05:00
* Issue #11092 - Allow MetaInfConfiguration parsing of java.class.path to support globs * Work with unwrapped URIs for uriPredicate (and test assertion)
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/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java b/jetty-ee10/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/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/MetaInfConfigurationTest.java b/jetty-ee10/jetty-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/jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/MetaInfConfiguration.java b/jetty-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/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/MetaInfConfigurationTest.java b/jetty-ee9/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());
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -447,21 +447,40 @@ default Resource newJarFileResource(URI uri)`
`447`	`447`	`}`
`448`	`448`
`449`	`449`	`/**`
`450`		`- * Split a string of references, that may be split with '{@code ,}', or '{@code ;}', or '{@code \|}' into URIs.`
	`450`	`+ * Split a string of references, that may be split with '{@code ,}', or '{@code ;}', or '{@code \|}' into a List of {@link Resource}.`
`451`	`451`	`* <p>`
`452`		`- * Each part of the input string could be path references (unix or windows style), or string URI references.`
	`452`	`+ * 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/*}).`
`453`	`453`	`* </p>`
`454`	`454`	`* <p>`
`455`	`455`	`* 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:...!/}`
`456`	`456`	`* </p>`
`457`	`457`	`*`
`458`	`458`	`* @param str the input string of references`
	`459`	`+ * @return list of resources`
`459`	`460`	`*/`
`460`	`461`	`default List<Resource> split(String str)`
	`462`	`+ {`
	`463`	`+ return split(str, ",;\|");`
	`464`	`+ }`
	`465`	`+`
	`466`	`+ /**`
	`467`	`+ * Split a string of references by provided delims into a List of {@link Resource}.`
	`468`	`+ * <p>`
	`469`	`+ * 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/*}).`
	`470`	`+ * Note: that if you use the {@code :} character in your delims, then URI references will be impossible.`
	`471`	`+ * </p>`
	`472`	`+ * <p>`
	`473`	`+ * 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:...!/}`
	`474`	`+ * </p>`
	`475`	`+ *`
	`476`	`+ * @param str the input string of references`
	`477`	`+ * @return list of resources`
	`478`	`+ */`
	`479`	`+ default List<Resource> split(String str, String delims)`
`461`	`480`	`{`
`462`	`481`	`List<Resource> list = new ArrayList<>();`
`463`	`482`
`464`		`- StringTokenizer tokenizer = new StringTokenizer(str, ",;\|");`
	`483`	`+ StringTokenizer tokenizer = new StringTokenizer(str, delims);`
`465`	`484`	`while (tokenizer.hasMoreTokens())`
`466`	`485`	`{`
`467`	`486`	`String reference = tokenizer.nextToken();`
`@@ -475,7 +494,6 @@ default List<Resource> split(String str)`
`475`	`494`	`{`
`476`	`495`	`List<Resource> expanded = dir.list();`
`477`	`496`	`expanded.sort(ResourceCollators.byName(true));`
`478`		`- // TODO it is unclear why non archive files are not expanded into the list`
`479`	`497`	`expanded.stream().filter(r -> FileID.isLibArchive(r.getName())).forEach(list::add);`
`480`	`498`	`}`
`481`	`499`	`}`
`@@ -487,11 +505,12 @@ default List<Resource> split(String str)`
`487`	`505`	`}`
`488`	`506`	`catch (Exception e)`
`489`	`507`	`{`
`490`		`- LOG.warn("Invalid Resource Reference: " + reference);`
	`508`	`+ LOG.warn("Invalid Resource Reference: {}", reference);`
`491`	`509`	`throw e;`
`492`	`510`	`}`
`493`	`511`	`}`
`494`	`512`
	`513`	`+ // Perform Archive file mounting (if needed)`
`495`	`514`	`for (ListIterator<Resource> i = list.listIterator(); i.hasNext(); )`
`496`	`515`	`{`
`497`	`516`	`Resource resource = i.next();`
Original file line number	Diff line number	Diff line change
`@@ -166,10 +166,10 @@ public void findAndFilterContainerPaths(final WebAppContext context) throws Exce`
`166`	`166`	`String classPath = System.getProperty("java.class.path");`
`167`	`167`	`if (classPath != null)`
`168`	`168`	`{`
`169`		`- Stream.of(classPath.split(File.pathSeparator))`
`170`		`- .map(resourceFactory::newResource)`
	`169`	`+ resourceFactory.split(classPath, File.pathSeparator)`
	`170`	`+ .stream()`
`171`	`171`	`.filter(Objects::nonNull)`
`172`		`- .filter(r -> uriPatternPredicate.test(r.getURI()))`
	`172`	`+ .filter(r -> uriPatternPredicate.test(URIUtil.unwrapContainer(r.getURI())))`
`173`	`173`	`.forEach(addContainerResource);`
`174`	`174`	`}`
`175`	`175`
Original file line number	Diff line number	Diff line change
`@@ -169,9 +169,10 @@ public void findAndFilterContainerPaths(final WebAppContext context) throws Exce`
`169`	`169`	`String classPath = System.getProperty("java.class.path");`
`170`	`170`	`if (classPath != null)`
`171`	`171`	`{`
`172`		`- Stream.of(classPath.split(File.pathSeparator))`
`173`		`- .map(resourceFactory::newResource)`
`174`		`- .filter(r -> uriPatternPredicate.test(r.getURI()))`
	`172`	`+ resourceFactory.split(classPath, File.pathSeparator)`
	`173`	`+ .stream()`
	`174`	`+ .filter(Objects::nonNull)`
	`175`	`+ .filter(r -> uriPatternPredicate.test(URIUtil.unwrapContainer(r.getURI())))`
`175`	`176`	`.forEach(addContainerResource);`
`176`	`177`	`}`
`177`	`178`
Original file line number	Diff line number	Diff line change
`@@ -20,14 +20,11 @@`
`20`	`20`
`21`	`21`	`import org.eclipse.jetty.server.Server;`
`22`	`22`	`import org.eclipse.jetty.toolchain.test.MavenTestingUtils;`
`23`		`-import org.eclipse.jetty.util.resource.FileSystemPool;`
	`23`	`+import org.eclipse.jetty.util.URIUtil;`
	`24`	`+import org.eclipse.jetty.util.component.LifeCycle;`
`24`	`25`	`import org.eclipse.jetty.util.resource.Resource;`
`25`		`-import org.junit.jupiter.api.AfterEach;`
`26`		`-import org.junit.jupiter.api.BeforeEach;`
`27`	`26`	`import org.junit.jupiter.api.Test;`
`28`	`27`
`29`		`-import static org.hamcrest.MatcherAssert.assertThat;`
`30`		`-import static org.hamcrest.Matchers.empty;`
`31`	`28`	`import static org.junit.jupiter.api.Assertions.assertEquals;`
`32`	`29`	`import static org.junit.jupiter.api.Assertions.assertNotNull;`
`33`	`30`	`import static org.junit.jupiter.api.Assertions.assertTrue;`
`@@ -153,13 +150,14 @@ public void testFindAndFilterContainerPathsJDK9() throws Exception`
`153`	`150`	`assertEquals(2, containerResources.size());`
`154`	`151`	`for (Resource r : containerResources)`
`155`	`152`	`{`
`156`		`- String s = r.toString();`
	`153`	`+ String s = URIUtil.unwrapContainer(r.getURI()).toASCIIString();`
`157`	`154`	`assertTrue(s.endsWith("foo-bar-janb.jar") \|\| s.contains("servlet-api"));`
`158`	`155`	`}`
`159`	`156`	`}`
`160`	`157`	`finally`
`161`	`158`	`{`
`162`	`159`	`config.postConfigure(context);`
	`160`	`+ LifeCycle.stop(context.getResourceFactory());`
`163`	`161`	`}`
`164`	`162`	`}`
`165`	`163`	`}`