Skip to content

Commit

Permalink
Fix #12128 Combined ClassLoader Resources (#12130)
Browse files Browse the repository at this point in the history 
Fix #12104 by returning a CombinedResource for a ClassLoader resource that has multiple directory matches.
  • Loading branch information
@gregw
gregw authored Aug 5, 2024
1 parent fe6b14b commit abab594
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 34 deletions.
80 changes: 47 additions & 33 deletions jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,19 @@

package org.eclipse.jetty.util.resource;

import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
import java.util.ListIterator;
import java.util.Objects;
import java.util.StringTokenizer;
import java.util.function.Function;

import org.eclipse.jetty.util.FileID;
import org.eclipse.jetty.util.StringUtil;
Expand Down Expand Up @@ -197,55 +198,68 @@ default Resource newSystemResource(String resource)
*
* @param resource the resource name to find in a classloader
* @param searchSystemClassLoader true to search {@link ClassLoader#getSystemResource(String)}, false to skip
* @return The new Resource
* @return The new Resource, which may be a {@link CombinedResource} if multiple directory resources are found.
* @throws IllegalArgumentException if resource name or resulting URL from ClassLoader is invalid.
*/
default Resource newClassLoaderResource(String resource, boolean searchSystemClassLoader)
{
if (StringUtil.isBlank(resource))
throw new IllegalArgumentException("Resource String is invalid: " + resource);

URL url = null;
// We need a local interface to combine static and non-static methods
interface Source
{
Enumeration<URL> getResources(String name) throws IOException;
}

List<Function<String, URL>> loaders = new ArrayList<>();
loaders.add(Thread.currentThread().getContextClassLoader()::getResource);
loaders.add(ResourceFactory.class.getClassLoader()::getResource);
List<Source> sources = new ArrayList<>();
sources.add(Thread.currentThread().getContextClassLoader()::getResources);
sources.add(ResourceFactory.class.getClassLoader()::getResources);
if (searchSystemClassLoader)
loaders.add(ClassLoader::getSystemResource);
sources.add(ClassLoader::getSystemResources);

for (Function<String, URL> loader : loaders)
{
if (url != null)
break;
List<Resource> resources = new ArrayList<>();
String[] names = resource.startsWith("/") ? new String[] {resource, resource.substring(1)} : new String[] {resource};

try
{
url = loader.apply(resource);
if (url == null && resource.startsWith("/"))
url = loader.apply(resource.substring(1));
}
catch (IllegalArgumentException e)
// For each source of resource
for (Source source : sources)
{
// for each variation of the resource name
for (String name : names)
{
// Catches scenario where a bad Windows path like "C:\dev" is
// improperly escaped, which various downstream classloaders
// tend to have a problem with
if (LOG.isTraceEnabled())
LOG.trace("Ignoring bad getResource(): {}", resource, e);
try
{
// Get all matching URLs
Enumeration<URL> urls = source.getResources(name);
while (urls.hasMoreElements())
{
// Get the resource
Resource r = newResource(urls.nextElement().toURI());
// If it is not a directory, then return it as the singular found resource
if (!r.isDirectory())
return r;
// otherwise add it to a list of resource to combine.
resources.add(r);
}
}
catch (Throwable e)
{
// Catches scenario where a bad Windows path like "C:\dev" is
// improperly escaped, which various downstream classloaders
// tend to have a problem with
if (LOG.isTraceEnabled())
LOG.trace("Ignoring bad getResource(): {}", resource, e);
}
}
}

if (url == null)
if (resources.isEmpty())
return null;

try
{
URI uri = url.toURI();
return newResource(uri);
}
catch (URISyntaxException e)
{
throw new IllegalArgumentException("Error creating resource from URL: " + url, e);
}
if (resources.size() == 1)
return resources.get(0);

return combine(resources);
}

/**
Expand Down
37 changes: 36 additions & 1 deletion jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;

import org.eclipse.jetty.toolchain.test.FS;
Expand All @@ -31,6 +33,7 @@
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.URIUtil;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.api.extension.ExtendWith;
Expand All @@ -43,6 +46,7 @@
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand All @@ -61,13 +65,44 @@ public class ResourceFactoryTest
"TestData/alphabet.txt", "/TestData/alphabet.txt",
"TestData/", "/TestData/", "TestData", "/TestData"
})
public void testNewClassLoaderResourceExists(String reference)
@Disabled
public void testNewClassLoaderResourceExists(String reference) throws IOException
{
Path alt = workDir.getEmptyPathDir().resolve("alt");
Files.createDirectories(alt.resolve("TestData"));
URI altURI = alt.toUri();
ClassLoader loader = new URLClassLoader(new URL[] {altURI.toURL()});

ClassLoader oldLoader = Thread.currentThread().getContextClassLoader();
try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable())
{
Resource altResource = resourceFactory.newResource(altURI);

Thread.currentThread().setContextClassLoader(loader);
Resource resource = resourceFactory.newClassLoaderResource(reference);
assertNotNull(resource, "Reference [" + reference + "] should be found");
assertTrue(resource.exists(), "Reference [" + reference + "] -> Resource[" + resource + "] should exist");

if (resource.isDirectory())
{
assertThat(resource, instanceOf(CombinedResource.class));
AtomicBoolean fromWorkDir = new AtomicBoolean();
AtomicBoolean fromResources = new AtomicBoolean();
resource.forEach(r ->
{
if (r.isContainedIn(altResource))
fromWorkDir.set(true);
else
fromResources.set(true);
});
assertTrue(fromWorkDir.get());
assertTrue(fromResources.get());
}
}
finally
{
Thread.currentThread().setContextClassLoader(oldLoader);
workDir.ensureEmpty();
}
}

Expand Down

0 comments on commit abab594

Please sign in to comment.